-
Notifications
You must be signed in to change notification settings - Fork 227
Fix helm upgrade procedure #431
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
base: main
Are you sure you want to change the base?
Conversation
Add helm values for mutation mode and threshold Fix readme
WalkthroughThe changes update documentation for configuring Robusta account connections, adjust the default mutation mode for the enforcer, and introduce new configuration options for mutation mode and update threshold in Helm values. The Helm chart version is incremented, deployment templates are updated with new environment variables and annotations, and certificate job logic is simplified by removing deployment restart steps. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant Enforcer Pod
User->>Helm: Install/Upgrade krr-enforcer with values.yaml
Helm->>Kubernetes: Deploy enforcer.yaml with env vars (mutationMode, updateThreshold)
Kubernetes->>Enforcer Pod: Start pod with configured environment variables
Note right of Enforcer Pod: Uses KRR_MUTATION_MODE_DEFAULT ("ignore" by default)\nUses UPDATE_THRESHOLD (default 20.0)
sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant CertJobPod
User->>Helm: Run enforcer-cert-job
Helm->>Kubernetes: Create cert-job pod
Kubernetes->>CertJobPod: Generate certs, create secrets, configure webhook
Note right of CertJobPod: No longer restarts enforcer deployment after cert changes
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
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: 2
🧹 Nitpick comments (1)
enforcer/env_vars.py (1)
18-18
: Align default mutation mode with Helm values
Changes the code fallback forKRR_MUTATION_MODE_DEFAULT
from"enforce"
to"ignore"
, matching the new HelmmutationMode
default. Consider validating this value at startup to catch typos or unsupported modes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
enforcer/README.md
(1 hunks)enforcer/env_vars.py
(1 hunks)helm/krr-enforcer/Chart.yaml
(1 hunks)helm/krr-enforcer/templates/enforcer-cert-job.yaml
(0 hunks)helm/krr-enforcer/templates/enforcer.yaml
(2 hunks)helm/krr-enforcer/values.yaml
(1 hunks)
💤 Files with no reviewable changes (1)
- helm/krr-enforcer/templates/enforcer-cert-job.yaml
🔇 Additional comments (6)
helm/krr-enforcer/values.yaml (2)
4-7
: Introduce Enforcer configuration defaults
Defines global default settings for mutation mode and update threshold, aligning with new environment variables in the enforcer container.
13-13
: Update container image tag
Bumps the enforcer’s image version from0.3.1
to0.3.2
, consistent with the Chart.yaml and deployment template.helm/krr-enforcer/Chart.yaml (1)
6-7
: Bump Helm chart and app versions
Synchronizes bothversion
andappVersion
fields to0.3.2
, aligning chart metadata with the updated image and configuration.helm/krr-enforcer/templates/enforcer.yaml (2)
27-27
: Add checksum annotation to trigger pod restarts
Includes a SHA256 hash of the release revision to the pod template annotations, ensuring the enforcer pod rolls when certificates change.
122-125
: Inject new environment variables
AddsKRR_MUTATION_MODE_DEFAULT
andUPDATE_THRESHOLD
into the container spec, sourcing from.Values
, to propagate Helm settings into the running pod. Values are correctly quoted for string-based env vars.enforcer/README.md (1)
62-69
: Clarify Robusta account configuration step
Introduces a new “Configure Robusta Account Connection” section to guide users through account setup. The instructions are concise, but verify that the environment variable names match those consumed by the code.
# enforcer-values.yaml | ||
additionalEnvVars: | ||
- name: CLUSTER_NAME | ||
value: my-cluster-name # should be the same as the robusta installation on this cluster | ||
value: my-cluster-name # should match your Robusta cluster name | ||
- name: ROBUSTA_UI_TOKEN | ||
value: "MY ROBUSTA UI TOKEN" | ||
# - name: ROBUSTA_UI_TOKEN # or pulled from a secret | ||
# valueFrom: | ||
# secretKeyRef: | ||
# name: robusta-secrets | ||
# key: robustaSinkToken | ||
value: "MY_ROBUSTA_UI_TOKEN" | ||
# OR pull from a secret: | ||
# - name: ROBUSTA_UI_TOKEN | ||
# valueFrom: | ||
# secretKeyRef: | ||
# name: robusta-secrets | ||
# key: robustaSinkToken |
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.
Correct environment variable names in Option B
Docs reference CLUSTER_NAME
and ROBUSTA_UI_TOKEN
, but the code reads ROBUSTA_ACCOUNT_ID
and STORE_*
variables. Confirm and align these names to avoid runtime mismatches.
🤖 Prompt for AI Agents
In enforcer/README.md between lines 85 and 96, the environment variable names in
the documentation do not match those used in the code, causing potential runtime
mismatches. Review the code to identify the exact environment variable names it
expects, such as `ROBUSTA_ACCOUNT_ID` and `STORE_*`, then update the README to
use these exact names instead of `CLUSTER_NAME` and `ROBUSTA_UI_TOKEN`. Ensure
the documentation and code use consistent environment variable names to prevent
confusion and errors.
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.
@arikalon1 I assume this is wrong?
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
enforcer/README.md
Outdated
```yaml | ||
# enforcer-values.yaml | ||
additionalEnvVars: | ||
- name: TOKEN_ENV_VAR_NAME | ||
valueFrom: | ||
secretKeyRef: | ||
name: robusta-secrets | ||
key: robustaSinkToken | ||
``` |
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.
🛠️ Refactor suggestion
Refine Option A example to match actual env var names
The placeholder TOKEN_ENV_VAR_NAME
may be confusing. Replace it with the concrete variable your deployment uses (e.g., ROBUSTA_UI_TOKEN
or STORE_API_KEY
) to ensure consistency between docs and code.
🤖 Prompt for AI Agents
In enforcer/README.md around lines 71 to 79, the example uses a placeholder
environment variable name TOKEN_ENV_VAR_NAME, which can be confusing. Replace
TOKEN_ENV_VAR_NAME with a concrete environment variable name that matches your
actual deployment, such as ROBUSTA_UI_TOKEN or STORE_API_KEY, to ensure the
documentation aligns with real usage and avoids ambiguity.
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
♻️ Duplicate comments (1)
enforcer/README.md (1)
78-82
: Correct environment variable names for Option B
The documentation currently showsCLUSTER_NAME
andROBUSTA_UI_TOKEN
, but the enforcer code expectsROBUSTA_ACCOUNT_ID
andSTORE_*
variables. Please verify and align the YAML example to use the exact environment variable names to avoid runtime mismatches.
🧹 Nitpick comments (1)
enforcer/README.md (1)
69-69
: Insert comma for grammatical correctness
Use a comma before "and" to separate independent clauses:- this method won’t work and you should use Option B (described below) instead. + this method won’t work, and you should use Option B (described below) instead.🧰 Tools
🪛 LanguageTool
[uncategorized] ~69-~69: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...netes-secrets)), this method won’t work and you should use Option B (described belo...(COMMA_COMPOUND_SENTENCE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enforcer/README.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
enforcer/README.md
[uncategorized] ~69-~69: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...netes-secrets)), this method won’t work and you should use Option B (described belo...
(COMMA_COMPOUND_SENTENCE)
🔇 Additional comments (4)
enforcer/README.md (4)
62-63
: Added a dedicated account-connection step
Inserting "Configure Robusta Account Connection" as step 2 clearly separates prerequisites from installation.
64-65
: Introductory explanation is clear
This sentence succinctly explains why the Enforcer needs account access before presenting configuration options.
66-68
: Option A description is well-structured
The "Same Namespace" option is clearly explained and highlights its simplicity for users co-located with Robusta.
72-74
: Option B section is clear
The heading and description for the different-namespace setup provide clear guidance on when explicit credentials are needed.
Add helm values for mutation mode and threshold
Fix readme
Summary by CodeRabbit