-
Notifications
You must be signed in to change notification settings - Fork 48
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
🌱 Allow SRM to change network of a registered VM for test failover #803
🌱 Allow SRM to change network of a registered VM for test failover #803
Conversation
SRM (Site Reliability Manager) needs to be able to change a VM's network interface to connect to the test failover network. This change skips part of the validation webhook if a predefined test failover label is present on the VM resource. A test failover does not need to add new interfaces, so keep that check as is. Testing Done: - Existing unit tests as well as newly added tests - Manually tested by loading VM operator in an env Without the label: root@4214a043bbff661bcddbde32adee61b3 [ ~ ]# kubectl patch vm -n parunesh-ns srm-vm --type='json' -p='[{"op": "replace", "path": "/spec/network/interfaces/1/network/name", "value": "my-subnet-2"}]' Error from server (spec.network.interfaces[1].network: Forbidden: field is immutable): admission webhook "default.validating.virtualmachine.v1alpha3.vmoperator.vmware.com" denied the request: spec.network.interfaces[1].network: Forbidden: field is immutable Edit the VM to apply the label root@4214a043bbff661bcddbde32adee61b3 [ ~ ]# k edit vm -n parunesh-ns srm-vm virtualmachine.vmoperator.vmware.com/srm-vm edited Verify that edits are allowed root@4214a043bbff661bcddbde32adee61b3 [ ~ ]# kubectl patch vm -n parunesh-ns srm-vm --type='json' -p='[{"op": "replace", "path": "/spec/network/interfaces/1/network/name", "value": "my-subnet-2"}]' virtualmachine.vmoperator.vmware.com/srm-vm patched
Why a label and not annotation? Unless the intent is to find resources with this label, then please change to an annotation. |
Minimum allowed line rate is |
Andrew, the label here is intentional. This allows a user (admin) to identify the VMs that are test failed over. This can specially be helpful if we want to round up these VMs for cleanup after a test failover is complete. I will add this in the MR description for posterity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thank you sir!
What does this PR do, and why is it needed?
SRM (Site Reliability Manager) needs to be able to change a VM's network interface to connect to the test failover network. This change skips part of the validation webhook if a predefined test failover label is present on the VM resource. This change introduces a label (as opposed to an annotation) because we want to allow server side indexing so clients can filter and operate on all VMs that are part of a test failover. This will help with debuggability, and cleanup of test failed over VMs.
A test failover does not need to add new interfaces, so keep that check as is.
Testing Done
Are there any special notes for your reviewer:
This is needed specifically for SRM (and possibly other vendors exercising a test failover). As such, I have called out in the API docs that it is not an otherwise supported workflow.
Please add a release note if necessary: