Skip to content

Commit

Permalink
feat: remove validation of virtual machine objects for simplicity
Browse files Browse the repository at this point in the history
Signed-off-by: Dustin Scott <[email protected]>
  • Loading branch information
scottd018 committed Nov 27, 2024
1 parent f78669d commit 293e624
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 18 deletions.
2 changes: 1 addition & 1 deletion manifests/deploy/deploy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ webhooks:
resources:
# TODO: correct logic if we ever need to account for both virtual machines and virtual machine instances. For now
# we are only counting virtual machine instances.
- "virtualmachines"
#- "virtualmachines"
- "virtualmachineinstances"
admissionReviewVersions:
- "v1"
Expand Down
2 changes: 1 addition & 1 deletion manifests/test/rosa-vm-3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ spec:
utc: {}
cpu:
cores: 1
sockets: 32
sockets: 64
threads: 1
devices:
disks:
Expand Down
36 changes: 22 additions & 14 deletions resources/virtualmachineinstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,20 @@ func (vmi virtualMachineInstance) Extract(admissionRequest *admissionv1.Admissio

// NeedsValidation returns if a virtual machine instance object needs validation or not.
func (vmi virtualMachineInstance) NeedsValidation() *WindowsValidationResult {
// if we have owner references, see if we are owned by a virtual machine
if len(vmi.GetOwnerReferences()) > 0 {
for _, ref := range vmi.GetOwnerReferences() {
// we only want to validate virtual machine instances that do not have a windows preference. this is
// because provisioning from a preference seems to have a specialized workflow that is difficult
// to determine for windows.
// TODO: this logic is likely to need adjusted.
if ref.Name == VirtualMachineType {
return &WindowsValidationResult{Reason: "virtual machine instance owned by virtual machine object"}
}
}
}
// TODO: correct logic if we ever need to account for both virtual machines and virtual machine instances. For now
// we are only counting virtual machine instances.
// // if we have owner references, see if we are owned by a virtual machine
// if len(vmi.GetOwnerReferences()) > 0 {
// for _, ref := range vmi.GetOwnerReferences() {
// // we only want to validate virtual machine instances that do not have a windows preference. this is
// // because provisioning from a preference seems to have a specialized workflow that is difficult
// // to determine for windows.
// // TODO: this logic is likely to need adjusted.
// if ref.Name == VirtualMachineType {
// return &WindowsValidationResult{Reason: "virtual machine instance owned by virtual machine object"}
// }
// }
// }

// finally use the windows logic to determine if we need validation
return vmi.isWindows()
Expand Down Expand Up @@ -159,11 +161,17 @@ func (vmi virtualMachineInstance) hasWindowsPreference() *WindowsValidationResul
}

if annotations["vm.kubevirt.io/os"] == "windows" {
return &WindowsValidationResult{NeedsValidation: true, Reason: "has 'vm.kubevirt.io/os' windows annotation"}
return &WindowsValidationResult{
NeedsValidation: true,
Reason: "has 'vm.kubevirt.io/os' windows annotation",
}
}

if strings.HasPrefix(annotations["kubevirt.io/cluster-preference-name"], "windows") {
return &WindowsValidationResult{NeedsValidation: true, Reason: "has 'kubevirt.io/cluster-preference-name' windows annotation"}
return &WindowsValidationResult{
NeedsValidation: true,
Reason: "has 'kubevirt.io/cluster-preference-name' windows annotation",
}
}

return &WindowsValidationResult{Reason: "has no windows preference"}
Expand Down
4 changes: 2 additions & 2 deletions webhook/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ func NewOperation(w http.ResponseWriter, r *http.Request) (*operation, error) {
// we are only counting virtual machine instances.
var validator resources.WindowsInstanceValidator
switch req.admissionRequest.Kind.Kind {
case resources.VirtualMachineType:
validator = resources.NewVirtualMachine()
// case resources.VirtualMachineType:
// validator = resources.NewVirtualMachine()
case resources.VirtualMachineInstanceType:
validator = resources.NewVirtualMachineInstance()
default:
Expand Down

0 comments on commit 293e624

Please sign in to comment.