-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
refactor!(crds): automatic CRD installation and upgrade #157
Conversation
Warning Rate limit exceeded@zyy17 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 39 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent update to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant K8s as Kubernetes
participant CRD as Custom Resource Definitions
User->>Helm: Install/Upgrade Chart
Helm->>CRD: Check CRD Installation Options
alt Install CRDs
CRD->>K8s: Install CRDs
end
User->>K8s: Manage CRDs
K8s->>CRD: Retrieve CRD Information
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- charts/greptimedb-operator/Chart.yaml (1 hunks)
- charts/greptimedb-operator/README.md (3 hunks)
- charts/greptimedb-operator/README.md.gotmpl (1 hunks)
- charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml (2 hunks)
- charts/greptimedb-operator/templates/crds/crd-greptimedbstandalone.yaml (2 hunks)
- charts/greptimedb-operator/values.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- charts/greptimedb-operator/Chart.yaml
Additional context used
LanguageTool
charts/greptimedb-operator/README.md
[uncategorized] ~49-~49: Possible missing preposition found.
Context: ...when installing the chart. If you want upgrade CRDs manually, you can use the followin...(AI_HYDRA_LEO_MISSING_TO)
yamllint
charts/greptimedb-operator/templates/crds/crd-greptimedbstandalone.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
Additional comments not posted (9)
charts/greptimedb-operator/values.yaml (1)
24-33
: New CRD configuration options look good.The addition of the
crds
section enhances configurability for CRD management. The default values and descriptions are clear and consistent with the rest of the file.charts/greptimedb-operator/README.md.gotmpl (1)
46-47
: Clarify CRD installation instructions.The documentation clearly explains the automatic CRD installation feature and how to disable it. This enhances user understanding and aligns with the new configuration options.
charts/greptimedb-operator/README.md (3)
5-5
: Version update is appropriate.The version number has been correctly updated to
0.2.1
to reflect the new release.
47-48
: Clarify CRD installation instructions.The documentation clearly explains the automatic CRD installation feature and how to disable it. This enhances user understanding and aligns with the new configuration options.
78-81
: New CRD configuration parameters are well-documented.The additional parameters for CRD management are clearly explained, providing users with detailed customization options.
charts/greptimedb-operator/templates/crds/crd-greptimedbstandalone.yaml (1)
6-15
: Conditional logic for CRD annotations and labels is well-implemented.The use of Helm templating to conditionally add annotations and labels based on user-defined values enhances flexibility and usability.
charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml (3)
1-1
: Conditional CRD installation logic looks good.The use of
{{- if .Values.crds.install }}
is appropriate for conditional CRD installation based on Helm chart values.Tools
yamllint
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
6-11
: Conditional annotations logic looks good.The conditional logic for
helm.sh/resource-policy
and additional annotations is well-implemented, allowing for flexible resource management.Ensure that the indentation is correct for YAML rendering.
12-15
: Conditional labels logic looks good.The conditional logic for additional labels is correctly implemented, enhancing the flexibility of label management.
Ensure that the indentation is correct for YAML rendering.
fba144d
to
56e6852
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
charts/greptimedb-operator/README.md (1)
47-48
: Clarify CRD installation instructions.The documentation now highlights automatic CRD installation. Consider rephrasing to improve clarity and address the LanguageTool suggestion.
- Yon can disable the CRD installation by using `--set crds.install=false` when installing the chart. + You can disable the CRD installation by using `--set crds.install=false` when installing the chart.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- Makefile (1 hunks)
- charts/greptimedb-operator/Chart.yaml (1 hunks)
- charts/greptimedb-operator/README.md (3 hunks)
- charts/greptimedb-operator/README.md.gotmpl (1 hunks)
- charts/greptimedb-operator/values.yaml (1 hunks)
- scripts/crds/templates/crd-greptimedbcluster.tmpl (1 hunks)
- scripts/crds/templates/crd-greptimedbstandalone.tmpl (1 hunks)
- scripts/crds/update-crds.sh (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- charts/greptimedb-operator/Chart.yaml
- charts/greptimedb-operator/README.md.gotmpl
- charts/greptimedb-operator/values.yaml
Additional context used
LanguageTool
charts/greptimedb-operator/README.md
[uncategorized] ~49-~49: Possible missing preposition found.
Context: ...when installing the chart. If you want upgrade CRDs manually, you can use the followin...(AI_HYDRA_LEO_MISSING_TO)
Additional comments not posted (7)
scripts/crds/templates/crd-greptimedbcluster.tmpl (1)
1-19
: Ensure CRD installation logic is correct.The template conditionally installs the CRD based on the
.Values.crds.install
flag. This approach is consistent with best practices for optional resource management in Helm charts. Ensure that thecrds.install
value is correctly set in thevalues.yaml
file or during deployment.scripts/crds/templates/crd-greptimedbstandalone.tmpl (1)
1-19
: Verify CRD installation logic and customization options.This template follows a similar pattern to the
crd-greptimedbcluster.tmpl
file, using the.Values.crds.install
flag to conditionally manage CRD installation. Ensure that this logic aligns with user expectations and that thevalues.yaml
file is configured appropriately.Makefile (3)
31-33
: Update CRD management target names for clarity.The
crds
target has been renamed toupdate-crds
, which improves clarity. Ensure that any documentation or scripts referencing the old target name are updated accordingly.
40-42
: Ensure consistency in CRD check logic.The
check-crds
target now depends onupdate-crds
. Ensure that this dependency correctly reflects the intended workflow and that any references tomake crds
are updated tomake update-crds
.
37-37
: Verify script path forupgrade-crds
.The script path for
upgrade-crds
has been updated. Ensure that the new path is correct and that the script is executable.Verification successful
Script path verified and executable
The script
./scripts/crds/upgrade-crds.sh
is correctly located and is executable. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and executability of the `upgrade-crds.sh` script. # Test: Check if the script exists and is executable. Expect: No output if the script is correct. if [[ ! -x "./scripts/crds/upgrade-crds.sh" ]]; then echo "The script ./scripts/crds/upgrade-crds.sh is missing or not executable." fiLength of output: 43
charts/greptimedb-operator/README.md (2)
5-5
: Version update confirmed.The version number has been updated to
0.2.1
, reflecting the new release. Ensure that this version is consistently used across all related documentation and files.
78-81
: New CRD configuration options documented.The README now includes new parameters for CRD management, providing users with more control. Ensure these options are accurately reflected in the
values.yaml
file and any related documentation.
56e6852
to
f16f487
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- Makefile (1 hunks)
- charts/greptimedb-operator/Chart.yaml (1 hunks)
- charts/greptimedb-operator/README.md (3 hunks)
- charts/greptimedb-operator/README.md.gotmpl (1 hunks)
- charts/greptimedb-operator/values.yaml (1 hunks)
- scripts/crds/templates/crd-greptimedbcluster.tmpl (1 hunks)
- scripts/crds/templates/crd-greptimedbstandalone.tmpl (1 hunks)
- scripts/crds/update-crds.sh (1 hunks)
Files skipped from review due to trivial changes (2)
- charts/greptimedb-operator/Chart.yaml
- scripts/crds/templates/crd-greptimedbstandalone.tmpl
Files skipped from review as they are similar to previous changes (6)
- Makefile
- charts/greptimedb-operator/README.md
- charts/greptimedb-operator/README.md.gotmpl
- charts/greptimedb-operator/values.yaml
- scripts/crds/templates/crd-greptimedbcluster.tmpl
- scripts/crds/update-crds.sh
f16f487
to
7f1a665
Compare
7f1a665
to
57104e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
57104e6
to
0ce70d4
Compare
Make the CRDs installation and upgrade automatically using the Helm chart.
The implementation reference is by argo-helm.
Summary by CodeRabbit
New Features
greptimedb-operator
Helm chart to 0.2.1.Documentation
Bug Fixes