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.

Sign up for GitHub

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

Jump to bottom

feat(marketplace): also annotate nodes on upgrade to support shim upgrade #35

Merged
merged 2 commits into from
Nov 7, 2024
Merged

feat(marketplace): also annotate nodes on upgrade to support shim upgrade #35

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18f9e00
feat(marketplace): also annotate nodes on upgrade to support shim upg…
vdice Nov 7, 2024
9d54816
ci(.github): add upgrade/shim re-install check to smoke-test
vdice Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions marketplace/charts/spinkube-azure-marketplace/templates/kwasm-annotate-nodes-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ kind: Job
metadata:
name: "{{ .Release.Name }}-kwasm-annotate-nodes"
annotations:
"helm.sh/hook": post-install
"helm.sh/hook": post-install,post-upgrade
Copy link
Member

Choose a reason for hiding this comment

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

It seems fine to re-labeling nodes for both the post-install and post-upgrade scenarios. Have you tested this? I am asking because I am concerend about if we need to have buffer time of labeling kwasm-node from false to true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have tested locally, though only on KinD. Maybe worth checking on AKS?

Oh, I could also add test coverage for this in our helm chart smoke test (or create a new workflow with scenarios like this)...

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of adding more tests

"helm.sh/hook-weight": "-4"
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
spec:
Expand All @@ -15,8 +15,13 @@ spec:
containers:
- name: kubectl
image: {{ printf "%s/%s:%s" .Values.global.azure.images.kubectl.registry .Values.global.azure.images.kubectl.image .Values.global.azure.images.kubectl.tag }}
command: ["kubectl"]
args: ["annotate", "node", "--all", "kwasm.sh/kwasm-node=true"]
command: ["/bin/sh", "-c"]
args:
- |-
echo "Annotating node with kwasm.sh/kwasm-node=false to reset installation of the shim for upgrade scenarios"
kubectl annotate node --all kwasm.sh/kwasm-node=false --overwrite
echo "Annotating node with kwasm.sh/kwasm-node=true to (re-)trigger installation of the shim"
kubectl annotate node --all kwasm.sh/kwasm-node=true --overwrite
restartPolicy: OnFailure
---
apiVersion: v1
Expand Down
Loading