allow deletion of overlay nads even if kubeovn is disabled#251
allow deletion of overlay nads even if kubeovn is disabled#251Vicente-Cheng merged 2 commits intoharvester:masterfrom
Conversation
Signed-off-by: Renuka Devi Rajendran <renuka.rajendran@suse.com>
rrajendran17
force-pushed
the
overlay-nad
branch
from
6790a99 to
4cb70ba
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the NAD admission webhook to prevent creating overlay (Kube-OVN) NetworkAttachmentDefinitions when the Kube-OVN addon/Subnet CRD is not enabled, and adjusts delete behavior for overlay NADs.
Changes:
- Add a create-time guard that blocks creating Kube-OVN NADs when
subnetEnabledis false. - Modify overlay NAD delete-path error handling (currently still blocks deletion when
subnetEnabledis false). - Add unit tests covering create/delete behavior when
subnetEnabledis false.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/webhook/nad/validator.go | Adds a create-time check for Kube-OVN NADs and changes delete-path behavior/errors. |
| pkg/webhook/nad/validator_test.go | Adds new tests for overlay NAD create/delete when Subnet CRD (kubeovn) is not enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rrajendran17
changed the title
w13915984028
left a comment
w13915984028
left a comment
There was a problem hiding this comment.
One open question, thanks.
| } | ||
|
|
||
| //allow overlay nad creation only if subnet crd/kubeovn is enabled | ||
| if conf.IsKubeOVNCNI() { |
There was a problem hiding this comment.
check !v.subnetEnabled to allow deletion is fine.
for create case, need to double check if this will affect those automatic tests:
if nad is created but the related vendor is not ready, normally the vm's backend pod will not be created successful as kubelet does not get response from cni driver
we need to do a test:
create overlay-nad & VM before OVN is enabled; or nad is leftover when ovn is disabled (e.g. the upgrade case or on old version without this check): will the VM just stuck on starting;
then enable OVN, could the VM turn to normal running status.
cc @irishgordo @TachunLin @khushboo-rancher to check if some test scripts could be affected, thanks.
There was a problem hiding this comment.
yeah that is a good case.Though we cannot create VMs in custom subnets when ovn is disabled, but if we create an overlay network when ovn is disabled and attach a vm to overlay interface, it will be stuck but once the ovn is enabled, the VM will be able to get ip address from default subnet ovn-default and will move to running state.
With this observation, I think we should allow creation of overlay networks even if ovn is disabled. WDYT ?
There was a problem hiding this comment.
Hi @rrajendran17, @w13915984028,
Since I would like to have this on today's RC3, I will merge this PR first.
Could you create another issue to track the above case?
Thanks!
There was a problem hiding this comment.
We can have a follow-up issue to track this (for whatever we should allow users to pre-create overlay networks and their VMs, or the test plan adaptation).
|
@Mergifyio backport v1.8 |
✅ Backports have been createdDetails
|
Problem:
currently overlay nads can be created but cannot be deleted when kubeovn add-on is disabled.
Solution:
do not allow overlay nad creation when kubeovn is disabled and allow deletion of overlay nads even if kubeovn is disabled.
Related Issue:
harvester/harvester#10273
Test plan:
case 1:
1.Install Harvester v1.8.0
2.Kubeovn addon is not enabled.
3.Create VM Network with type overlay
4.webhook must return error
case 2:
1.Install Harvester v1.8.0
2.Kubeovn addon is enabled.
3.Create VM Network with type overlay
4.Create subnets, VMs
5.Delete VMs,subnets, disable kubeovn addon
6.Delete VM Network with type overlay
7.Deletion of overlay VM Network must be successful