chore: controller runtime upgrade fixes

Signed-off-by: ashish <asnaraya@redhat.com>
openshift · Sep 18, 2024 · 1ed29d3 · 1ed29d3
1 parent dbb5ab0
commit 1ed29d3
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 37 deletions.
diff --git a/cmd/addon-operator-webhook/main.go b/cmd/addon-operator-webhook/main.go
@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -76,7 +77,7 @@ func main() {
 		Client: mgr.GetClient(),
 	}
 
-	if err = wbHandler.InjectDecoder(admission.NewDecoder(mgr.GetScheme())); err != nil {
+	if err = wbHandler.InjectDecoder(ptr.To(admission.NewDecoder(mgr.GetScheme()))); err != nil {
 		setupLog.Error(err, "unable to inject decoder")
 		os.Exit(1)
 	}

diff --git a/controllers/addon/addon_controller.go b/controllers/addon/addon_controller.go
@@ -318,16 +318,20 @@ func (r *AddonReconciler) SetupWithManager(mgr ctrl.Manager, opts ...AddonReconc
 		Owns(&operatorsv1alpha1.Subscription{}).
 		Owns(&addonsv1alpha1.AddonInstance{}).
 		Owns(&monitoringv1.ServiceMonitor{}).
-		WatchesRawSource(source.Kind(
-			mgr.GetCache(), &corev1.Secret{}),
-			handler.EnqueueRequestForOwner(r.Scheme, mgr.GetRESTMapper(), &addonsv1alpha1.Addon{})).
-		WatchesRawSource(source.Kind(
-			mgr.GetCache(), &operatorsv1.Operator{}),
-			r.operatorResourceHandler, builder.OnlyMetadata,
+		Watches(&corev1.Secret{},
+			handler.EnqueueRequestForOwner(
+				mgr.GetScheme(),
+				mgr.GetRESTMapper(),
+				&addonsv1alpha1.Addon{},
+			),
 		).
-		WatchesRawSource(&source.Channel{ // Requeue everything when entering/leaving global pause.
-			Source: r.addonRequeueCh,
-		}, &handler.EnqueueRequestForObject{})
+		Watches(&operatorsv1.Operator{}, r.operatorResourceHandler, builder.OnlyMetadata).
+		WatchesRawSource(
+			source.Channel(
+				r.addonRequeueCh,
+				&handler.EnqueueRequestForObject{},
+			),
+		)
 
 	for _, opt := range opts {
 		opt.ApplyToControllerBuilder(adoControllerBuilder)

diff --git a/controllers/addon/handler/operator_resource_eventhandler.go b/controllers/addon/handler/operator_resource_eventhandler.go
@@ -27,27 +27,27 @@ func NewOperatorResourceHandler() *OperatorResourceHandler {
 }
 
 // Create is called in response to an create event.
-func (h *OperatorResourceHandler) Create(_ context.Context, evt event.CreateEvent, q workqueue.RateLimitingInterface) {
+func (h *OperatorResourceHandler) Create(_ context.Context, evt event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	h.enqueueObject(evt.Object, q)
 }
 
 // Update is called in response to an update event.
-func (h *OperatorResourceHandler) Update(_ context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) {
+func (h *OperatorResourceHandler) Update(_ context.Context, evt event.UpdateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	h.enqueueObject(evt.ObjectNew, q)
 }
 
 // Delete is called in response to a delete event.
-func (h *OperatorResourceHandler) Delete(_ context.Context, evt event.DeleteEvent, q workqueue.RateLimitingInterface) {
+func (h *OperatorResourceHandler) Delete(_ context.Context, evt event.DeleteEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	h.enqueueObject(evt.Object, q)
 }
 
 // Generic is called in response to an event of an unknown type or a synthetic event triggered as a cron or
 // external trigger request - e.g. reconcile Autoscaling, or a Webhook.
-func (h *OperatorResourceHandler) Generic(_ context.Context, evt event.GenericEvent, q workqueue.RateLimitingInterface) {
+func (h *OperatorResourceHandler) Generic(_ context.Context, evt event.GenericEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	h.enqueueObject(evt.Object, q)
 }
 
-func (h *OperatorResourceHandler) enqueueObject(obj client.Object, q workqueue.RateLimitingInterface) {
+func (h *OperatorResourceHandler) enqueueObject(obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	h.mux.RLock()
 	defer h.mux.RUnlock()
 

diff --git a/controllers/addon/monitoring_federation_reconciler_test.go b/controllers/addon/monitoring_federation_reconciler_test.go
@@ -12,6 +12,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -342,10 +343,10 @@ func TestEnsureMonitoringFederation_MonitoringPresentInSpec_SMPresentInCluster(t
 						TLSConfig: &monitoringv1.TLSConfig{
 							CAFile: "/etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt",
 							SafeTLSConfig: monitoringv1.SafeTLSConfig{
-								ServerName: fmt.Sprintf(
+								ServerName: ptr.To(fmt.Sprintf(
 									"prometheus.%s.svc",
 									addon.Spec.Monitoring.Federation.Namespace,
-								),
+								)),
 							},
 						},
 					},

diff --git a/controllers/addon/monitoring_stack_reconciler_test.go b/controllers/addon/monitoring_stack_reconciler_test.go
@@ -248,11 +248,11 @@ func TestGetWriteRelabelConfigFromAllowlist(t *testing.T) {
 	expectedResult := []monv1.RelabelConfig{
 		{
 			SourceLabels: []monv1.LabelName{"[__name__]"},
-			Separator:    "",
+			Separator:    nil,
 			TargetLabel:  "",
 			Regex:        "(cpu_usage|memory_usage|disk_space_used)",
 			Modulus:      0,
-			Replacement:  "",
+			Replacement:  nil,
 			Action:       "keep",
 		},
 	}

diff --git a/controllers/addon/utils.go b/controllers/addon/utils.go
@@ -16,6 +16,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/utils/ptr"
 	pkov1alpha1 "package-operator.run/apis/core/v1alpha1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -495,7 +496,7 @@ func GetMonitoringFederationServiceMonitorEndpoints(addon *addonsv1alpha1.Addon,
 	tlsConfig := &monitoringv1.TLSConfig{
 		CAFile: cacert,
 		SafeTLSConfig: monitoringv1.SafeTLSConfig{
-			ServerName: fmt.Sprintf("prometheus.%s.svc", addon.Spec.Monitoring.Federation.Namespace),
+			ServerName: ptr.To(fmt.Sprintf("prometheus.%s.svc", addon.Spec.Monitoring.Federation.Namespace)),
 		},
 	}
 

diff --git a/controllers/addon/utils_test.go b/controllers/addon/utils_test.go
@@ -14,6 +14,7 @@ import (
 	"k8s.io/client-go/util/workqueue"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	addonsv1alpha1 "github.com/openshift/addon-operator/api/v1alpha1"
 	"github.com/openshift/addon-operator/controllers"
@@ -110,23 +111,23 @@ type operatorResourceHandlerMock struct {
 var _ operatorResourceHandler = (*operatorResourceHandlerMock)(nil)
 
 // Create is called in response to an create event - e.g. Pod Creation.
-func (m *operatorResourceHandlerMock) Create(_ context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) {
+func (m *operatorResourceHandlerMock) Create(_ context.Context, e event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	m.Called(e, q)
 }
 
 // Update is called in response to an update event -  e.g. Pod Updated.
-func (m *operatorResourceHandlerMock) Update(_ context.Context, e event.UpdateEvent, q workqueue.RateLimitingInterface) {
+func (m *operatorResourceHandlerMock) Update(_ context.Context, e event.UpdateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	m.Called(e, q)
 }
 
 // Delete is called in response to a delete event - e.g. Pod Deleted.
-func (m *operatorResourceHandlerMock) Delete(_ context.Context, e event.DeleteEvent, q workqueue.RateLimitingInterface) {
+func (m *operatorResourceHandlerMock) Delete(_ context.Context, e event.DeleteEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	m.Called(e, q)
 }
 
 // Generic is called in response to an event of an unknown type or a synthetic event triggered as a cron or
 // external trigger request - e.g. reconcile Autoscaling, or a Webhook.
-func (m *operatorResourceHandlerMock) Generic(_ context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) {
+func (m *operatorResourceHandlerMock) Generic(_ context.Context, e event.GenericEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	m.Called(e, q)
 }
 

diff --git a/controllers/addonoperator/addonoperator_controller.go b/controllers/addonoperator/addonoperator_controller.go
@@ -26,7 +26,6 @@ import (
 	"github.com/openshift/addon-operator/internal/ocm"
 
 	"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/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -52,12 +51,11 @@ func (r *AddonOperatorReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&addonsv1alpha1.AddonOperator{}).
 		WithEventFilter(predicate.GenerationChangedPredicate{}).
-		WatchesRawSource(source.Func(enqueueAddonOperator), &handler.EnqueueRequestForObject{}).
+		WatchesRawSource(source.Func(enqueueAddonOperator)).
 		Complete(r)
 }
 
-func enqueueAddonOperator(ctx context.Context, h handler.EventHandler,
-	q workqueue.RateLimitingInterface, p ...predicate.Predicate) error {
+func enqueueAddonOperator(ctx context.Context, q workqueue.TypedRateLimitingInterface[reconcile.Request]) error {
 	q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
 		Name: addonsv1alpha1.DefaultAddonOperatorName,
 	}})

diff --git a/controllers/addonoperator/addonoperator_controller_test.go b/controllers/addonoperator/addonoperator_controller_test.go
@@ -11,7 +11,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/workqueue"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	addonsv1alpha1 "github.com/openshift/addon-operator/api/v1alpha1"
@@ -190,23 +189,22 @@ func TestAreSlicesEquivalent(t *testing.T) {
 // a reconcile request for the default addon operator.
 func TestEnqueueAddonOperator(t *testing.T) {
 	ctx := context.Background()
-	q := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
+	q := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]())
 	expectedRequest := reconcile.Request{
 		NamespacedName: types.NamespacedName{
 			Name: addonsv1alpha1.DefaultAddonOperatorName,
 		},
 	}
 
-	err := enqueueAddonOperator(ctx, &handler.EnqueueRequestForObject{}, q)
+	err := enqueueAddonOperator(ctx, q)
 	require.NoError(t, err, "Expected no error")
 
 	// Check that a single request was added to the queue
 	assert.Equal(t, 1, q.Len(), "Expected 1 item in the queue")
 
 	// Retrieve the added request from the queue
 	item, _ := q.Get()
-	request, ok := item.(reconcile.Request)
-	assert.True(t, ok, "Expected item to be of type reconcile.Request")
+	request := item
 	assert.Equal(t, expectedRequest, request, "Expected request does not match the added request")
 }
 

diff --git a/integration/monitoring_federation_test.go b/integration/monitoring_federation_test.go
@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	addonsv1alpha1 "github.com/openshift/addon-operator/api/v1alpha1"
@@ -244,10 +245,10 @@ func validateMonitoringFederationServiceMonitor(t *testing.T, ctx context.Contex
 				TLSConfig: &monitoringv1.TLSConfig{
 					CAFile: "/etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt",
 					SafeTLSConfig: monitoringv1.SafeTLSConfig{
-						ServerName: fmt.Sprintf(
+						ServerName: ptr.To(fmt.Sprintf(
 							"prometheus.%s.svc",
 							addon.Spec.Monitoring.Federation.Namespace,
-						),
+						)),
 					},
 				},
 			},

diff --git a/internal/webhooks/addon_webhook.go b/internal/webhooks/addon_webhook.go
@@ -2,6 +2,7 @@ package webhooks
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
 	v1 "k8s.io/api/admission/v1"
@@ -34,7 +35,12 @@ func (r *AddonWebhookHandler) Handle(ctx context.Context, req admission.Request)
 		return r.validateCreate(&obj)
 	case v1.Operation(adminv1beta1.Update):
 		oldObj := addonsv1alpha1.Addon{}
-		if err := r.decoder.DecodeRaw(req.OldObject, &oldObj); err != nil {
+		if r.decoder == nil {
+			return admission.Errored(http.StatusBadRequest, fmt.Errorf("decoder is nil"))
+		}
+		decoder := *r.decoder
+
+		if err := decoder.DecodeRaw(req.OldObject, &oldObj); err != nil {
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		return r.validateUpdate(&obj, &oldObj)
@@ -46,7 +52,11 @@ func (r *AddonWebhookHandler) Handle(ctx context.Context, req admission.Request)
 func (r *AddonWebhookHandler) decodeAddon(req admission.Request) (addonsv1alpha1.Addon, error) {
 	obj := addonsv1alpha1.Addon{}
 	if req.Operation != v1.Operation(adminv1beta1.Delete) {
-		if err := r.decoder.Decode(req, &obj); err != nil {
+		if r.decoder == nil {
+			return obj, fmt.Errorf("decoder is nil")
+		}
+		decoder := *r.decoder
+		if err := decoder.Decode(req, &obj); err != nil {
 			return obj, err
 		}
 		return obj, nil