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

Templatize KubeVirt namespace #61

Merged
merged 9 commits into from
Dec 12, 2023
Merged

Templatize KubeVirt namespace #61

merged 9 commits into from
Dec 12, 2023

Conversation

atanasdinov
Copy link
Contributor

@atanasdinov atanasdinov commented Dec 11, 2023

  • Extracted the namespace resource out of the operator manifest
  • Chart is now installable via helm install kubevirt <chart-repo> -n <namespace> --create-namespace
  • This should prevent errors in Rancher where a kubevirt namespace was specified but it was already part of the chart itself
  • Upgrade and deletion flows are confirmed to be working properly

Comment on lines +70 to +71
kubectl label namespace {{ .Release.Namespace }} kubevirt.io="";
kubectl label namespace {{ .Release.Namespace }} pod-security.kubernetes.io/enforce="privileged";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are labels which the upstream manifests are shipped with. Unfortunately, there wasn't an easier way to do this than a pre-install hook... helm/helm#3503 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

My only concern is that everything related to this is a leftover that will live in the cluster for no reason. Honestly I don't know how to fix it but maybe the container can remove "everything" after the namespace is labeled? I mean, running kubectl delete role kubevirt-namespace-modifier, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They won't live forever, the delete policy drops them all immediately.

root@atanas:~/charts/charts# helm install kubevirt kubevirt/0.2.0 -n kubevirt --create-namespace --debug
install.go:192: [debug] Original chart version: ""
install.go:209: [debug] CHART PATH: /root/charts/charts/kubevirt/0.2.0

client.go:128: [debug] creating 1 resource(s)
install.go:151: [debug] CRD kubevirts.kubevirt.io is already present. Skipping.
client.go:128: [debug] creating 1 resource(s)
client.go:128: [debug] creating 1 resource(s)
client.go:128: [debug] creating 1 resource(s)
client.go:128: [debug] creating 1 resource(s)
client.go:128: [debug] creating 1 resource(s)
client.go:540: [debug] Watching for changes to Job kubevirt-namespace-modifier with timeout of 5m0s
client.go:568: [debug] Add/Modify event for kubevirt-namespace-modifier: ADDED
client.go:607: [debug] kubevirt-namespace-modifier: Jobs active: 0, jobs failed: 0, jobs succeeded: 0
client.go:568: [debug] Add/Modify event for kubevirt-namespace-modifier: MODIFIED
client.go:607: [debug] kubevirt-namespace-modifier: Jobs active: 1, jobs failed: 0, jobs succeeded: 0
client.go:568: [debug] Add/Modify event for kubevirt-namespace-modifier: MODIFIED
client.go:607: [debug] kubevirt-namespace-modifier: Jobs active: 0, jobs failed: 0, jobs succeeded: 0
client.go:568: [debug] Add/Modify event for kubevirt-namespace-modifier: MODIFIED
client.go:310: [debug] Starting delete for "kubevirt-namespace-modifier" ServiceAccount
client.go:310: [debug] Starting delete for "kubevirt-namespace-modifier" Role
client.go:310: [debug] Starting delete for "kubevirt-namespace-modifier" RoleBinding
client.go:310: [debug] Starting delete for "kubevirt-namespace-modifier" Job

Copy link
Contributor

Choose a reason for hiding this comment

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

You can ignore my comment then :D Sorry for the noise.

@atanasdinov atanasdinov marked this pull request as ready for review December 11, 2023 17:41
Copy link
Contributor

@ipetrov117 ipetrov117 left a comment

Choose a reason for hiding this comment

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

Changes look solid, I have concerns about the security implications that come with labelling a full namespace as pod-security.kubernetes.io/enforce="privileged" though..
Taking into account that this is something that comes from the upstream manifests, I'll approve this.

@atanasdinov atanasdinov merged commit 9604303 into main Dec 12, 2023
1 check passed
@atanasdinov atanasdinov deleted the kubevirt-namespace branch December 12, 2023 09:38
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.

None yet

3 participants