[ISSUEE 60] Add features to the broker and nameserver by linjiemiao · Pull Request #61 · apache/rocketmq-operator

linjiemiao · 2020-09-25T08:25:27Z

add affinity / securityContext / imagePullSecrets / tolerations / nodeSelector / podAnnotations / priorityClassName

nodeSelector / podAnnotations / priorityClassName

AdheipSingh · 2020-11-15T19:08:49Z

+					Affinity: broker.Spec.Affinity,
+					SecurityContext: broker.Spec.SecurityContext,
+					ImagePullSecrets: broker.Spec.ImagePullSecrets,
+					Tolerations: broker.Spec.Tolerations,
+					NodeSelector: broker.Spec.NodeSelector,
+					PriorityClassName: broker.Spec.PriorityClassName,


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

+1 This pr added features that help enrich operator, hope that corresponding null check would be supplemented.

AdheipSingh · 2020-11-15T19:09:22Z

+					Affinity: nameService.Spec.Affinity,
+					SecurityContext: nameService.Spec.SecurityContext,
+					ImagePullSecrets: nameService.Spec.ImagePullSecrets,
+					Tolerations: nameService.Spec.Tolerations,
+					NodeSelector: nameService.Spec.NodeSelector,
+					PriorityClassName: nameService.Spec.PriorityClassName,


same here, we need to write functions to get values to handle nil/empty scenarios

caigy · 2022-03-24T12:37:47Z

+					Affinity: broker.Spec.Affinity,
+					SecurityContext: broker.Spec.SecurityContext,
+					ImagePullSecrets: broker.Spec.ImagePullSecrets,
+					Tolerations: broker.Spec.Tolerations,
+					NodeSelector: broker.Spec.NodeSelector,
+					PriorityClassName: broker.Spec.PriorityClassName,


+1 This pr added features that help enrich operator, hope that corresponding null check would be supplemented.

caigy · 2022-03-24T12:57:01Z

+	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"`


It seems null checks are required for them, would define them as pointer be better?

caigy · 2022-03-24T12:57:30Z

+	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"`


It seems null checks are required for them, would define them as pointer be better?

RockteMQ-AI · 2026-07-30T10:40:37Z

⚠️ Merge conflict detected

This PR has conflicts with the base branch and cannot be merged. Please rebase or merge the base branch into your branch and resolve the conflicts:

git fetch origin
git checkout issue-60-dev
git rebase origin/main
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved.

Automated notification by github-manager-bot

RockteMQ-AI

Summary

This PR modifies 7 file(s) with 351 lines of diff. No test changes detected — consider adding test coverage.

Automated review by github-manager-bot

RockteMQ-AI · 2026-08-05T14:57:41Z

@@ -56,6 +56,21 @@ type BrokerSpec struct {
 	VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`


No test changes detected alongside source modifications. Consider adding tests to cover the changes.

RockteMQ-AI · 2026-08-20T00:41:55Z

This PR has been open since September 2020 (nearly 6 years) and currently has merge conflicts with the master branch.

Status check: Is this PR still actively being worked on? The feature additions (affinity, tolerations, nodeSelector, etc.) are valuable, but the code needs to be rebased to resolve conflicts.

If this is still relevant, please:

Rebase onto the latest master
Address the changes requested by @caigy
Update the PR description with current status

If this has been abandoned, consider closing it.

Automated review by github-manager-bot

RockteMQ-AI

Summary

PR received and logged for review. This PR requires detailed code review by a maintainer.

Diff size: 351 lines
Author: linjiemiao (NONE)

Automated review by RockteMQ-AI

RockteMQ-AI

Summary

Review of PR #61: [ISSUEE 60] Add features to the broker and nameserver

Findings: 4 issue(s) identified (1 critical).
CLA: unknown

Please address the inline comments above.

Automated review by github-manager-bot

RockteMQ-AI · 2026-08-23T20:02:38Z

 	VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`
 	// The name of pod where the metadata from
 	ScalePodName string `json:"scalePodName"`
+	// Affinity, affinity and anti-affinity scheduling


New pointer/slice/map fields (Affinity, SecurityContext, ImagePullSecrets, Tolerations, NodeSelector, PodAnnotations) were added to BrokerSpec, but the PR does not include updates to zz_generated_deepcopy.go. If the generated DeepCopyInto is not regenerated, these fields are shallow-copied: Affinity/SecurityContext pointers and NodeSelector/PodAnnotations maps will be shared between the informer cache and the object handed to the reconciler. Any mutation corrupts the shared cache and can cause cross-reconcile state corruption. Run the operator-sdk/k8s codegen to regenerate deepcopy for both broker_types.go and nameservice_types.go.

RockteMQ-AI · 2026-08-23T20:02:38Z

 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: ls,
+					Annotations: broker.Spec.PodAnnotations,


The new pod-scheduling fields are wired into getBrokerStatefulSet (Annotations, Affinity, SecurityContext, ImagePullSecrets, Tolerations, NodeSelector, PriorityClassName) and similarly into nameservice_controller.go, but no test changes accompany this PR. There is no coverage verifying that CR-spec values propagate to the rendered StatefulSet PodTemplateSpec, nor that nil/empty values (e.g. unset Affinity, empty PriorityClassName) render safely. Add reconciler tests asserting these fields pass through correctly.

RockteMQ-AI · 2026-08-23T20:02:38Z

+    prometheus.io/scrape: "true"
+  securityContext:
+    allowPrivilegeEscalation: true
+    runAsUser: 0


The NameService securityContext sets allowPrivilegeEscalation: true, runAsUser: 0, runAsGroup: 0. This example is intended as a copyable template, and running the name server as root with privilege escalation enabled is a poor security pattern for an Apache-shipped reference. Consider a non-root runAsUser (the alpine image supports a dedicated UID) and allowPrivilegeEscalation: false, or at minimum document the risk and make it opt-in.

RockteMQ-AI · 2026-08-23T20:02:38Z

              items:
                type: object
              type: array
+            affinity:


The new complex Kubernetes types (affinity, securityContext, tolerations, imagePullSecrets items) are declared as bare type: object/item type: object with no OpenAPI v3 properties. This provides no server-side validation, so malformed scheduling rules are accepted silently and only surface as Pod creation errors later. For a cleaner CRD (and better kubectl explain output), define the full schema via x-kubernetes-preserve-unknown-fields or the concrete property trees. Same applies to rocketmq_v1alpha1_nameservice_crd.yaml.

1. add affinity / securityContext / imagePullSecrets / tolerations /

562fd39

nodeSelector / podAnnotations / priorityClassName

AdheipSingh reviewed Nov 15, 2020

View reviewed changes

caigy requested changes Mar 24, 2022

View reviewed changes

caigy approved these changes Mar 24, 2022

View reviewed changes

caigy requested changes Mar 24, 2022

View reviewed changes

RockteMQ-AI reviewed Aug 5, 2026

View reviewed changes

RockteMQ-AI reviewed Aug 20, 2026

View reviewed changes

RockteMQ-AI suggested changes Aug 23, 2026

View reviewed changes

		@@ -56,6 +56,21 @@ type BrokerSpec struct {
		VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`

Uh oh!

Conversation

linjiemiao commented Sep 25, 2020

Uh oh!

AdheipSingh Nov 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RockteMQ-AI commented Jul 30, 2026

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RockteMQ-AI commented Aug 20, 2026

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AdheipSingh Nov 15, 2020 •

edited

Loading