Skip to content

[ISSUE #54 #50] Operator multiple mq cluster && start sequence #56

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.

Sign up for GitHub

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

Open
linjiemiao wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[ISSUE #54 #50] Operator multiple mq cluster && start sequence #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4cf865
1. ensure rocketmq-operator can operator more than one rocketmq cluster.
linjiemiao Sep 21, 2020
93a9383
1. fixed bug, if nameserver.spec.size is 0, need requeue
linjiemiao Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ spec:
size: 1
# nameServers is the [ip:port] list of name service
nameServers: ""
# rocketMQName is the rocketmq name, must equal to nameservice.spec.rocketMQName and and topictransfer.spec.rocketMQName
rocketMQName: "rocketmq_name"
# replicaPerGroup is the number of each broker cluster
replicaPerGroup: 1
# brokerImage is the customized docker image repo of the RocketMQ broker
Expand Down Expand Up @@ -245,6 +247,8 @@ spec:
nameServiceImage: apacherocketmq/rocketmq-nameserver:4.5.0-alpine-operator-0.3.0
# imagePullPolicy is the image pull policy
imagePullPolicy: Always
# rocketMQName is the rocketmq name, must equal to broker.spec.rocketMQName and and topictransfer.spec.rocketMQName
rocketMQName: "rocketmq_name"
# hostNetwork can be true or false
hostNetwork: true
# Set DNS policy for the pod.
Expand Down Expand Up @@ -440,6 +444,8 @@ spec:
sourceCluster: broker-0
# targetCluster defines the target cluster
targetCluster: broker-1
# rocketMQName is the rocketmq name, must equal to nameservice.spec.rocketMQName and and broker.spec.rocketMQName
rocketMQName: "rocketmq_name"
```

Then apply the ```TopicTransfer``` resource:
Expand Down
8 changes: 0 additions & 8 deletions cmd/manager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/apache/rocketmq-operator/pkg/apis"

@RockteMQ-AI RockteMQ-AI Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"github.com/apache/rocketmq-operator/pkg/controller"

"github.com/operator-framework/operator-sdk/pkg/k8sutil"
"github.com/operator-framework/operator-sdk/pkg/leader"
"github.com/operator-framework/operator-sdk/pkg/log/zap"
"github.com/operator-framework/operator-sdk/pkg/metrics"
Expand Down Expand Up @@ -76,12 +75,6 @@ func main() {

printVersion()

namespace, err := k8sutil.GetWatchNamespace()
if err != nil {
log.Error(err, "Failed to get watch namespace")
os.Exit(1)
}

// Get a config to talk to the apiserver
cfg, err := config.GetConfig()
if err != nil {
Expand All @@ -100,7 +93,6 @@ func main() {

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing Namespace from manager.Options changes the operator from namespace-scoped to cluster-scoped watching. This is intentional (given the new ClusterRole), but it is a significant behavioral change: the operator will now watch all namespaces, increasing API server load and requiring the new ClusterRole/ClusterRoleBinding to be applied. This should be explicitly documented in the PR and migration notes, and the old namespace-scoped Role/RoleBinding files should be deprecated or removed to avoid confusion.

// Create a new Cmd to provide shared dependencies and start components

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing GetWatchNamespace()/the Namespace option silently ignores the WATCH_NAMESPACE env var that existing operator deployments set. After upgrading, the operator expands from namespaced to cluster-wide watching with no notice; deployments that deliberately scoped it for least privilege will break at startup because the caches now need list/watch across all namespaces (and old namespaced Role manifests no longer suffice). Keep honoring WATCH_NAMESPACE, or at minimum log a loud warning when it is set.

mgr, err := manager.New(cfg, manager.Options{
Namespace: namespace,
MetricsBindAddress: fmt.Sprintf("%s:%d", metricsHost, metricsPort),
})
if err != nil {
Expand Down
72 changes: 72 additions & 0 deletions deploy/cluster_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
creationTimestamp: null
name: rocketmq-operator
rules:
- apiGroups:
- ""
resources:
- pods
- services
- endpoints
- persistentvolumeclaims
- events
- configmaps
- secrets
- pods/exec
verbs:
- '*'
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- apiGroups:
- apps
resources:
- deployments
- daemonsets
- replicasets
- statefulsets
verbs:
- '*'
- apiGroups:
- monitoring.coreos.com
resources:
- servicemonitors
verbs:
- get
- create
- apiGroups:
- apps
resourceNames:
- rocketmq-operator
resources:
- deployments/finalizers
verbs:

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rocketmq.apache.org ClusterRole rule lists both '*' (wildcard) and explicit resources brokers, pods/exec, topictransfers. The wildcard already covers everything, making the explicit entries redundant. More importantly, pods/exec is not a valid subresource of the rocketmq.apache.org API group — it belongs to the core "" group. This is a copy-paste error that should be removed to avoid confusion and potential future misinterpretation.

- update
- apiGroups:
- rocketmq.apache.org
resources:

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pods/exec is listed under the rocketmq.apache.org API group, which is incorrect — pods/exec belongs to the core ("") API group and is already listed there (line 30). The duplicate under the custom API group is meaningless and should be removed.

- '*'
- brokers
- pods/exec
- topictransfers
verbs:
- '*'
27 changes: 27 additions & 0 deletions deploy/clusterrole_binding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.

kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: rocketmq-operator
subjects:
- kind: ServiceAccount

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ClusterRoleBinding subject hardcodes namespace: default. If the operator is installed in any other namespace, its ServiceAccount never receives the ClusterRole and — since the manager now watches all namespaces — informer cache sync fails and the operator cannot start. The binding should be parameterized by the install namespace, and deploy/operator.yaml needs to be updated to reference this cluster-scoped RBAC (the diff does not touch it, so existing deployments still ship the namespaced Role and WATCH_NAMESPACE).

name: rocketmq-operator

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ClusterRoleBinding hardcodes namespace: default for the ServiceAccount subject. If the operator is deployed in a non-default namespace, this binding will be incorrect and the operator will lack permissions. This should be templated or documented clearly, since ClusterRole + hardcoded namespace is a common misconfiguration in multi-tenant deployments.

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ServiceAccount namespace is hardcoded to default. If the operator is deployed to any other namespace, the binding will reference a non-existent ServiceAccount and RBAC will deny all API calls. Make this configurable (e.g., via Kustomize or Helm templating).

namespace: default
roleRef:
kind: ClusterRole
name: rocketmq-operator
apiGroup: rbac.authorization.k8s.io
4 changes: 4 additions & 0 deletions deploy/crds/rocketmq_v1alpha1_broker_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ spec:
items:
type: object
type: array
rocketMQName:
description: The name of rocketmq, and the broker and nameserver in the same cluster must be filled with the same name
type: string
required:
- size
- replicaPerGroup
Expand All @@ -93,6 +96,7 @@ spec:
- volumes

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rocketMQName is now a required field in the CRD. This is a breaking change for any existing Broker CR that does not have this field set. Existing clusters that upgrade to this operator version will have their Broker CRs fail validation. A defaulting webhook or a non-required field with a documented migration path is needed for safe upgrades.

- volumeClaimTemplates
- scalePodName
- rocketMQName

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rocketMQName is added to the required list. Any existing Broker CR that does not include this field will fail validation after CRD upgrade, breaking backward compatibility. Either provide a default value via an admission webhook / mutating webhook, remove it from required, or use a conversion/migration step for existing resources.

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rocketMQName is added to required (also in the nameservice and topictransfer CRDs). Existing Broker/NameService/TopicTransfer CRs created by older operator versions lack this field, so after the CRD update any subsequent write to those objects (kubectl apply/edit, and potentially the operator's own status updates) fails OpenAPI validation — there is no migration path documented. Also note that CRs patched with an empty value silently collide on the key "-". Consider making the field optional with a documented default, or document a mandatory pre-upgrade migration for all existing CRs.

type: object
status:
properties:
Expand Down
4 changes: 4 additions & 0 deletions deploy/crds/rocketmq_v1alpha1_nameservice_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ spec:
items:
type: object
type: array
rocketMQName:
description: The name of rocketmq, and the broker and nameserver in the same cluster must be filled with the same name
type: string
required:
- size
- nameServiceImage
Expand All @@ -73,6 +76,7 @@ spec:
- storageMode

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same breaking change as the Broker CRD: making rocketMQName required on NameService CRD will invalidate all existing NameService CRs on upgrade. Needs a migration strategy.

- hostPath
- volumeClaimTemplates
- rocketMQName

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same backward-compatibility break as the Broker CRD: rocketMQName is added to required, which will reject any existing NameService CR that lacks the field upon upgrade.

type: object
status:
properties:
Expand Down
5 changes: 5 additions & 0 deletions deploy/crds/rocketmq_v1alpha1_topictransfer_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ spec:
topic:
description: Topic name
type: string
rocketMQName:
description: The name of rocketmq, and the broker and nameserver in the same cluster must be filled with the same name
type: string
required:
- rocketMQName
type: object
status:
type: object
Expand Down
2 changes: 2 additions & 0 deletions example/rocketmq_v1alpha1_broker_cr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ spec:
size: 1
# nameServers is the [ip:port] list of name service
nameServers: ""
# rocketMQName is the rocketmq name, must equal to nameserver.spec.rocketMQName and topictransfer.spec.rocketMQName

@liuruiyiyang liuruiyiyang Sep 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can not ensure users set insistent name correctly, is there a better way to do this?

@linjiemiao linjiemiao Sep 24, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because we don't have a rocketmq resource as the parent resource of the broker and nameserver. Without a common parent controller, we can only specify the connection of the child resource in the spec. So I hope to add a rocketmq api.

@liuruiyiyang liuruiyiyang Sep 25, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case should we add rocketmq higher level api before this PR?

@linjiemiao linjiemiao Sep 25, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so

rocketMQName: "rocketmq_name"
# replicaPerGroup is the number of each broker cluster
replicaPerGroup: 1
# brokerImage is the customized docker image repo of the RocketMQ broker
Expand Down
2 changes: 2 additions & 0 deletions example/rocketmq_v1alpha1_nameservice_cr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ spec:
nameServiceImage: apacherocketmq/rocketmq-nameserver:4.5.0-alpine-operator-0.3.0
# imagePullPolicy is the image pull policy
imagePullPolicy: Always
# rocketMQName is the rocketmq name, must equal to broker.spec.rocketMQName and topictransfer.spec.rocketMQName
rocketMQName: "rocketmq_name"
# hostNetwork can be true or false
hostNetwork: true
# Set DNS policy for the pod.
Expand Down
4 changes: 4 additions & 0 deletions example/rocketmq_v1alpha1_rocketmq_cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ spec:
size: 1
# nameServers is the [ip:port] list of name service
nameServers: ""
# rocketMQName is the rocketmq name, must equal to nameserver.spec.rocketMQName
rocketMQName: "rocketmq_name"
# replicaPerGroup is the number of each broker cluster
replicaPerGroup: 1
# brokerImage is the customized docker image repo of the RocketMQ broker
Expand Down Expand Up @@ -99,6 +101,8 @@ spec:
nameServiceImage: apacherocketmq/rocketmq-nameserver:4.5.0-alpine-operator-0.3.0
# imagePullPolicy is the image pull policy
imagePullPolicy: Always
# rocketMQName is the rocketmq name, must equal to broker.spec.rocketMQName and and topictransfer.spec.rocketMQName
rocketMQName: "rocketmq_name"
# hostNetwork can be true or false
hostNetwork: true
# Set DNS policy for the pod.
Expand Down
2 changes: 2 additions & 0 deletions example/rocketmq_v1alpha1_topictransfer_cr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ spec:
sourceCluster: broker-0
# targetCluster defines the target cluster
targetCluster: broker-1
# rocketMQName is the rocketmq name, must equal to nameserver.spec.rocketMQName and broker.spec.rocketMQName
rocketMQName: "rocketmq_name"
2 changes: 2 additions & 0 deletions pkg/apis/rocketmq/v1alpha1/broker_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ type BrokerSpec struct {
VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`
// The name of pod where the metadata from
ScalePodName string `json:"scalePodName"`
// RocketMQ Name, the broker and nameserver in the same cluster must be filled with the same name
RocketMQName string `json:"rocketMQName"`
}

// BrokerStatus defines the observed state of Broker
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/rocketmq/v1alpha1/nameservice_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ type NameServiceSpec struct {
HostPath string `json:"hostPath"`
// VolumeClaimTemplates defines the StorageClass
VolumeClaimTemplates []corev1.PersistentVolumeClaim `json:"volumeClaimTemplates"`
// RocketMQ Name, the broker and nameserver in the same cluster must be filled with the same name
RocketMQName string `json:"rocketMQName"`
}

// NameServiceStatus defines the observed state of NameService
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/rocketmq/v1alpha1/topictransfer_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type TopicTransferSpec struct {
SourceCluster string `json:"sourceCluster,omitempty"`
// The cluster where the topic will be transferred to
TargetCluster string `json:"targetCluster,omitempty"`
// // RocketMQ Name, the broker and nameserver in the same cluster must be filled with the same name

@RockteMQ-AI RockteMQ-AI Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double comment marker on line 40: // // RocketMQ Name, ... — there is a tab and an extra // before the actual comment text. This is a copy-paste artifact and should be // RocketMQ Name, ....

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Malformed comment: // // RocketMQ Name... contains a stray tab and duplicate // prefix, likely a copy-paste artifact. Clean up to // RocketMQ Name, the broker and nameserver in the same cluster must be filled with the same name.

@RockteMQ-AI RockteMQ-AI Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is doubled (// \t// RocketMQ Name...) — leftover from editing. Similar "and and" duplications appear in the new README and example YAML comments; worth a quick copy pass.

RocketMQName string `json:"rocketMQName"`
}

// TopicTransferStatus defines the observed state of TopicTransfer
Expand Down
Loading