-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ISSUEE 60] Add features to the broker and nameserver #61
base: master
Are you sure you want to change the base?
Conversation
linjiemiao
commented
Sep 25, 2020
- add affinity / securityContext / imagePullSecrets / tolerations / nodeSelector / podAnnotations / priorityClassName
nodeSelector / podAnnotations / priorityClassName
Affinity: broker.Spec.Affinity, | ||
SecurityContext: broker.Spec.SecurityContext, | ||
ImagePullSecrets: broker.Spec.ImagePullSecrets, | ||
Tolerations: broker.Spec.Tolerations, | ||
NodeSelector: broker.Spec.NodeSelector, | ||
PriorityClassName: broker.Spec.PriorityClassName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 This pr added features that help enrich operator, hope that corresponding null check would be supplemented.
Affinity: nameService.Spec.Affinity, | ||
SecurityContext: nameService.Spec.SecurityContext, | ||
ImagePullSecrets: nameService.Spec.ImagePullSecrets, | ||
Tolerations: nameService.Spec.Tolerations, | ||
NodeSelector: nameService.Spec.NodeSelector, | ||
PriorityClassName: nameService.Spec.PriorityClassName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, we need to write functions to get values to handle nil/empty scenarios
Affinity: broker.Spec.Affinity, | ||
SecurityContext: broker.Spec.SecurityContext, | ||
ImagePullSecrets: broker.Spec.ImagePullSecrets, | ||
Tolerations: broker.Spec.Tolerations, | ||
NodeSelector: broker.Spec.NodeSelector, | ||
PriorityClassName: broker.Spec.PriorityClassName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 This pr added features that help enrich operator, hope that corresponding null check would be supplemented.
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"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems null checks are required for them, would define them as pointer be better?
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"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems null checks are required for them, would define them as pointer be better?