Merge pull request #1906 from mythi/PR-2024-024

operator: rework webhooks

Author: tkatila

pkg/apis/deviceplugin/v1/dlbdeviceplugin_webhook.go

@@ -15,73 +15,35 @@
 package v1
 
 import (
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-var (
-	// dlbdevicepluginlog is for logging in this package.
-	dlbdevicepluginlog = logf.Log.WithName("dlbdeviceplugin-resource")
-
-	dlbMinVersion = controllers.ImageMinVersion
-)
-
 // SetupWebhookWithManager sets up a webhook for DlbDevicePlugin custom resources.
 func (r *DlbDevicePlugin) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithDefaulter(&commonDevicePluginDefaulter{
+			defaultImage: "intel/intel-dlb-plugin:" + controllers.ImageMinVersion.String(),
+		}).
+		WithValidator(&commonDevicePluginValidator{
+			expectedImage:     "intel-dlb-plugin",
+			expectedInitImage: "intel-dlb-initimage",
+			expectedVersion:   *controllers.ImageMinVersion,
+		}).
 		Complete()
 }
 
 // +kubebuilder:webhook:path=/mutate-deviceplugin-intel-com-v1-dlbdeviceplugin,mutating=true,failurePolicy=fail,groups=deviceplugin.intel.com,resources=dlbdeviceplugins,verbs=create;update,versions=v1,name=mdlbdeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
-
-var _ webhook.Defaulter = &DlbDevicePlugin{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (r *DlbDevicePlugin) Default() {
-	dlbdevicepluginlog.Info("default", "name", r.Name)
-
-	if len(r.Spec.Image) == 0 {
-		r.Spec.Image = "intel/intel-dlb-plugin:" + dlbMinVersion.String()
-	}
-}
-
 // +kubebuilder:webhook:verbs=create;update,path=/validate-deviceplugin-intel-com-v1-dlbdeviceplugin,mutating=false,failurePolicy=fail,groups=deviceplugin.intel.com,resources=dlbdeviceplugins,versions=v1,name=vdlbdeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
 
-var _ webhook.Validator = &DlbDevicePlugin{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *DlbDevicePlugin) ValidateCreate() (admission.Warnings, error) {
-	dlbdevicepluginlog.Info("validate create", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *DlbDevicePlugin) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	dlbdevicepluginlog.Info("validate update", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *DlbDevicePlugin) ValidateDelete() (admission.Warnings, error) {
-	dlbdevicepluginlog.Info("validate delete", "name", r.Name)
-
-	return nil, nil
-}
-
-func (r *DlbDevicePlugin) validatePlugin() error {
+func (r *DlbDevicePlugin) validatePlugin(ref *commonDevicePluginValidator) error {
 	if r.Spec.InitImage != "" {
-		if err := validatePluginImage(r.Spec.InitImage, "intel-dlb-initcontainer", dlbMinVersion); err != nil {
+		if err := validatePluginImage(r.Spec.InitImage, ref.expectedInitImage, &ref.expectedVersion); err != nil {
 			return err
 		}
 	}
 
-	return validatePluginImage(r.Spec.Image, "intel-dlb-plugin", dlbMinVersion)
+	return validatePluginImage(r.Spec.Image, ref.expectedImage, &ref.expectedVersion)
 }

pkg/apis/deviceplugin/v1/dsadeviceplugin_webhook.go

@@ -15,79 +15,42 @@
 package v1
 
 import (
-	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/runtime"
+	"fmt"
+
 	ctrl "sigs.k8s.io/controller-runtime"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-var (
-	// dsadevicepluginlog is for logging in this package.
-	dsadevicepluginlog = logf.Log.WithName("dsadeviceplugin-resource")
-
-	dsaMinVersion = controllers.ImageMinVersion
-)
-
 // SetupWebhookWithManager sets up a webhook for DsaDevicePlugin custom resources.
 func (r *DsaDevicePlugin) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithDefaulter(&commonDevicePluginDefaulter{
+			defaultImage: "intel/intel-dsa-plugin:" + controllers.ImageMinVersion.String(),
+		}).
+		WithValidator(&commonDevicePluginValidator{
+			expectedImage:     "intel-dsa-plugin",
+			expectedInitImage: "intel-idxd-config-initcontainer",
+			expectedVersion:   *controllers.ImageMinVersion,
+		}).
 		Complete()
 }
 
 // +kubebuilder:webhook:path=/mutate-deviceplugin-intel-com-v1-dsadeviceplugin,mutating=true,failurePolicy=fail,groups=deviceplugin.intel.com,resources=dsadeviceplugins,verbs=create;update,versions=v1,name=mdsadeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
-
-var _ webhook.Defaulter = &DsaDevicePlugin{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (r *DsaDevicePlugin) Default() {
-	dsadevicepluginlog.Info("default", "name", r.Name)
-
-	if len(r.Spec.Image) == 0 {
-		r.Spec.Image = "intel/intel-dsa-plugin:" + dsaMinVersion.String()
-	}
-}
-
 // +kubebuilder:webhook:verbs=create;update,path=/validate-deviceplugin-intel-com-v1-dsadeviceplugin,mutating=false,failurePolicy=fail,groups=deviceplugin.intel.com,resources=dsadeviceplugins,versions=v1,name=vdsadeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
 
-var _ webhook.Validator = &DsaDevicePlugin{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *DsaDevicePlugin) ValidateCreate() (admission.Warnings, error) {
-	dsadevicepluginlog.Info("validate create", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *DsaDevicePlugin) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	dsadevicepluginlog.Info("validate update", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *DsaDevicePlugin) ValidateDelete() (admission.Warnings, error) {
-	dsadevicepluginlog.Info("validate delete", "name", r.Name)
-
-	return nil, nil
-}
-
-func (r *DsaDevicePlugin) validatePlugin() error {
-	if err := validatePluginImage(r.Spec.Image, "intel-dsa-plugin", dsaMinVersion); err != nil {
+func (r *DsaDevicePlugin) validatePlugin(ref *commonDevicePluginValidator) error {
+	if err := validatePluginImage(r.Spec.Image, ref.expectedImage, &ref.expectedVersion); err != nil {
 		return err
 	}
 
 	if len(r.Spec.ProvisioningConfig) > 0 && len(r.Spec.InitImage) == 0 {
-		return errors.Errorf("ProvisioningConfig is set with no InitImage")
+		return fmt.Errorf("%w: ProvisioningConfig is set with no InitImage", errValidation)
 	}
 
 	if len(r.Spec.InitImage) > 0 {
-		return validatePluginImage(r.Spec.InitImage, "intel-idxd-config-initcontainer", dsaMinVersion)
+		return validatePluginImage(r.Spec.InitImage, ref.expectedInitImage, &ref.expectedVersion)
 	}
 
 	return nil

pkg/apis/deviceplugin/v1/fpgadeviceplugin_webhook.go

@@ -15,75 +15,33 @@
 package v1
 
 import (
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-var (
-	// fpgadevicepluginlog is for logging in this package.
-	fpgadevicepluginlog = logf.Log.WithName("fpgadeviceplugin-resource")
-
-	fpgaMinVersion = controllers.ImageMinVersion
-)
-
 // SetupWebhookWithManager sets up a webhook for FpgaDevicePlugin custom resources.
 func (r *FpgaDevicePlugin) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithDefaulter(&commonDevicePluginDefaulter{
+			defaultImage: "intel/intel-fpga-plugin:" + controllers.ImageMinVersion.String(),
+		}).
+		WithValidator(&commonDevicePluginValidator{
+			expectedImage:     "intel-fpga-plugin",
+			expectedInitImage: "intel-fpga-initimage",
+			expectedVersion:   *controllers.ImageMinVersion,
+		}).
 		Complete()
 }
 
 // +kubebuilder:webhook:path=/mutate-deviceplugin-intel-com-v1-fpgadeviceplugin,mutating=true,failurePolicy=fail,groups=deviceplugin.intel.com,resources=fpgadeviceplugins,verbs=create;update,versions=v1,name=mfpgadeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
-
-var _ webhook.Defaulter = &FpgaDevicePlugin{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (r *FpgaDevicePlugin) Default() {
-	fpgadevicepluginlog.Info("default", "name", r.Name)
-
-	if len(r.Spec.Image) == 0 {
-		r.Spec.Image = "intel/intel-fpga-plugin:" + fpgaMinVersion.String()
-	}
-
-	if len(r.Spec.InitImage) == 0 {
-		r.Spec.InitImage = "intel/intel-fpga-initcontainer:" + fpgaMinVersion.String()
-	}
-}
-
 // +kubebuilder:webhook:verbs=create;update,path=/validate-deviceplugin-intel-com-v1-fpgadeviceplugin,mutating=false,failurePolicy=fail,groups=deviceplugin.intel.com,resources=fpgadeviceplugins,versions=v1,name=vfpgadeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
 
-var _ webhook.Validator = &FpgaDevicePlugin{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *FpgaDevicePlugin) ValidateCreate() (admission.Warnings, error) {
-	fpgadevicepluginlog.Info("validate create", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *FpgaDevicePlugin) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	fpgadevicepluginlog.Info("validate update", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *FpgaDevicePlugin) ValidateDelete() (admission.Warnings, error) {
-	fpgadevicepluginlog.Info("validate delete", "name", r.Name)
-
-	return nil, nil
-}
-
-func (r *FpgaDevicePlugin) validatePlugin() error {
-	if err := validatePluginImage(r.Spec.Image, "intel-fpga-plugin", fpgaMinVersion); err != nil {
+func (r *FpgaDevicePlugin) validatePlugin(ref *commonDevicePluginValidator) error {
+	if err := validatePluginImage(r.Spec.Image, ref.expectedImage, &ref.expectedVersion); err != nil {
 		return err
 	}
 
-	return validatePluginImage(r.Spec.InitImage, "intel-fpga-initcontainer", fpgaMinVersion)
+	return validatePluginImage(r.Spec.InitImage, ref.expectedInitImage, &ref.expectedVersion)
 }

pkg/apis/deviceplugin/v1/gpudeviceplugin_webhook.go

@@ -16,25 +16,15 @@ package v1
 
 import (
 	"context"
+	"fmt"
 
-	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-var (
-	// gpudevicepluginlog is for logging in this package.
-	gpudevicepluginlog = logf.Log.WithName("gpudeviceplugin-resource")
-
-	gpuMinVersion = controllers.ImageMinVersion
-)
-
 var cli client.Client
 
 // SetupWebhookWithManager sets up a webhook for GpuDevicePlugin custom resources.
@@ -43,53 +33,25 @@ func (r *GpuDevicePlugin) SetupWebhookWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
+		WithDefaulter(&commonDevicePluginDefaulter{
+			defaultImage: "intel/intel-gpu-plugin:" + controllers.ImageMinVersion.String(),
+		}).
+		WithValidator(&commonDevicePluginValidator{
+			expectedImage:   "intel-gpu-plugin",
+			expectedVersion: *controllers.ImageMinVersion,
+		}).
 		Complete()
 }
 
 // +kubebuilder:webhook:path=/mutate-deviceplugin-intel-com-v1-gpudeviceplugin,mutating=true,failurePolicy=fail,groups=deviceplugin.intel.com,resources=gpudeviceplugins,verbs=create;update,versions=v1,name=mgpudeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
-
-var _ webhook.Defaulter = &GpuDevicePlugin{}
-
-// Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (r *GpuDevicePlugin) Default() {
-	gpudevicepluginlog.Info("default", "name", r.Name)
-
-	if len(r.Spec.Image) == 0 {
-		r.Spec.Image = "intel/intel-gpu-plugin:" + gpuMinVersion.String()
-	}
-}
-
 // +kubebuilder:webhook:verbs=create;update,path=/validate-deviceplugin-intel-com-v1-gpudeviceplugin,mutating=false,failurePolicy=fail,groups=deviceplugin.intel.com,resources=gpudeviceplugins,versions=v1,name=vgpudeviceplugin.kb.io,sideEffects=None,admissionReviewVersions=v1
 
-var _ webhook.Validator = &GpuDevicePlugin{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *GpuDevicePlugin) ValidateCreate() (admission.Warnings, error) {
-	gpudevicepluginlog.Info("validate create", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *GpuDevicePlugin) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	gpudevicepluginlog.Info("validate update", "name", r.Name)
-
-	return nil, r.validatePlugin()
-}
-
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *GpuDevicePlugin) ValidateDelete() (admission.Warnings, error) {
-	gpudevicepluginlog.Info("validate delete", "name", r.Name)
-
-	return nil, nil
-}
-
-func (r *GpuDevicePlugin) crossCheckResourceManagement() bool {
-	ctx := context.Background()
+func (r *GpuDevicePlugin) crossCheckResourceManagement(ctx context.Context) bool {
+	log := logf.FromContext(ctx)
 	gpuCrs := GpuDevicePluginList{}
 
 	if err := cli.List(ctx, &gpuCrs); err != nil {
-		gpudevicepluginlog.Info("unable to list GPU CRs")
+		log.Info("unable to list GPU CRs")
 
 		return false
 	}
@@ -108,18 +70,18 @@ func (r *GpuDevicePlugin) crossCheckResourceManagement() bool {
 	return true
 }
 
-func (r *GpuDevicePlugin) validatePlugin() error {
+func (r *GpuDevicePlugin) validatePlugin(ctx context.Context, ref *commonDevicePluginValidator) error {
 	if r.Spec.SharedDevNum == 1 && r.Spec.PreferredAllocationPolicy != "none" {
-		return errors.Errorf("PreferredAllocationPolicy is valid only when setting sharedDevNum > 1")
+		return fmt.Errorf("%w: PreferredAllocationPolicy is valid only when setting sharedDevNum > 1", errValidation)
 	}
 
 	if r.Spec.SharedDevNum == 1 && r.Spec.ResourceManager {
-		return errors.Errorf("resourceManager is valid only when setting sharedDevNum > 1")
+		return fmt.Errorf("%w: resourceManager is valid only when setting sharedDevNum > 1", errValidation)
 	}
 
-	if !r.crossCheckResourceManagement() {
-		return errors.Errorf("All GPU CRs must be with or without resource management")
+	if !r.crossCheckResourceManagement(ctx) {
+		return fmt.Errorf("%w: All GPU CRs must be with or without resource management", errValidation)
 	}
 
-	return validatePluginImage(r.Spec.Image, "intel-gpu-plugin", gpuMinVersion)
+	return validatePluginImage(r.Spec.Image, ref.expectedImage, &ref.expectedVersion)
 }