You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2020/09/25 08:25:36 UTC
[GitHub] [rocketmq-operator] linjiemiao opened a new pull request #61: [ISSUEE 60] Add features to the broker and nameserver
linjiemiao opened a new pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61
1. add affinity / securityContext / imagePullSecrets / tolerations / nodeSelector / podAnnotations / priorityClassName
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #61: [ISSUEE 60] Add features to the broker and nameserver
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61#discussion_r834277621
##########
File path: pkg/apis/rocketmq/v1alpha1/nameservice_types.go
##########
@@ -49,6 +49,20 @@ type NameServiceSpec struct {
HostPath string `json:"hostPath"`
// VolumeClaimTemplates defines the StorageClass
VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`
+ // Affinity, affinity and anti-affinity scheduling
+ Affinity *corev1.Affinity `json:"affinity,omitempty"`
+ // SecurityContext defines privilege and access control settings for a Pod or Container.
+ SecurityContext *corev1.PodSecurityContext `json:"securityContext,omitempty"`
+ // ImagePullSecrets pull an image from a private registry
+ ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
+ // Tolerations taints and tolerations work together to ensure that pods are not scheduled onto inappropriate nodes
+ Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
+ // NodeSelector is the simplest recommended form of node selection constraint.
+ NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+ // PodAnnotations you can use annotations to attach arbitrary non-identifying metadata to objects.
+ PodAnnotations map[string]string `json:"podAnnotations,omitempty"`
+ // PriorityClassName defines priority class's name
+ PriorityClassName string `json:"priorityClassName,omitempty"`
Review comment:
It seems null checks are required for them, would define them as pointer be better?
##########
File path: pkg/apis/rocketmq/v1alpha1/broker_types.go
##########
@@ -56,6 +56,21 @@ type BrokerSpec struct {
VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`
// The name of pod where the metadata from
ScalePodName string `json:"scalePodName"`
+ // Affinity, affinity and anti-affinity scheduling
+ Affinity *corev1.Affinity `json:"affinity,omitempty"`
+ // SecurityContext defines privilege and access control settings for a Pod or Container.
+ SecurityContext *corev1.PodSecurityContext `json:"securityContext,omitempty"`
+ // ImagePullSecrets pull an image from a private registry
+ ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
+ // Tolerations taints and tolerations work together to ensure that pods are not scheduled onto inappropriate nodes
+ Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
+ // NodeSelector is the simplest recommended form of node selection constraint.
+ NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+ // PodAnnotations you can use annotations to attach arbitrary non-identifying metadata to objects.
+ PodAnnotations map[string]string `json:"podAnnotations,omitempty"`
+ // PriorityClassName defines priority class's name
+ PriorityClassName string `json:"priorityClassName,omitempty"`
Review comment:
It seems null checks are required for them, would define them as pointer be better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] AdheipSingh commented on a change in pull request #61: [ISSUEE 60] Add features to the broker and nameserver
Posted by GitBox <gi...@apache.org>.
AdheipSingh commented on a change in pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61#discussion_r523799429
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -401,8 +401,15 @@ func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: ls,
+ Annotations: broker.Spec.PodAnnotations,
},
Spec: corev1.PodSpec{
+ Affinity: broker.Spec.Affinity,
+ SecurityContext: broker.Spec.SecurityContext,
+ ImagePullSecrets: broker.Spec.ImagePullSecrets,
+ Tolerations: broker.Spec.Tolerations,
+ NodeSelector: broker.Spec.NodeSelector,
+ PriorityClassName: broker.Spec.PriorityClassName,
Review comment:
You need to write functions for each of them and handle nil value scenarios, by directly adding this you are forcing the user at all times to specify these values, the operator will crash if you don't pass in tolerations.
@liuruiyiyang
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] AdheipSingh commented on a change in pull request #61: [ISSUEE 60] Add features to the broker and nameserver
Posted by GitBox <gi...@apache.org>.
AdheipSingh commented on a change in pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61#discussion_r523799429
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -401,8 +401,15 @@ func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: ls,
+ Annotations: broker.Spec.PodAnnotations,
},
Spec: corev1.PodSpec{
+ Affinity: broker.Spec.Affinity,
+ SecurityContext: broker.Spec.SecurityContext,
+ ImagePullSecrets: broker.Spec.ImagePullSecrets,
+ Tolerations: broker.Spec.Tolerations,
+ NodeSelector: broker.Spec.NodeSelector,
+ PriorityClassName: broker.Spec.PriorityClassName,
Review comment:
You need to write functions for each of them and handle nil value scenarios, by directly adding this you are forcing the user at all times to specify these values, the operator will crash if you don't pass in tolerations.
##########
File path: pkg/controller/nameservice/nameservice_controller.go
##########
@@ -316,10 +316,17 @@ func (r *ReconcileNameService) statefulSetForNameService(nameService *rocketmqv1
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: ls,
+ Annotations: nameService.Spec.PodAnnotations,
},
Spec: corev1.PodSpec{
HostNetwork: nameService.Spec.HostNetwork,
DNSPolicy: nameService.Spec.DNSPolicy,
+ Affinity: nameService.Spec.Affinity,
+ SecurityContext: nameService.Spec.SecurityContext,
+ ImagePullSecrets: nameService.Spec.ImagePullSecrets,
+ Tolerations: nameService.Spec.Tolerations,
+ NodeSelector: nameService.Spec.NodeSelector,
+ PriorityClassName: nameService.Spec.PriorityClassName,
Review comment:
same here, we need to write functions to get values to handle nil/empty scenarios
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] AdheipSingh commented on a change in pull request #61: [ISSUEE 60] Add features to the broker and nameserver
Posted by GitBox <gi...@apache.org>.
AdheipSingh commented on a change in pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61#discussion_r523799429
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -401,8 +401,15 @@ func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: ls,
+ Annotations: broker.Spec.PodAnnotations,
},
Spec: corev1.PodSpec{
+ Affinity: broker.Spec.Affinity,
+ SecurityContext: broker.Spec.SecurityContext,
+ ImagePullSecrets: broker.Spec.ImagePullSecrets,
+ Tolerations: broker.Spec.Tolerations,
+ NodeSelector: broker.Spec.NodeSelector,
+ PriorityClassName: broker.Spec.PriorityClassName,
Review comment:
You need to write functions for each of them and handle nil value scenarios, by directly adding this you are forcing the user at all times to specify these values, the operator will crash if you don't pass in tolerations.
@liuruiyiyang since these values will go to both broker/nameserver we can create single functions which can be used both in broker and nameserver
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq-operator] caigy commented on a change in pull request #61: [ISSUEE 60] Add features to the broker and nameserver
Posted by GitBox <gi...@apache.org>.
caigy commented on a change in pull request #61:
URL: https://github.com/apache/rocketmq-operator/pull/61#discussion_r834260789
##########
File path: pkg/controller/broker/broker_controller.go
##########
@@ -401,8 +401,15 @@ func (r *ReconcileBroker) getBrokerStatefulSet(broker *rocketmqv1alpha1.Broker,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: ls,
+ Annotations: broker.Spec.PodAnnotations,
},
Spec: corev1.PodSpec{
+ Affinity: broker.Spec.Affinity,
+ SecurityContext: broker.Spec.SecurityContext,
+ ImagePullSecrets: broker.Spec.ImagePullSecrets,
+ Tolerations: broker.Spec.Tolerations,
+ NodeSelector: broker.Spec.NodeSelector,
+ PriorityClassName: broker.Spec.PriorityClassName,
Review comment:
+1 This pr added features that help enrich operator, hope that corresponding null check would be supplemented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org