93 auto rollout for dev environment #130
Conversation
WalkthroughThe changes introduce OpenShift ImageStream support to the Helm chart for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant OpenShift
User->>Helm: Install/upgrade chart with imagestream set
Helm->>Kubernetes: Deploy resources (Deployment, ImageStream)
Kubernetes->>OpenShift: Create ImageStream resource
OpenShift->>OpenShift: Monitor ImageStream tags for updates
OpenShift->>Kubernetes: Trigger rollout of Deployment if image tag changes
Poem
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
Documentation and Community
|
| {{/* | ||
| Get the truncated module name for the image stream | ||
| */}} | ||
| {{- define "getFullnameImageStream" -}} | ||
| {{- $moduleName := $.Values.imagestream }} | ||
| {{- printf "%s-%s" $.Release.Name $moduleName | trunc 63 | trimSuffix "-" -}} | ||
| {{- end -}} | ||
|
|
There was a problem hiding this comment.
schade dass ich hier die code kopieren musst und nicht getFullname nutzen könnte. man hätte da einfach zwei Parameter machen können .
![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
♻️ Duplicate comments (1)
charts/refarch-templates/templates/_helpers.tpl (1)
17-24: 🧹 Nitpick (assertive)Consider refactoring to reduce duplication.
The
getFullnameImageStreamfunction duplicates logic from the existinggetFullnamefunction. It would be better to parameterize the existing function to handle both cases.Here's how you could refactor to use a single function:
{{/* Get the truncated module name with the release name #dev-backend */}} {{- define "getFullname" -}} -{{- $moduleName := .module.name }} -{{- printf "%s-%s" .dot.Release.Name $moduleName | trunc 63 | trimSuffix "-" -}} +{{- if kindIs "map" . -}} + {{- $moduleName := .module.name }} + {{- printf "%s-%s" .dot.Release.Name $moduleName | trunc 63 | trimSuffix "-" -}} +{{- else -}} + {{- $moduleName := . }} + {{- printf "%s-%s" $.Release.Name $moduleName | trunc 63 | trimSuffix "-" -}} +{{- end -}} {{- end -}}Then use
getFullnamein place ofgetFullnameImageStream.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
charts/refarch-templates/Chart.yaml(1 hunks)charts/refarch-templates/README.md(1 hunks)charts/refarch-templates/templates/_helpers.tpl(2 hunks)charts/refarch-templates/templates/deployment.yaml(1 hunks)charts/refarch-templates/templates/imagestream.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/refarch-templates/templates/imagestream.yml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 4-4: too many spaces inside braces
(braces)
🔇 Additional comments (4)
charts/refarch-templates/Chart.yaml (1)
5-5: Version bump looks good.The chart version has been incremented from 1.0.1 to 1.0.4 to reflect the addition of the new OpenShift ImageStream integration feature.
charts/refarch-templates/templates/deployment.yaml (1)
9-12: Good implementation of conditional image trigger annotations.The annotations are correctly added only when the
$.Values.imagestreamvalue is set, which keeps this OpenShift-specific feature optional. The template properly uses the newtriggerAnnotationhelper to generate the correct annotation format.charts/refarch-templates/README.md (1)
328-336: Clear documentation for the new autorollout feature.The documentation clearly explains the purpose of the feature (development environments), how it works (automatic rollouts when image tags change), and includes a link to OpenShift documentation for further details. The example configuration is simple and easy to understand.
charts/refarch-templates/templates/_helpers.tpl (1)
56-63: Well-implemented image trigger annotation template.The
triggerAnnotationhelper correctly generates the OpenShift-specific annotation format required to trigger automatic deployments when the ImageStream tag changes. The parameter handling and JSON formatting are properly implemented.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
charts/refarch-templates/templates/imagestream.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/refarch-templates/templates/imagestream.yml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[error] 5-5: too many spaces inside braces
(braces)
🔇 Additional comments (1)
charts/refarch-templates/templates/imagestream.yml (1)
16-20: Import and reference policy settings look good
Thescheduled: trueimportPolicy andSourcereferencePolicy align with the autorollout behavior for development. No changes needed here.
| metadata: | ||
| name: {{ include "getFullnameImageStream" $}} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Refine helper template spacing in metadata
Although {{ include "getFullnameImageStream" $}} works, it triggers a “braces” warning in YAMLLint. For better readability and to satisfy linters, consider adding a space before the closing braces:
name: {{ include "getFullnameImageStream" $ }}
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: too many spaces inside braces
(braces)
🤖 Prompt for AI Agents
In charts/refarch-templates/templates/imagestream.yml at lines 4 to 5, the Helm
template expression for the name field lacks a space before the closing braces,
causing a YAMLLint "braces" warning. To fix this, add a space before the closing
braces in the include statement, changing {{ include "getFullnameImageStream"
$}} to {{ include "getFullnameImageStream" $ }} for proper spacing and lint
compliance.
| spec: | ||
| lookupPolicy: | ||
| local: false | ||
| tags: |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Guard spec.tags against empty or undefined modules
If .Values.modules is unset or empty, you’ll emit a tags: block with no entries, potentially invalidating the ImageStream. Wrap this section in an if or default modules to [] in your values.yaml:
spec:
lookupPolicy:
local: false
{{- if .Values.modules }}
tags:
{{- range .Values.modules }}
…
{{- end }}
{{- end }}
🤖 Prompt for AI Agents
In charts/refarch-templates/templates/imagestream.yml around lines 6 to 9, the
spec.tags block is rendered even when .Values.modules is empty or undefined,
which can produce invalid YAML. To fix this, wrap the tags section and its
contents inside an if conditional that checks if .Values.modules exists and is
non-empty, or alternatively set a default empty array for modules in
values.yaml. This prevents emitting an empty tags block and ensures valid
ImageStream manifests.
| {{- if $.Values.imagestream }} | ||
| kind: ImageStream | ||
| apiVersion: image.openshift.io/v1 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Manage YAMLLint false positives & adjust field ordering
The leading {{- if $.Values.imagestream }} is valid Helm syntax but YAMLLint will flag the trim directive as a syntax error. You can safely ignore these false positives by configuring your .yamllint to skip templates under charts/**/templates.
Also, while not functionally required, swapping the order to have apiVersion before kind improves consistency with other Kubernetes resources.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/refarch-templates/templates/imagestream.yml around lines 1 to 3,
YAMLLint flags the Helm trim directive in the conditional as a syntax error,
which is a false positive; configure your .yamllint file to exclude linting for
templates under charts/**/templates to ignore these safely. Additionally,
reorder the fields so that apiVersion appears before kind to align with
Kubernetes resource conventions, improving consistency.
| - name: {{.name}} | ||
| annotations: null |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use an empty mapping for annotations
Rendering annotations: null produces a YAML null. If you intend no annotations, prefer an empty map:
- annotations: null
+ annotations: {}📝 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.
| - name: {{.name}} | |
| annotations: null | |
| - name: {{.name}} | |
| annotations: {} |
🤖 Prompt for AI Agents
In charts/refarch-templates/templates/imagestream.yml at lines 11 to 12, replace
the line setting annotations to null with an empty mapping to avoid rendering a
YAML null. Change the annotations field from 'annotations: null' to
'annotations: {}' to represent no annotations correctly as an empty map.
| {{- range .Values.modules }} | ||
| - name: {{.name}} | ||
| annotations: null | ||
| from: | ||
| kind: DockerImage | ||
| name: {{.image.repository}}:{{.image.tag}} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix indentation of range block under tags:
The {{- range .Values.modules }} and its list items must be indented to align under tags:. For example:
tags:
+ {{- range .Values.modules }}
- name: {{ .name }}
annotations: null
from:
kind: DockerImage
name: {{ .image.repository }}:{{ .image.tag }}
{{- end }}This alignment ensures valid YAML after rendering.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In charts/refarch-templates/templates/imagestream.yml around lines 10 to 15, the
{{- range .Values.modules }} block and its nested list items are not properly
indented under the tags: key. Adjust the indentation so that the range block and
its contents align correctly beneath tags:, ensuring the rendered YAML structure
is valid and properly nested.
Description
Short description or comments
Reference
Issues it-at-m/lhm_actions#93
Summary by CodeRabbit
imagestreamvalue.