Changes due to upgrade of klogs and controller-runtime

This commit includes several changes to webhook package due to bumping up controller
runtime deps and also makes changes to logs lines to remove formatter wherever it is not required.

Signed-off-by: vbadrina <[email protected]>

addons/agent_mirrorpeer_controller.go

@@ -117,7 +117,7 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	klog.Infof("creating s3 buckets")
-	err = r.createS3(ctx, req, mirrorPeer, scr.Namespace)
+	err = r.createS3(ctx, mirrorPeer, scr.Namespace)
 	if err != nil {
 		klog.Error(err, "Failed to create ODR S3 resources")
 		return ctrl.Result{}, err
@@ -158,8 +158,8 @@ func (r *MirrorPeerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 		// Trying this at last to allow bootstrapping to be completed
 		if mirrorPeer.Spec.OverlappingCIDR {
-			klog.Infof("enabling multiclusterservice", "MirrorPeer", mirrorPeer.GetName(), "Peers", mirrorPeer.Spec.Items)
-			err := r.enableMulticlusterService(ctx, scr.Name, scr.Namespace, &mirrorPeer)
+			klog.Info("enabling multiclusterservice", "MirrorPeer", mirrorPeer.GetName(), "Peers", mirrorPeer.Spec.Items)
+			err := r.enableMulticlusterService(ctx, scr.Name, scr.Namespace)
 			if err != nil {
 				return ctrl.Result{}, fmt.Errorf("failed to enable multiclusterservice for storagecluster %q in namespace %q: %v", scr.Name, scr.Namespace, err)
 			}
@@ -258,7 +258,7 @@ func (r *MirrorPeerReconciler) labelRBDStorageClasses(ctx context.Context, stora
 	return errs
 }
 
-func (r *MirrorPeerReconciler) createS3(ctx context.Context, req ctrl.Request, mirrorPeer multiclusterv1alpha1.MirrorPeer, scNamespace string) error {
+func (r *MirrorPeerReconciler) createS3(ctx context.Context, mirrorPeer multiclusterv1alpha1.MirrorPeer, scNamespace string) error {
 	noobaaOBC, err := r.getS3bucket(ctx, mirrorPeer, scNamespace)
 	if err != nil {
 		if errors.IsNotFound(err) {
@@ -303,7 +303,7 @@ func (r *MirrorPeerReconciler) getS3bucket(ctx context.Context, mirrorPeer multi
 }
 
 // enableMulticlusterService sets the multiclusterservice flag on StorageCluster if submariner globalnet is enabled
-func (r *MirrorPeerReconciler) enableMulticlusterService(ctx context.Context, storageClusterName string, namespace string, mp *multiclusterv1alpha1.MirrorPeer) error {
+func (r *MirrorPeerReconciler) enableMulticlusterService(ctx context.Context, storageClusterName string, namespace string) error {
 	klog.Infof("Enabling MCS for StorageCluster %q in %q namespace.", storageClusterName, namespace)
 	var sc ocsv1.StorageCluster
 	err := r.SpokeClient.Get(ctx, types.NamespacedName{

addons/blue_secret_controller.go

@@ -9,7 +9,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -48,7 +47,7 @@ func (r *BlueSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("bluesecret_controller").
-		Watches(&source.Kind{Type: &corev1.Secret{}}, &handler.EnqueueRequestForObject{},
+		Watches(&corev1.Secret{}, &handler.EnqueueRequestForObject{},
 			builder.WithPredicates(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{}, blueSecretPredicate)).
 		Complete(r)
 }
@@ -57,14 +56,14 @@ func (r *BlueSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	var err error
 	var secret corev1.Secret
 
-	klog.Infof("Reconciling blue secret", "secret", req.NamespacedName.String())
+	klog.Info("Reconciling blue secret", "secret", req.NamespacedName.String())
 	err = r.SpokeClient.Get(ctx, req.NamespacedName, &secret)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			klog.Infof("Could not find secret. Ignoring since it must have been deleted")
 			return ctrl.Result{}, nil
 		}
-		klog.Errorf("Failed to get secret.", err)
+		klog.Error("Failed to get secret.", err)
 		return ctrl.Result{}, err
 	}
 

addons/green_secret_controller.go

@@ -16,7 +16,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 // GreenSecretReconciler reconciles a MirrorPeer object
@@ -54,7 +53,7 @@ func (r *GreenSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("greensecret_controller").
-		Watches(&source.Kind{Type: &corev1.Secret{}}, &handler.EnqueueRequestForObject{},
+		Watches(&corev1.Secret{}, &handler.EnqueueRequestForObject{},
 			builder.WithPredicates(predicate.GenerationChangedPredicate{}, greenSecretPredicate)).
 		Complete(r)
 }
@@ -63,14 +62,14 @@ func (r *GreenSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	var err error
 	var greenSecret corev1.Secret
 
-	klog.Infof("Reconciling green secret", "secret", req.NamespacedName.String())
+	klog.Info("Reconciling green secret", "secret", req.NamespacedName.String())
 	err = r.HubClient.Get(ctx, req.NamespacedName, &greenSecret)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			klog.Infof("Could not find secret. Ignoring since it must have been deleted")
 			return ctrl.Result{}, nil
 		}
-		klog.Errorf("Failed to get secret.", err)
+		klog.Error("Failed to get secret.", err)
 		return ctrl.Result{}, err
 	}
 

addons/maintenance_mode_controller.go

@@ -22,7 +22,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 type MaintenanceModeReconciler struct {
@@ -34,7 +33,7 @@ type MaintenanceModeReconciler struct {
 func (r *MaintenanceModeReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("maintenancemode_controller").
-		Watches(&source.Kind{Type: &ramenv1alpha1.MaintenanceMode{}}, &handler.EnqueueRequestForObject{},
+		Watches(&ramenv1alpha1.MaintenanceMode{}, &handler.EnqueueRequestForObject{},
 			builder.WithPredicates(predicate.GenerationChangedPredicate{})).
 		Complete(r)
 }
@@ -49,7 +48,7 @@ func (r *MaintenanceModeReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 			klog.Infof("Could not find MaintenanceMode. Ignoring since object must have been deleted.")
 			return ctrl.Result{}, nil
 		}
-		klog.Errorf("Failed to get MaintenanceMode.", err)
+		klog.Error("Failed to get MaintenanceMode.", err)
 		return ctrl.Result{}, err
 	}
 
@@ -80,7 +79,7 @@ func (r *MaintenanceModeReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 			klog.Errorf("failed to complete maintenance actions on %s. err=%v", mmode.Name, err)
 			return result, err
 		}
-		if !result.Requeue && err == nil {
+		if !result.Requeue {
 			mmode.Finalizers = utils.RemoveString(mmode.Finalizers, MaintenanceModeFinalizer)
 			err = r.SpokeClient.Update(ctx, &mmode)
 			if err != nil {
@@ -136,7 +135,7 @@ func (r *MaintenanceModeReconciler) startMaintenanceActions(ctx context.Context,
 				SetStatus(mmode, mode, ramenv1alpha1.MModeStateError, err)
 				return result, err
 			}
-			if !result.Requeue && err == nil {
+			if !result.Requeue {
 				SetStatus(mmode, mode, ramenv1alpha1.MModeStateCompleted, nil)
 			}
 			return result, err

addons/manager.go

@@ -222,12 +222,12 @@ func runSpokeManager(ctx context.Context, options AddonAgentOptions) {
 	if err = mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
 		klog.Infof("Waiting for MaintenanceMode CRD to be created. MaintenanceMode controller is not running yet.")
 		// Wait for 45s as it takes time for MaintenanceMode CRD to be created.
-		return runtimewait.PollUntilWithContext(ctx, 15*time.Second,
+		return runtimewait.PollUntilContextCancel(ctx, 15*time.Second, true,
 			func(ctx context.Context) (done bool, err error) {
 				var crd extv1.CustomResourceDefinition
 				readErr := mgr.GetAPIReader().Get(ctx, types.NamespacedName{Name: "maintenancemodes.ramendr.openshift.io"}, &crd)
 				if readErr != nil {
-					klog.Errorf("Unable to get MaintenanceMode CRD", readErr)
+					klog.Error("Unable to get MaintenanceMode CRD", readErr)
 					// Do not initialize err as we want to retry.
 					// err!=nil or done==true will end polling.
 					done = false
@@ -239,7 +239,7 @@ func runSpokeManager(ctx context.Context, options AddonAgentOptions) {
 						SpokeClient:      mgr.GetClient(),
 						SpokeClusterName: options.SpokeClusterName,
 					}).SetupWithManager(mgr); err != nil {
-						klog.Errorf("Unable to create MaintenanceMode controller.", err)
+						klog.Error("Unable to create MaintenanceMode controller.", err)
 						return
 					}
 					klog.Infof("MaintenanceMode CRD exists. MaintenanceMode controller is now running.")
@@ -250,7 +250,7 @@ func runSpokeManager(ctx context.Context, options AddonAgentOptions) {
 				return
 			})
 	})); err != nil {
-		klog.Errorf("unable to poll MaintenanceMode", err)
+		klog.Error("unable to poll MaintenanceMode", err)
 		os.Exit(1)
 	}
 

addons/s3_controller.go

@@ -18,7 +18,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 // S3SecretReconciler reconciles a MirrorPeer object
@@ -59,7 +58,7 @@ func (r *S3SecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("s3secret_controller").
-		Watches(&source.Kind{Type: &obv1alpha1.ObjectBucketClaim{}}, &handler.EnqueueRequestForObject{},
+		Watches(&obv1alpha1.ObjectBucketClaim{}, &handler.EnqueueRequestForObject{},
 			builder.WithPredicates(predicate.GenerationChangedPredicate{}, s3BucketPredicate)).
 		Complete(r)
 }
@@ -68,14 +67,14 @@ func (r *S3SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	var err error
 	var obc obv1alpha1.ObjectBucketClaim
 
-	klog.Infof("Reconciling OBC", "OBC", req.NamespacedName.String())
+	klog.Info("Reconciling OBC", "OBC", req.NamespacedName.String())
 	err = r.SpokeClient.Get(ctx, req.NamespacedName, &obc)
 	if err != nil {
 		if errors.IsNotFound(err) {
-			klog.Infof("Could not find OBC. Ignoring since object must have been deleted.")
+			klog.Info("Could not find OBC. Ignoring since object must have been deleted.")
 			return ctrl.Result{}, nil
 		}
-		klog.Errorf("Failed to get OBC.", err)
+		klog.Error("Failed to get OBC.", err)
 		return ctrl.Result{}, err
 	}
 

addons/s3_secret_handler.go

@@ -55,7 +55,7 @@ func (r *S3SecretReconciler) syncBlueSecretForS3(ctx context.Context, name strin
 	}
 
 	if storageClusterRef == nil {
-		klog.Error("failed to find storage cluster ref using spoke cluster name %s from mirrorpeers ", r.SpokeClusterName)
+		klog.Errorf("failed to find storage cluster ref using spoke cluster name %s from mirrorpeers ", r.SpokeClusterName)
 		return err
 	}
 

api/v1alpha1/mirrorpeer_webhook.go

@@ -23,27 +23,15 @@ import (
 	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"
 )
 
 // log is for logging in this package.
 var mirrorpeerlog = logf.Log.WithName("mirrorpeer-webhook")
 
-const (
-	WebhookCertDir  = "/apiserver.local.config/certificates"
-	WebhookCertName = "apiserver.crt"
-	WebhookKeyName  = "apiserver.key"
-)
-
 func (r *MirrorPeer) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	bldr := ctrl.NewWebhookManagedBy(mgr).
-		For(r)
-
-	srv := mgr.GetWebhookServer()
-	srv.CertDir = WebhookCertDir
-	srv.CertName = WebhookCertName
-	srv.KeyName = WebhookKeyName
-
-	return bldr.Complete()
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(r).Complete()
 }
 
 //+kubebuilder:webhook:path=/mutate-multicluster-odf-openshift-io-v1alpha1-mirrorpeer,mutating=true,failurePolicy=fail,sideEffects=None,groups=multicluster.odf.openshift.io,resources=mirrorpeers,verbs=create;update,versions=v1alpha1,name=mmirrorpeer.kb.io,admissionReviewVersions=v1;v1beta1
@@ -58,26 +46,25 @@ func (r *MirrorPeer) Default() {}
 var _ webhook.Validator = &MirrorPeer{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (r *MirrorPeer) ValidateCreate() error {
+func (r *MirrorPeer) ValidateCreate() (warnings admission.Warnings, err error) {
 	mirrorpeerlog.Info("validate create", "name", r.ObjectMeta.Name)
-
-	return validateMirrorPeer(r)
+	return []string{}, validateMirrorPeer(r)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (r *MirrorPeer) ValidateUpdate(old runtime.Object) error {
+func (r *MirrorPeer) ValidateUpdate(old runtime.Object) (warnings admission.Warnings, err error) {
 	mirrorpeerlog.Info("validate update", "name", r.ObjectMeta.Name)
 	oldMirrorPeer, ok := old.(*MirrorPeer)
 	if !ok {
-		return fmt.Errorf("error casting old object to MirrorPeer")
+		return []string{}, fmt.Errorf("error casting old object to MirrorPeer")
 	}
 
 	if len(r.Spec.Items) != len(oldMirrorPeer.Spec.Items) {
-		return fmt.Errorf("error updating MirrorPeer, new and old spec.items have different lengths")
+		return []string{}, fmt.Errorf("error updating MirrorPeer, new and old spec.items have different lengths")
 	}
 
 	if r.Spec.Type != oldMirrorPeer.Spec.Type {
-		return fmt.Errorf("error updating MirrorPeer, the type cannot be changed from %s to %s", oldMirrorPeer.Spec.Type, r.Spec.Type)
+		return []string{}, fmt.Errorf("error updating MirrorPeer, the type cannot be changed from %s to %s", oldMirrorPeer.Spec.Type, r.Spec.Type)
 	}
 
 	refs := make(map[string]int)
@@ -89,14 +76,14 @@ func (r *MirrorPeer) ValidateUpdate(old runtime.Object) error {
 	for _, pr := range r.Spec.Items {
 		key := fmt.Sprintf("%s-%s-%s", pr.ClusterName, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)
 		if _, ok := refs[key]; !ok {
-			return fmt.Errorf("error validating update: new MirrorPeer %s references a StorageCluster %s/%s that is not in the old MirrorPeer", r.ObjectMeta.Name, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)
+			return []string{}, fmt.Errorf("error validating update: new MirrorPeer %s references a StorageCluster %s/%s that is not in the old MirrorPeer", r.ObjectMeta.Name, pr.StorageClusterRef.Namespace, pr.StorageClusterRef.Name)
 		}
 	}
 
 	if oldMirrorPeer.Spec.OverlappingCIDR && !r.Spec.OverlappingCIDR {
-		return fmt.Errorf("error updating MirrorPeer: OverlappingCIDR value can not be changed from %t to %t. This is to prevent Disaster Recovery from being unusable between clusters that have overlapping IPs", oldMirrorPeer.Spec.OverlappingCIDR, r.Spec.OverlappingCIDR)
+		return []string{}, fmt.Errorf("error updating MirrorPeer: OverlappingCIDR value can not be changed from %t to %t. This is to prevent Disaster Recovery from being unusable between clusters that have overlapping IPs", oldMirrorPeer.Spec.OverlappingCIDR, r.Spec.OverlappingCIDR)
 	}
-	return validateMirrorPeer(r)
+	return []string{}, validateMirrorPeer(r)
 }
 
 // validateMirrorPeer validates the MirrorPeer
@@ -108,7 +95,7 @@ func validateMirrorPeer(instance *MirrorPeer) error {
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (r *MirrorPeer) ValidateDelete() error {
+func (r *MirrorPeer) ValidateDelete() (warnings admission.Warnings, err error) {
 	mirrorpeerlog.Info("validate delete", "name", r.ObjectMeta.Name)
-	return nil
+	return []string{}, nil
 }

bundle/manifests/odf-multicluster-orchestrator.clusterserviceversion.yaml

@@ -36,7 +36,7 @@ metadata:
       ]
     capabilities: Basic Install
     console.openshift.io/plugins: '["odf-multicluster-console"]'
-    createdAt: "2024-03-20T12:51:52Z"
+    createdAt: "2024-03-21T07:54:52Z"
     olm.skipRange: ""
     operators.openshift.io/infrastructure-features: '["disconnected"]'
     operators.operatorframework.io/builder: operator-sdk-v1.34.1

controllers/drpolicy_controller.go

@@ -218,7 +218,7 @@ func (r *DRPolicyReconciler) fetchClusterFSIDs(ctx context.Context, peer *multic
 		hs, err := utils.FetchSecretWithName(ctx, r.HubClient, types.NamespacedName{Name: rookSecretName, Namespace: pr.ClusterName})
 		if err != nil {
 			if errors.IsNotFound(err) {
-				klog.Info("could not find secret %q. will attempt to fetch it again after a delay", rookSecretName)
+				klog.Infof("could not find secret %q. will attempt to fetch it again after a delay", rookSecretName)
 			}
 			return err
 		}

controllers/manager.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 var (
@@ -44,6 +45,12 @@ func init() {
 	//+kubebuilder:scaffold:scheme
 }
 
+const (
+	WebhookCertDir  = "/apiserver.local.config/certificates"
+	WebhookCertName = "apiserver.crt"
+	WebhookKeyName  = "apiserver.key"
+)
+
 type ManagerOptions struct {
 	MetricsAddr             string
 	EnableLeaderElection    bool
@@ -89,13 +96,20 @@ func (o *ManagerOptions) runManager() {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&o.ZapOpts)))
 	setupLog := ctrl.Log.WithName("setup")
 
+	srv := webhook.NewServer(webhook.Options{
+		CertDir:  WebhookCertDir,
+		CertName: WebhookCertName,
+		KeyName:  WebhookKeyName,
+	})
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:                 mgrScheme,
 		MetricsBindAddress:     o.MetricsAddr,
 		Port:                   9443,
 		HealthProbeBindAddress: o.ProbeAddr,
 		LeaderElection:         o.EnableLeaderElection,
 		LeaderElectionID:       "1d19c724.odf.openshift.io",
+		WebhookServer:          srv,
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")