Open iholder101 opened 1 week ago
To move the logic of validating the clone's source away from webhooks to the clone controller, you can refactor the validation logic from the VirtualMachineCloneAdmitter
into the clone controller. This involves creating a new validation function in the clone controller and calling it during the clone creation process.
Here is an example of how you can achieve this:
Extract Validation Logic: Move the validation functions from vmclone-admitter.go
to a new file or an existing file in the clone controller package.
Update Clone Controller: Modify the clone controller to call the new validation function during the clone creation process.
Create a new file pkg/virt-controller/clone/validation.go
and move the validation functions there:
package clone
import (
"context"
"fmt"
"strings"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sfield "k8s.io/apimachinery/pkg/util/validation/field"
"kubevirt.io/api/clone/v1alpha1"
"kubevirt.io/client-go/kubecli"
"kubevirt.io/kubevirt/pkg/network/link"
"kubevirt.io/kubevirt/pkg/storage/snapshot"
"kubevirt.io/kubevirt/pkg/virt-config"
)
const (
virtualMachineKind = "VirtualMachine"
virtualMachineSnapshotKind = "VirtualMachineSnapshot"
)
func ValidateClone(ctx context.Context, client kubecli.KubevirtClient, config *virtconfig.ClusterConfig, vmClone *v1alpha1.VirtualMachineClone) []metav1.StatusCause {
var causes []metav1.StatusCause
if newCauses := validateFilters(vmClone.Spec.AnnotationFilters, "spec.annotations"); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateFilters(vmClone.Spec.LabelFilters, "spec.labels"); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateFilters(vmClone.Spec.Template.AnnotationFilters, "spec.template.annotations"); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateFilters(vmClone.Spec.Template.LabelFilters, "spec.template.labels"); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateSourceAndTargetKind(vmClone); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateSource(ctx, client, vmClone); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateTarget(vmClone); newCauses != nil {
causes = append(causes, newCauses...)
}
if newCauses := validateNewMacAddresses(vmClone); newCauses != nil {
causes = append(causes, newCauses...)
}
return causes
}
// Include other validation functions here (validateFilters, validateSourceAndTargetKind, validateSource, validateTarget, validateNewMacAddresses, etc.)
In the clone controller, call the ValidateClone
function during the clone creation process. For example, in pkg/virt-controller/clone/clone_controller.go
:
package clone
import (
"context"
"fmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
"kubevirt.io/client-go/kubecli"
"kubevirt.io/kubevirt/pkg/virt-config"
"kubevirt.io/kubevirt/pkg/virt-controller/watch"
)
type CloneController struct {
clientset kubecli.KubevirtClient
queue workqueue.RateLimitingInterface
config *virtconfig.ClusterConfig
}
func (c *CloneController) processClone(ctx context.Context, key string) error {
// Fetch the VirtualMachineClone object
obj, exists, err := c.queue.GetByKey(key)
if err != nil {
return err
}
if !exists {
return nil
}
vmClone := obj.(*v1alpha1.VirtualMachineClone)
// Validate the clone
causes := ValidateClone(ctx, c.clientset, c.config, vmClone)
if len(causes) > 0 {
return fmt.Errorf("validation failed: %v", causes)
}
// Proceed with the clone creation process
// ...
return nil
}
// Other controller methods...
By moving the validation logic to the clone controller, you avoid potential races and enhance eventual consistency by ensuring that the validation is performed as part of the clone creation process [1][2].
To continue talking to Dosu, mention @dosu.
Currently, webhooks for clone make API calls to fetch the clone's source (e.g. Snapshot, VM) in order to perform validations upon it, such if it exists.
To avoid races and enhance the eventual consistency paradigm, move this logic away to the clone controller instead.
/sig code-quality