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/08/01 14:21:49 UTC
[GitHub] [rocketmq-operator] drivebyer opened a new pull request, #165: [ISSUE #163] Add chart for operator deployment
drivebyer opened a new pull request, #165:
URL: https://github.com/apache/rocketmq-operator/pull/165
## What is the purpose of the change
Add chart for operator deployment, it's best practice in k8s environment.
## Brief changelog
Add chart for operator deployment.
## Verifying this change
<img width="1277" alt="截屏2023-08-01 22 17 20" src="https://github.com/apache/rocketmq-operator/assets/32033618/7c1fda4b-09df-413a-b01f-6ce658492934">
**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`.
- [x] 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.
- [x] 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.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Check RBAC rights for Kubernetes roles.
- [x] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist.
- [x] 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.
- [x] Before committing your changes, remember to run `make manifests` to make sure the CRD files are updated.
- [x] Update documentation if necessary.
- [x] 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
[GitHub] [rocketmq-operator] caigy merged pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy merged PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165
--
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
[GitHub] [rocketmq-operator] caigy commented on a diff in pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy commented on code in PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#discussion_r1285222259
##########
charts/rocketmq-operator/Chart.yaml:
##########
@@ -0,0 +1,24 @@
+apiVersion: v2
+name: rocketmq-operator
+description: A Helm chart for Kubernetes
+
+# A chart can be either an 'application' or a 'library' chart.
+#
+# Application charts are a collection of templates that can be packaged into versioned archives
+# to be deployed.
+#
+# Library charts provide useful utilities or functions for the chart developer. They're included as
+# a dependency of application charts to inject those utilities and functions into the rendering
+# pipeline. Library charts do not define any templates and therefore cannot be deployed.
+type: application
+
+# This is the chart version. This version number should be incremented each time you make changes
+# to the chart and its templates, including the app version.
+# Versions are expected to follow Semantic Versioning (https://semver.org/)
+version: 0.1.0
+
+# This is the version number of the application being deployed. This version number should be
+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "0.1.0"
Review Comment:
The `appVersion` should be the same as that of rocketmq-operator.
--
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
[GitHub] [rocketmq-operator] drivebyer commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1666873162
> Should those definition files of CRD be placed in `templates`?
Based on my understanding, the crds directory is used to place CRD definitions which are only installed once (https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-1-let-helm-do-it-for-you). Helm expects users to upgrade CRDs on their own.
--
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
[GitHub] [rocketmq-operator] drivebyer commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1668854434
> Were crd files copied into charts directory manually? If so, pls do it in `make manifest` to avoid missing that copy operation.
I got you.
Actually, the files in crds directory are copied directory, but the files in templated is copy then tempalted with values.yaml.
--
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
[GitHub] [rocketmq-operator] drivebyer commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1668860917
Maybe we should remove the old deploy fashion, and migrate to helm. @caigy
--
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
[GitHub] [rocketmq-operator] caigy commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "caigy (via GitHub)" <gi...@apache.org>.
caigy commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1674133757
> Maybe we should remove the old deploy fashion, and migrate to helm. @caigy
We'd better provide helm charts as another way to deploy rocketmq-operator. Although I think helm is a general way to manage app in Kubernetes, we should reserve the original way for users to choose.
--
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
[GitHub] [rocketmq-operator] drivebyer commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1674136460
> We'd better provide helm charts as another way to deploy rocketmq-operator. Although I think helm is a general way to manage app in Kubernetes, we should reserve the original way for users to choose.
Sure
--
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
[GitHub] [rocketmq-operator] drivebyer commented on a diff in pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on code in PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#discussion_r1285224423
##########
charts/rocketmq-operator/Chart.yaml:
##########
@@ -0,0 +1,24 @@
+apiVersion: v2
+name: rocketmq-operator
+description: A Helm chart for Kubernetes
+
+# A chart can be either an 'application' or a 'library' chart.
+#
+# Application charts are a collection of templates that can be packaged into versioned archives
+# to be deployed.
+#
+# Library charts provide useful utilities or functions for the chart developer. They're included as
+# a dependency of application charts to inject those utilities and functions into the rendering
+# pipeline. Library charts do not define any templates and therefore cannot be deployed.
+type: application
+
+# This is the chart version. This version number should be incremented each time you make changes
+# to the chart and its templates, including the app version.
+# Versions are expected to follow Semantic Versioning (https://semver.org/)
+version: 0.1.0
+
+# This is the version number of the application being deployed. This version number should be
+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "0.1.0"
Review Comment:
fixed to 0.3.0 already
--
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
[GitHub] [rocketmq-operator] drivebyer commented on pull request #165: [ISSUE #163] Add chart for operator deployment
Posted by "drivebyer (via GitHub)" <gi...@apache.org>.
drivebyer commented on PR #165:
URL: https://github.com/apache/rocketmq-operator/pull/165#issuecomment-1660433782
PTAL @caigy
--
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