Skip to content
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

Add Conditional Logic for deploymentAnnotations in Helm Chart #99

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

iGusev
Copy link
Contributor

@iGusev iGusev commented Apr 16, 2024

Changes Description

This pull request introduces changes to the Helm template to implement conditional logic for deploymentAnnotations. The update ensures that the annotations: block is only included in the Kubernetes Deployment manifest if deploymentAnnotations are defined in the values.yaml file.

Motivation

The motivation behind this change is to keep the Deployment manifest clean and free from empty or unnecessary annotations when no deploymentAnnotations are specified, thereby preventing potential deployment issues and maintaining clarity in the manifest files.

Implementation Details

  • An if statement checks for the presence of deploymentAnnotations in the values.yaml.
  • If deploymentAnnotations exist, the annotations block is added to the Deployment manifest.
  • If there are no deploymentAnnotations, the annotations block is completely omitted, ensuring that only relevant metadata is included in the deployed resources.

This approach helps in managing the Deployment's metadata more dynamically based on the configuration provided and avoids cluttering the Kubernetes manifest with empty or irrelevant annotations.

Additional Notes

Please review the changes to ensure that the conditional logic aligns with our deployment standards and let me know if any adjustments are required.

@@ -9,6 +9,10 @@ metadata:
name: {{ include "krakend.fullname" . }}
labels:
{{- include "krakend.labels" . | nindent 4 }}
{{- if .Values.deploymentAnnotations }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might want to follow the same pattern we've been using:

  {{- with .Values.deploymentAnnotations }}
    annotations:
    {{- toYaml . | nindent 4 }}
  {{- end }}

for consistency.

templates/deployment.yaml Outdated Show resolved Hide resolved
@JAORMX JAORMX merged commit f431a76 into equinixmetal-helm:main Apr 17, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants