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

Fix indentation of the sa annotations #879

Merged
merged 1 commit into from Aug 7, 2023
Merged

Conversation

Kidswiss
Copy link
Contributor

@Kidswiss Kidswiss commented Aug 7, 2023

Summary

  • The indentation for the sa annotations was wrong.

Fixes issue found in: #876

Checklist

For Helm Chart changes

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • PR contains the label area:chart
  • PR contains the chart label, e.g. chart:k8up
  • Variables are documented in the values.yaml using the format required by Helm-Docs.
  • Chart Version bumped if immediate release after merging is planned
  • I have run make chart-docs
  • Link this PR to related code release or other issues.

@Kidswiss Kidswiss requested a review from a team as a code owner August 7, 2023 06:51
@Kidswiss Kidswiss requested review from zugao and lieneluksika and removed request for a team August 7, 2023 06:51
@Kidswiss Kidswiss added area:chart chart:k8up bug Something isn't working labels Aug 7, 2023
Signed-off-by: Simon Beck <simon.beck@vshn.ch>
@Kidswiss
Copy link
Contributor Author

Kidswiss commented Aug 7, 2023

@mkotsalainen can you test the branch from this PR? If that works for you I'll merge it.

@mkotsalainen
Copy link

This looks good! Thank you!

➜  k8up git:(fix/helm_indentation) helm template --debug --dry-run foo charts/k8up/ --set serviceAccount.annotations.qwer=asdf | grep asdf -B 10 -A10
install.go:200: [debug] Original chart version: ""
install.go:217: [debug] CHART PATH: /tmp/k8up/charts/k8up

apiVersion: v1
kind: ServiceAccount
metadata:
  name: foo-k8up
  labels:
    helm.sh/chart: k8up-4.4.1
    app.kubernetes.io/name: k8up
    app.kubernetes.io/instance: foo
    app.kubernetes.io/managed-by: Helm
  annotations:
    qwer: asdf

@mkotsalainen
Copy link

Tried installing also and it works.

➜  k8up git:(fix/helm_indentation) helm install -n foo foo charts/k8up/ --set serviceAccount.annotations.qwer=asdf
NAME: foo
LAST DEPLOYED: Mon Aug  7 09:31:25 2023
NAMESPACE: foo
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
#####################
! Attention !
#####################

This Helm chart does not include CRDs.
Please make sure you have installed or upgraded the necessary CRDs as instructed in the Chart README.

#####################
➜  k8up git:(fix/helm_indentation) k describe sa foo-k8up -n foo
Name:                foo-k8up
Namespace:           foo
Labels:              app.kubernetes.io/instance=foo
                     app.kubernetes.io/managed-by=Helm
                     app.kubernetes.io/name=k8up
                     helm.sh/chart=k8up-4.4.1
Annotations:         meta.helm.sh/release-name: foo
                     meta.helm.sh/release-namespace: foo
                     qwer: asdf
Image pull secrets:  <none>
Mountable secrets:   <none>
Tokens:              <none>
Events:              <none>

@Kidswiss
Copy link
Contributor Author

Kidswiss commented Aug 7, 2023

Great! Merging and release.

@Kidswiss Kidswiss merged commit 4abacf0 into master Aug 7, 2023
6 of 7 checks passed
@Kidswiss Kidswiss deleted the fix/helm_indentation branch August 7, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:chart bug Something isn't working chart:k8up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants