Repository navigation
[platform] Another logic for deleting components - #811
Conversation
Signed-off-by: Andrei Kvapil <[email protected]>
WalkthroughThis pull request removes and consolidates certain deletion targets within the Makefile and refines the HelmRelease template logic. In the Makefile, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Kubernetes
User->>Makefile: Invoke 'reconcile' target
Makefile->>Makefile: Execute 'apply' target
Makefile->>Kubernetes: Run 'kubectl delete' for marked HelmReleases
sequenceDiagram
participant TemplateEngine
participant HelmTemplates
participant Kubernetes
TemplateEngine->>HelmTemplates: Evaluate $shouldInstall & $shouldDelete conditions
alt $shouldDelete is true
HelmTemplates->>Kubernetes: Render YAML with deletion label
else $shouldInstall is true
HelmTemplates->>Kubernetes: Render standard HelmRelease YAML
end
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
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:
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 (
|
Andrei Kvapil (kvaps)
added
the
ok-to-test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/core/platform/Makefile (2)
9-13: Consolidated deletion logic in the apply target.
The new line 12 now directly deletes HelmReleases tagged withcozystack.io/marked-for-deletion=trueacross all namespaces. This simplifies the workflow by eliminating the separatedeletetarget. However, please verify that this deletion command does not inadvertently remove resources that should be retained.
16-24: Pipeline failure investigation needed.
The pipeline log shows an error related to a target namedgen-versions-map(make: *** [Makefile:17: gen-versions-map] Error 128). Although this target is not present in the diff, please double-check if any dependencies on it remain (or in other parts of the Makefile) so that the CI process can pass.🧰 Tools
🪛 GitHub Actions: Pre-Commit Checks
[error] 17-17: make: *** [Makefile:17: gen-versions-map] Error 128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/platform/Makefile(1 hunks)packages/core/platform/templates/helmreleases.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/core/platform/templates/helmreleases.yaml
[warning] 18-18: wrong indentation: expected 0 but found 2
(indentation)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 4
(indentation)
[warning] 21-21: wrong indentation: expected 0 but found 2
(indentation)
🪛 GitHub Actions: Pre-Commit Checks
packages/core/platform/Makefile
[error] 17-17: make: *** [Makefile:17: gen-versions-map] Error 128
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test
🔇 Additional comments (2)
packages/core/platform/Makefile (1)
14-15: Updated reconcile target dependency.
Thereconciletarget now depends solely onapply, which is consistent with your consolidation effort. Ensure that any dependencies or workflows that previously relied on the removeddeletetarget are updated accordingly.packages/core/platform/templates/helmreleases.yaml (1)
24-36: Conditional YAML output for deletion label.
The conditional block (lines 24–36) correctly appends the deletion label when$shouldDeleteis true. Double-check that the generated YAML reflects the intended metadata structure and that the label matches what the Makefile deletion command expects.
|
|
||
| {{- $shouldInstall := true }} | ||
| {{- $shouldDelete := false }} | ||
| {{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }} | ||
| {{- $shouldInstall = false }} | ||
| {{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }} | ||
| {{- $shouldDelete = true }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Review template logic and YAML indentation.
The new block initializing $shouldInstall and $shouldDelete and adjusting their values based on component status (lines 15–21) correctly implements the desired logic. However, YAML lint warnings indicate that lines 18–21 have inconsistent indentation (expected 0 but found extra spaces). Since these are Go template directives within a YAML file, consider unindenting these lines or aligning them with the surrounding template syntax to ensure YAML rendering and linting pass correctly. For instance:
-{{- $shouldInstall := true }}
-{{- $shouldDelete := false }}
-{{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }}
- {{- $shouldInstall = false }}
- {{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }}
- {{- $shouldDelete = true }}
- {{- end }}
-{{- end }}
+{{- $shouldInstall := true }}
+{{- $shouldDelete := false }}
+{{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }}
+{{- $shouldInstall = false }}
+{{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }}
+{{- $shouldDelete = true }}
+{{- end }}
+{{- end }}Please adjust the indentation as needed for consistency in your templating context.
Action Required: Fix Indentation in YAML Template
Based on our review, the templating logic in the snippet is correct, but the YAML linter is flagging extra indentation in lines 18–21. Please adjust these lines so that the Go template directives are aligned consistently with the surrounding code. For example:
-{{- $shouldInstall := true }}
-{{- $shouldDelete := false }}
-{{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }}
- {{- $shouldInstall = false }}
- {{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }}
- {{- $shouldDelete = true }}
- {{- end }}
-{{- end }}
+{{- $shouldInstall := true }}
+{{- $shouldDelete := false }}
+{{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }}
+{{- $shouldInstall = false }}
+{{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }}
+{{- $shouldDelete = true }}
+{{- end }}
+{{- end }}Please update the file at packages/core/platform/templates/helmreleases.yaml accordingly to resolve the YAML linting warnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- $shouldInstall := true }} | |
| {{- $shouldDelete := false }} | |
| {{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }} | |
| {{- $shouldInstall = false }} | |
| {{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }} | |
| {{- $shouldDelete = true }} | |
| {{- end }} | |
| {{- end }} | |
| {{- $shouldInstall := true }} | |
| {{- $shouldDelete := false }} | |
| {{- if or (has $x.name $disabledComponents) (and ($x.optional) (not (has $x.name $enabledComponents))) }} | |
| {{- $shouldInstall = false }} | |
| {{- if lookup "helm.toolkit.fluxcd.io/v2" "HelmRelease" $x.namespace $x.name }} | |
| {{- $shouldDelete = true }} | |
| {{- end }} | |
| {{- end }} |
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 18-18: wrong indentation: expected 0 but found 2
(indentation)
[warning] 19-19: wrong indentation: expected 0 but found 2
(indentation)
[warning] 20-20: wrong indentation: expected 0 but found 4
(indentation)
[warning] 21-21: wrong indentation: expected 0 but found 2
(indentation)
2 participants
Signed-off-by: Andrei Kvapil [email protected]
Summary by CodeRabbit