You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "drivebyer (via GitHub)" <gi...@apache.org> on 2023/10/09 09:24:08 UTC
[PR] [ISSUE #185] Fix pod ready status reflect to cr [rocketmq-operator]
drivebyer opened a new pull request, #186:
URL: https://github.com/apache/rocketmq-operator/pull/186
## What is the purpose of the change
https://github.com/apache/rocketmq-operator/issues/185
## Brief changelog
Fix pod ready status reflect to cr.
## Verifying this change
Apply:
```
apiVersion: rocketmq.apache.org/v1alpha1
kind: Broker
metadata:
name: broker
namespace: mcamel-system
spec:
allowRestart: true
brokerImage: apacherocketmq/rocketmq-broker:4.5.0-alpine-operator-0.3.0
env:
- name: BROKER_MEM
valueFrom:
configMapKeyRef:
key: BROKER_MEM
name: broker-config
hostPath: /data/rocketmq/broker
imagePullPolicy: Always
nameServers: ""
replicaPerGroup: 1
resources:
limits:
cpu: 500m
memory: 20Mi ########### low memory here #############
requests:
cpu: 250m
memory: 10Mi ########### low memory here #############
scalePodName: broker-0-master-0
size: 1
storageMode: EmptyDir
volumeClaimTemplates:
- metadata:
annotations:
volume.beta.kubernetes.io/storage-class: rocketmq-storage
name: broker-storage
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 8Gi
volumes:
- configMap:
items:
- key: broker-common.conf
path: broker-common.conf
name: broker-config
name: broker-config
```
Status always empty
**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`.
- [ ] Make sure there is a [Github issue](https://github.com/apache/rocketmq-operator/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [ ] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Check RBAC rights for Kubernetes roles.
- [ ] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist.
- [ ] Run `make docker-build` to 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.
- [ ] Before committing your changes, remember to run `make manifests` to make sure the CRD files are updated.
- [ ] Update documentation if necessary.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #185] Fix pod ready status reflect to cr [rocketmq-operator]
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy merged PR #186:
URL: https://github.com/apache/rocketmq-operator/pull/186
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #185] Fix pod ready status reflect to cr [rocketmq-operator]
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on code in PR #186:
URL: https://github.com/apache/rocketmq-operator/pull/186#discussion_r1415304255
##########
pkg/controller/broker/broker_controller.go:
##########
@@ -398,6 +402,15 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
return broker.Name + "-" + strconv.Itoa(brokerGroupIndex)
}
+func isReady(po corev1.Pod) bool {
+ for _, cond := range po.Status.Conditions {
+ if cond.Type == corev1.PodReady {
Review Comment:
how do you think about it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #185] Fix pod ready status reflect to cr [rocketmq-operator]
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on code in PR #186:
URL: https://github.com/apache/rocketmq-operator/pull/186#discussion_r1415303793
##########
pkg/controller/broker/broker_controller.go:
##########
@@ -398,6 +402,15 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
return broker.Name + "-" + strconv.Itoa(brokerGroupIndex)
}
+func isReady(po corev1.Pod) bool {
+ for _, cond := range po.Status.Conditions {
+ if cond.Type == corev1.PodReady {
Review Comment:
Yes. I means we should wait pod to be ready, then execute follow reconcile code(https://github.com/apache/rocketmq-operator/blob/a8665716c8028741ec106814798e39fe07e6dcaa/pkg/controller/broker/broker_controller.go#L277-L343)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #185] Fix pod ready status reflect to cr [rocketmq-operator]
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy commented on code in PR #186:
URL: https://github.com/apache/rocketmq-operator/pull/186#discussion_r1411613390
##########
pkg/controller/broker/broker_controller.go:
##########
@@ -398,6 +402,15 @@ func getBrokerName(broker *rocketmqv1alpha1.Broker, brokerGroupIndex int) string
return broker.Name + "-" + strconv.Itoa(brokerGroupIndex)
}
+func isReady(po corev1.Pod) bool {
+ for _, cond := range po.Status.Conditions {
+ if cond.Type == corev1.PodReady {
Review Comment:
Should we check `PodReady` status at this stage? The brokers may be just started and initializing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org