[ISSUE #176] Support manage multi cluster #183
Conversation
|
@caigy PTAL |
| err := mgr.GetFieldIndexer().IndexField(context.TODO(), &rocketmqv1alpha1.NameService{}, rocketmqv1alpha1.NameServiceRocketMqNameIndexKey, | ||
| func(rawObj client.Object) []string { | ||
| n, ok := rawObj.(*rocketmqv1alpha1.NameService) | ||
| if !ok { | ||
| return nil | ||
| } | ||
| return []string{n.Spec.RocketMqName + "-" + n.Namespace} | ||
| }, | ||
| ) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
What's the usage of this block? You'd better show more about your design.
| // get cluster of output | ||
| clusterName := "" | ||
| for _, line := range strings.Split(string(clusterListOutput), "\n") { | ||
| if strings.HasPrefix(line, "#Cluster Name") { | ||
| continue | ||
| } |
There was a problem hiding this comment.
It's not stable to get cluster name by analyzing the output of a command.
| // #Cluster Name #Broker Name #BID #Addr #Version #InTPS(LOAD) #OutTPS(LOAD) #PCWait(ms) #Hour #SPACE | ||
| // broker broker-0 0 192.168.180.40:10911 V4_5_0 0.00(0,0ms) 0.00(0,0ms) 0 471030.34 -1.0000 | ||
| // broker broker-0 1 192.168.137.89:10911 V4_5_0 0.00(0,0ms) 0.00(0,0ms) 0 471030.34 0.2673 | ||
| clusterListCmd := exec.Command("sh", cons.AdminToolDir, cons.ClusterList, "-n", oldNameServerListStr) |
There was a problem hiding this comment.
As isNameServersStrUpdated is true, why using oldNameServerListStr as the addresses of name servers?
| command := mqAdmin + " " + subCmd + " -c " + clusterName + " -k " + key + " -n " + oldNameServerListStr + " -v " + newNameServerListStr | ||
| cmd := exec.Command("sh", mqAdmin, subCmd, "-c", clusterName, "-k", key, "-n", oldNameServerListStr, "-v", newNameServerListStr) |
There was a problem hiding this comment.
Only brokers registering successfully to name servers can receive the command, others will not be updated.
| runningNameServerNum := getRunningNameServersNum(podList.Items) | ||
| if runningNameServerNum == instance.Spec.Size { | ||
| share.IsNameServersStrInitialized = true | ||
| share.NameServersStr = nameServerListStr // reassign if operator restarts | ||
| } |
There was a problem hiding this comment.
Can this block be safely removed?
OK, i would add this. |
What is the purpose of the change
In order to manage multi cluster by one operator.
task1 && task2 of #176
Brief changelog
XX
Verifying this change
tested against old cluster()
tested against creating two cluster in one namespace
yaml1(set rocketmq cluster name to empty string): https://gist.github.com/drivebyer/8795a96966be4fd6ebf395cc347a159d
yaml2(set rocketmq cluster name to test-rocketmq): https://gist.github.com/drivebyer/338a0cba168249dd3c02ce0f33f775d8
Please go through this checklist to help us incorporate your contribution quickly and easily.
Notice:
It would be helpful if you could finish the following checklist (the last one is not necessary) before request the community to review your PR.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.make docker-buildto build docker image for operator, try your changes from Pod inside your Kubernetes cluster, not just locally. Also provide screenshots to show that the RocketMQ cluster is healthy after the changes.make manifeststo make sure the CRD files are updated.