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

🌱 Allow imported and registered VMs to skip image validation checks on Create/Update #814

Conversation

aruneshpa
Copy link
Contributor

What does this PR do, and why is it needed?

VMs that are imported, or registered already have the virtual machine created on the infrastructure. As such, we don't need to apply the same strict validations for them such as making sure the image is not null / valid. To that end, this change introduces an annotation that will be applied on the Imported, or Registered VMs. This will allow our webhooks to skip these checks if the annotation is present.

For now, maintain the existing mechanism which allows all VM creations to be imageless if the import or the incremental restore feature is enabled. Once this change is merged, we will yank out that code to make that check stricter (only annotation based).

Are there any special notes for your reviewer:

There;s also a change to the UT method that validates messages. Instead of splitting that into a string array and matching elements, we are now comparing substrings. This is because the comma delimiter used to split the reasons results in false splits when a reason contains an object.

Please add a release note if necessary:

Allow imported and registered VMs to skip image validation checks on Create/Update

@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines. label Dec 5, 2024
Copy link
Collaborator

@akutz akutz left a comment

Choose a reason for hiding this comment

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

Looks good; left a few comments.

api/v1alpha3/virtualmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha3/virtualmachine_types.go Outdated Show resolved Hide resolved
api/v1alpha3/virtualmachine_types.go Outdated Show resolved Hide resolved
@aruneshpa aruneshpa force-pushed the feature/relax-webhook-checks-for-import-and-registervm branch from 4d5df56 to 0d43914 Compare December 6, 2024 22:55
Copy link
Collaborator

@akutz akutz left a comment

Choose a reason for hiding this comment

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

Left a few comments. Also, where are we restricting setting the new annotations to privileged users?

@aruneshpa aruneshpa force-pushed the feature/relax-webhook-checks-for-import-and-registervm branch from 0d43914 to 24e7329 Compare December 9, 2024 23:03
@github-actions github-actions bot added size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Dec 9, 2024
Copy link
Collaborator

@akutz akutz left a comment

Choose a reason for hiding this comment

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

Thanks for your patience Arunesh!

…create/update

VMs that are imported, or registered already have the virtual machine
created on the infrastructure.  As such, we don't need to apply the
same strict validations for them such as making sure the image is not
null / valid.  To that end, this change introduces annotations that
will be applied on the Imported, and Registered VMs.  This will allow
our webhooks to skip these checks if the annotation is present.

For now, maintain the existing mechanism which allows _all_ VM
creations to be imageless if the import or the incremental restore
feature is enabled.  Once this change is merged, we will yank out that
code to make that check stricter (only annotation based).
@aruneshpa aruneshpa force-pushed the feature/relax-webhook-checks-for-import-and-registervm branch from 24e7329 to d1b3d66 Compare December 9, 2024 23:24
Copy link

github-actions bot commented Dec 9, 2024

Code Coverage

Package Line Rate Health
github.com/vmware-tanzu/vm-operator/controllers/contentlibrary/clustercontentlibraryitem 82%
github.com/vmware-tanzu/vm-operator/controllers/contentlibrary/contentlibraryitem 86%
github.com/vmware-tanzu/vm-operator/controllers/contentlibrary/utils 97%
github.com/vmware-tanzu/vm-operator/controllers/infra/capability/configmap 86%
github.com/vmware-tanzu/vm-operator/controllers/infra/capability/crd 93%
github.com/vmware-tanzu/vm-operator/controllers/infra/configmap 71%
github.com/vmware-tanzu/vm-operator/controllers/infra/node 77%
github.com/vmware-tanzu/vm-operator/controllers/infra/secret 77%
github.com/vmware-tanzu/vm-operator/controllers/infra/validatingwebhookconfiguration 85%
github.com/vmware-tanzu/vm-operator/controllers/infra/zone 76%
github.com/vmware-tanzu/vm-operator/controllers/storageclass 95%
github.com/vmware-tanzu/vm-operator/controllers/storagepolicyquota 97%
github.com/vmware-tanzu/vm-operator/controllers/util/encoding 73%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachine/storagepolicyusage 99%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachine/virtualmachine 87%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachine/volume 87%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachineclass 75%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinepublishrequest 81%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinereplicaset 68%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachineservice 83%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachineservice/providers 92%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinesetresourcepolicy 80%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinewebconsolerequest/v1alpha1 72%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinewebconsolerequest/v1alpha1/conditions 88%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinewebconsolerequest/v1alpha1/patch 78%
github.com/vmware-tanzu/vm-operator/controllers/virtualmachinewebconsolerequest/v1alpha2 72%
github.com/vmware-tanzu/vm-operator/pkg/bitmask 100%
github.com/vmware-tanzu/vm-operator/pkg/builder 95%
github.com/vmware-tanzu/vm-operator/pkg/conditions 88%
github.com/vmware-tanzu/vm-operator/pkg/config 100%
github.com/vmware-tanzu/vm-operator/pkg/config/capabilities 100%
github.com/vmware-tanzu/vm-operator/pkg/config/env 100%
github.com/vmware-tanzu/vm-operator/pkg/context/generic 100%
github.com/vmware-tanzu/vm-operator/pkg/context/operation 100%
github.com/vmware-tanzu/vm-operator/pkg/patch 78%
github.com/vmware-tanzu/vm-operator/pkg/prober 91%
github.com/vmware-tanzu/vm-operator/pkg/prober/probe 90%
github.com/vmware-tanzu/vm-operator/pkg/prober/worker 77%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere 75%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/client 80%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/clustermodules 71%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/config 89%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/contentlibrary 74%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/credentials 100%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/network 80%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/placement 79%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/session 71%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/storage 44%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/sysprep 100%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vcenter 82%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/virtualmachine 84%
github.com/vmware-tanzu/vm-operator/pkg/providers/vsphere/vmlifecycle 69%
github.com/vmware-tanzu/vm-operator/pkg/record 87%
github.com/vmware-tanzu/vm-operator/pkg/topology 91%
github.com/vmware-tanzu/vm-operator/pkg/util 87%
github.com/vmware-tanzu/vm-operator/pkg/util/annotations 100%
github.com/vmware-tanzu/vm-operator/pkg/util/cloudinit 89%
github.com/vmware-tanzu/vm-operator/pkg/util/cloudinit/validate 91%
github.com/vmware-tanzu/vm-operator/pkg/util/image 100%
github.com/vmware-tanzu/vm-operator/pkg/util/kube 89%
github.com/vmware-tanzu/vm-operator/pkg/util/kube/cource 100%
github.com/vmware-tanzu/vm-operator/pkg/util/kube/internal 100%
github.com/vmware-tanzu/vm-operator/pkg/util/kube/proxyaddr 75%
github.com/vmware-tanzu/vm-operator/pkg/util/kube/spq 100%
github.com/vmware-tanzu/vm-operator/pkg/util/netplan 100%
github.com/vmware-tanzu/vm-operator/pkg/util/ovfcache 75%
github.com/vmware-tanzu/vm-operator/pkg/util/ovfcache/internal 100%
github.com/vmware-tanzu/vm-operator/pkg/util/paused 100%
github.com/vmware-tanzu/vm-operator/pkg/util/ptr 100%
github.com/vmware-tanzu/vm-operator/pkg/util/resize 97%
github.com/vmware-tanzu/vm-operator/pkg/util/vmopv1 93%
github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/client 64%
github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/vm 79%
github.com/vmware-tanzu/vm-operator/pkg/util/vsphere/watcher 87%
github.com/vmware-tanzu/vm-operator/pkg/vmconfig 95%
github.com/vmware-tanzu/vm-operator/pkg/vmconfig/crypto 98%
github.com/vmware-tanzu/vm-operator/pkg/webconsolevalidation 100%
github.com/vmware-tanzu/vm-operator/services/vm-watcher 92%
github.com/vmware-tanzu/vm-operator/webhooks/common 100%
github.com/vmware-tanzu/vm-operator/webhooks/persistentvolumeclaim/validation 95%
github.com/vmware-tanzu/vm-operator/webhooks/unifiedstoragequota/validation 89%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/mutation 87%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachine/validation 95%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/mutation 62%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineclass/validation 89%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinepublishrequest/validation 92%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinereplicaset/validation 90%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/mutation 67%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachineservice/validation 92%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinesetresourcepolicy/validation 89%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinewebconsolerequest/v1alpha1/validation 92%
github.com/vmware-tanzu/vm-operator/webhooks/virtualmachinewebconsolerequest/v1alpha2/validation 92%
Summary 83% (10710 / 12845)

Minimum allowed line rate is 79%

@aruneshpa aruneshpa merged commit c5fecaf into vmware-tanzu:main Dec 9, 2024
9 checks passed
@aruneshpa aruneshpa deleted the feature/relax-webhook-checks-for-import-and-registervm branch December 9, 2024 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-not-required size/XL Denotes a PR that changes 500-999 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants