【ISSUE #133】RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pattern by shendongsd · Pull Request #134 · apache/rocketmq-operator

shendongsd · 2022-10-18T02:15:02Z

What is the purpose of the change

ISSUE #133RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pattern
And I will add local pattern later

…tern

…q-operator into feature_proxy

…tern

caigy

It seems that ClusterRole for proxy is missing.

caigy · 2022-10-18T03:32:56Z

+	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
+	// Important: Run "operator-sdk generate k8s" to regenerate code after modifying this file
+	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
+	ProxyStatefulSet v1.StatefulSet `json:"proxyStatefulSet"`


Proxy is stateless, so is deployment more appropriate than statefulset?

Furthermore, it would be more elegant that users should only provide configs which they care about, instead of providing the whole statefulset or deployment.

…tern

AllenZMC · 2024-05-11T15:08:47Z

How is it going? Can you merge it? @shendongsd

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

⚠️ 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 feature_proxy
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 11 file(s) with 8682 lines of diff. No test changes detected — consider adding test coverage.

Automated review by github-manager-bot

Additional notes (not anchored to a changed line)

[INFO] Makefile:1 — Large diff (8682 lines). Consider breaking into smaller, focused PRs for easier review. (line outside diff)

RockteMQ-AI · 2026-08-05T14:55:07Z

@@ -23,6 +23,7 @@ import (
 	"github.com/apache/rocketmq-operator/pkg/controller/broker"


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

RockteMQ-AI

Summary

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

Diff size: 8682 lines
Author: shendongsd (NONE)

Automated review by RockteMQ-AI

RockteMQ-AI

Summary

Review of PR #134: 【ISSUE #133】RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pattern

Findings: 14 issue(s) identified (8 critical).
CLA: unknown

Please address the inline comments above.

Automated review by github-manager-bot

RockteMQ-AI · 2026-08-23T19:57:18Z

 install: manifests kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config.
 	kubectl create -f deploy/crds/rocketmq.apache.org_brokers.yaml
 	kubectl create -f deploy/crds/rocketmq.apache.org_nameservices.yaml
+	kubectl create -f deploy/crds/rocketmq.apache.proxys.yaml


The install target references deploy/crds/rocketmq.apache.proxys.yaml, which is missing .org_. The actual generated file is rocketmq.apache.org_proxys.yaml, so the CRD will not be installed and the operator cannot reconcile Proxy resources.

RockteMQ-AI · 2026-08-23T19:57:18Z

 uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion.
 	kubectl delete --ignore-not-found=$(ignore-not-found) -f deploy/crds/rocketmq.apache.org_brokers.yaml
 	kubectl delete --ignore-not-found=$(ignore-not-found) -f deploy/crds/rocketmq.apache.org_nameservices.yaml
+    kubectl delete --ignore-not-found=$(ignore-not-found) -f deploy/crds/rocketmq.apache.org_proxys.yaml


The added uninstall command is indented with spaces instead of a tab. Make requires recipe lines to begin with a tab, so make uninstall will fail with a missing separator error.

RockteMQ-AI · 2026-08-23T19:57:18Z

+
+	// TODO(user): Modify this to be the types you create that are owned by the primary resource
+	// Watch for changes to secondary resource Pods and requeue the owner Proxy
+	err = c.Watch(&source.Kind{Type: &corev1.Pod{}}, &handler.EnqueueRequestForOwner{


The controller watches corev1.Pod as a secondary resource, but there is no corresponding RBAC marker granting get/list/watch on pods. The controller will fail to start the watch with a Forbidden error unless the role is manually fixed.

RockteMQ-AI · 2026-08-23T19:57:18Z

+//+kubebuilder:rbac:groups=rocketmq.apache.org,resources=proxys,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=rocketmq.apache.org,resources=proxys/status,verbs=get;update;patch
+//+kubebuilder:rbac:groups=rocketmq.apache.org,resources=proxys/finalizers,verbs=update
+//+kubebuilder:rbac:groups="apps",resources=Deployments,verbs=get;list;watch;create;update;patch;delete


The RBAC marker uses resources=Deployments. Kubernetes RBAC resource names should be lowercase plural (deployments); Deployments is non-idiomatic and may not be accepted by controller-gen or the API server.

RockteMQ-AI · 2026-08-23T19:57:18Z

+
+	// Fetch the Proxy instance
+	instance := &rocketmqv1alpha1.Proxy{}
+	err := r.client.Get(context.TODO(), request.NamespacedName, instance)


Reconcile receives a ctx context.Context parameter but calls r.client.Get(context.TODO(), ...). It should use the supplied ctx so cancellation and deadlines are respected.

RockteMQ-AI · 2026-08-23T19:57:18Z

+					Affinity:           cr.Spec.ProxyDeployment.Spec.Template.Spec.Affinity,
+					ImagePullSecrets:   cr.Spec.ProxyDeployment.Spec.Template.Spec.ImagePullSecrets,
+					Containers: []corev1.Container{{
+						Resources:       cr.Spec.ProxyDeployment.Spec.Template.Spec.Containers[0].Resources,


newDeploymentForCR accesses Spec.Template.Spec.Containers[0] without verifying that at least one container is provided. Omitting containers causes an index-out-of-range panic.

RockteMQ-AI · 2026-08-23T19:57:18Z

+
+func getContainerSecurityContext(proxy *rocketmqv1alpha1.Proxy) *corev1.SecurityContext {
+	var securityContext = corev1.SecurityContext{}
+	if proxy.Spec.ProxyDeployment.Spec.Template.Spec.Containers[0].SecurityContext != nil {


getContainerSecurityContext also dereferences Containers[0] without a length check, leading to a panic when the container list is empty.

RockteMQ-AI · 2026-08-23T19:57:18Z

+	}
+
+	// Support proxy Deployment scaling
+	if !reflect.DeepEqual(instance.Spec.ProxyDeployment.Spec.Replicas, found.Spec.Replicas) {


After the Deployment exists, the reconcile loop only updates Replicas. Changes to image, environment variables, volumes, labels, affinity, or other template fields are ignored, so updates to the Proxy CR will not be fully applied.

RockteMQ-AI · 2026-08-23T19:57:18Z

+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+if [ $PROXY_MODE == "LOCAL" ]; then


When PROXY_MODE is LOCAL, the script runs ./mqproxy -bc ... -pm LOCAL and then falls through to unconditionally run ./mqproxy -pc ... -pm ... on line 20. The second invocation lacks the broker config and will either fail or create an unwanted cluster-mode process. An else branch is needed.

RockteMQ-AI · 2026-08-23T19:57:18Z

@@ -0,0 +1,236 @@
+/*


No unit tests or integration tests are added for the new Proxy type, controller, or Deployment reconciliation logic. The PR introduces a new CRD and controller that should have test coverage.

shendong added 4 commits October 10, 2022 09:25

support mounting a configMap to rocketmq-dashboard

f00a015

RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pat…

2277105

…tern

Merge branch 'feature_proxy' of https://github.com/shendongsd/rocketm…

273f4d0

…q-operator into feature_proxy

RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pat…

944c669

…tern

caigy requested changes Oct 18, 2022

View reviewed changes

caigy added the enhancement New feature or request label Oct 18, 2022

shendong added 2 commits October 24, 2022 14:56

RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pat…

8887e43

…tern

RocketMQ-Operator support the rocketmq-5.0 proxy node for cluster pat…

f6e5d6b

…tern

caigy mentioned this pull request Dec 5, 2022

How to quickly deploy RocketMQ 5.0 on k8s? apache/rocketmq#5612

Closed

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

		@@ -23,6 +23,7 @@ import (
		"github.com/apache/rocketmq-operator/pkg/controller/broker"

Uh oh!

Conversation

shendongsd commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Uh oh!

caigy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AllenZMC commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RockteMQ-AI commented Jul 30, 2026

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Additional notes (not anchored to a changed line)

Uh oh!

Choose a reason for hiding this comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shendongsd commented Oct 18, 2022 •

edited

Loading

AllenZMC commented May 11, 2024 •

edited

Loading