[ISSUE 57] add rocketmq api by linjiemiao · Pull Request #63 · apache/rocketmq-operator

linjiemiao · 2020-09-27T09:41:07Z

add rocketmq api

liuruiyiyang · 2020-09-28T07:18:25Z

@linjiemiao ping, could you please help to resolve the conflict.

linjiemiao · 2020-09-28T07:51:06Z

@linjiemiao ping, could you please help to resolve the conflict.

Okay, I will cherry-pick your latest commit, and then I resolve the conflict.

* feat(console): add CRD for RocketMQ Console * chore(*): add ASF header Co-authored-by: liuruiyiyang <liurui@apache.org>

drivebyer · 2023-09-04T02:40:40Z

still need this

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

⚠️ 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-57-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 10 file(s) with 1043 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] README.md:1 — Large diff (1043 lines). Consider breaking into smaller, focused PRs for easier review. (line outside diff)

RockteMQ-AI · 2026-08-05T14:56:50Z

@@ -33,7 +33,7 @@ type BrokerSpec struct {
 	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html


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: 1043 lines
Author: linjiemiao (NONE)

Automated review by RockteMQ-AI

RockteMQ-AI

Summary

Review of PR #63: [ISSUE 57] add rocketmq api

Findings: 13 issue(s) identified (3 critical).
CLA: unknown

Please address the inline comments above.

Automated review by github-manager-bot

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

+				brokerSts.Namespace, "broker.Name", brokerSts.Name)
+		}
+		return reconcile.Result{Requeue: true}, nil
+	} else if err != nil {


Broker creation error is logged but not returned. The function returns reconcile.Result{Requeue: true}, nil which discards the error, defeating the controller-runtime's exponential backoff retry mechanism. The error should be returned so the framework can properly retry with backoff.

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

+	} else {
+		// Resource broker will change; Only ReplicaPerGroup Size ImagePullPolicy BrokerImage can update
+		if !reflect.DeepEqual(brokerSts.Spec.ReplicaPerGroup, brokerFound.Spec.ReplicaPerGroup) ||
+			!reflect.DeepEqual(brokerSts.Spec.Size, brokerFound.Spec.Size) ||


When broker Get fails with a non-NotFound error, the error is logged but not returned. The code falls through to the status update section where brokerFound is still a zero-value &Broker{} (never populated), causing the status to be set to empty strings and potentially overwriting valid status.

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

+		}
+	}
+	if instance.Spec.Console.ConsoleDeployment.Spec.Replicas != nil {
+		consoleFound := &rocketmqv1alpha1.Console{}


Console update calls r.client.Update(context.TODO(), consoleDep) on the desired-state object (consoleDep) instead of the existing cluster object (consoleFound). This will fail because consoleDep lacks the server-assigned ResourceVersion, and even if it succeeded it would overwrite the existing resource with a stale version.

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

+		// Resource NameService will change; Only size nameServiceImage imagePullPolicy can update
+		if !reflect.DeepEqual(nameServiceSts.Spec.Size, nameServiceFound.Spec.Size) ||
+			!reflect.DeepEqual(nameServiceSts.Spec.NameServiceImage, nameServiceFound.Spec.NameServiceImage) ||
+			!reflect.DeepEqual(nameServiceSts.Spec.ImagePullPolicy, nameServiceFound.Spec.ImagePullPolicy) {


No readiness gate between NameService and Broker creation: the controller creates the Broker immediately after the NameService resource exists, without waiting for NameService pods to be Running/Ready. The README states 'the name server cluster will be created first, after all name server cluster is in running state, the operator will create the broker cluster' but the code does not enforce this ordering, which can cause brokers to fail on startup.

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

 	env := corev1.EnvVar{
 		Name:  "JAVA_OPTS",
 		Value: fmt.Sprintf("-Drocketmq.namesrv.addr=%s -Dcom.rocketmq.sendMessageWithVIPChannel=false", share.NameServersStr),
 	}


The label mutation selectorLabels["console-cr"] = cr.Name and labels["console-cr"] = cr.Name modifies the maps from cr.Spec in place. Since maps are reference types in Go, this permanently mutates the CR's spec in memory, which can cause spurious diffs on subsequent reconcile passes and incorrect Update calls to the API server.

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

 	Size int `json:"size"`
 	// NameServers defines the name service list e.g. 192.168.1.1:9876;192.168.1.2:9876
-	NameServers string `json:"nameServers,omitempty"`
+	NameServers string `json:"nameServers"`


Removing omitempty from the nameServers JSON tag (json:"nameServers,omitempty" -> json:"nameServers") is a breaking API change. Existing Broker resources serialized without nameServers will now fail deserialization or validation. This breaks backward compatibility for users of the standalone Broker CRD.

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

+        spec:
+          properties:
+            broker:
+              description: Broker defines rocketmq broker spec info


The CRD validation schema defines broker, nameService, and console as type: object with no sub-properties. This provides zero validation for nested fields, meaning any malformed spec will be accepted by the API server. At minimum, required sub-fields should be enumerated, or the full nested schemas from the Broker/NameService/Console CRDs should be inlined.

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

+		err = r.client.Get(context.TODO(), types.NamespacedName{Name: consoleDep.Name, Namespace: consoleDep.Namespace}, consoleFound)
+		if err != nil && errors.IsNotFound(err) {
+			err = r.client.Create(context.TODO(), consoleDep)
+			if err != nil {


Console creation error is logged but not returned (reqLogger.Error(err, ...) without return reconcile.Result{}, err). This means console creation failures are silently swallowed and won't trigger proper retry with backoff.

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

+// Note:
+// The Controller will requeue the Request to be processed again if the returned error is non-nil or
+// Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
+func (r *ReconcileRocketmq) Reconcile(request reconcile.Request) (reconcile.Result, error) {


No unit or integration tests are included for the new Rocketmq controller, which orchestrates three sub-resources (NameService, Broker, Console) with non-trivial reconciliation logic including conditional creation, selective field updates, and status management. This is a significant gap in test coverage for a new controller.

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

 spec:
  type: NodePort
  selector:
+    name_service_cr: ${rocketmq-name}-name-service


The selector name_service_cr: ${rocketmq-name}-name-service uses a shell variable placeholder ${rocketmq-name} that is not substituted by kubectl. Users must manually replace this value, but this is not documented and will silently result in a Service with no matching pods if applied as-is.

add rocketmq api

6b7520f

linjiemiao force-pushed the issue-57-dev branch from 4b07bd0 to 6b7520f Compare September 28, 2020 03:13

liuruiyiyang and others added 3 commits September 28, 2020 16:07

[ISSUE apache#39] Add RocketMQ Console CRD and Controller (apache#53)

5b9c061

* feat(console): add CRD for RocketMQ Console * chore(*): add ASF header Co-authored-by: liuruiyiyang <liurui@apache.org>

1. pick console api

7ee06fb

Merge branch 'master' into issue-57-dev

1f91011

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

		@@ -33,7 +33,7 @@ type BrokerSpec struct {
		// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html

Uh oh!

Conversation

linjiemiao commented Sep 27, 2020

Uh oh!

liuruiyiyang commented Sep 28, 2020

Uh oh!

linjiemiao commented Sep 28, 2020

Uh oh!

drivebyer commented Sep 4, 2023

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