Skip to content

Commit

Permalink
Honor the --log-enqueue-events option in all controllers (#432)
Browse files Browse the repository at this point in the history
Previously, this was only honored in the IstioRevision controller.

Signed-off-by: Marko Lukša <[email protected]>
  • Loading branch information
luksa authored Oct 17, 2024
1 parent dea3547 commit 62d9c90
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 13 deletions.
24 changes: 21 additions & 3 deletions controllers/istio/istio_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/go-logr/logr"
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
"github.com/istio-ecosystem/sail-operator/pkg/kube"
"github.com/istio-ecosystem/sail-operator/pkg/reconciler"
Expand All @@ -35,6 +36,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand Down Expand Up @@ -171,18 +173,30 @@ func getActiveRevisionName(istio *v1alpha1.Istio) string {

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
logger := mgr.GetLogger().WithName("ctrlr").WithName("istio")

// mainObjectHandler handles the IstioRevision watch events
mainObjectHandler := wrapEventHandler(logger, &handler.EnqueueRequestForObject{})

// ownedResourceHandler handles resources that are owned by the IstioRevision CR
ownedResourceHandler := wrapEventHandler(logger,
handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &v1alpha1.Istio{}, handler.OnlyControllerOwner()))

return ctrl.NewControllerManagedBy(mgr).
WithOptions(controller.Options{
LogConstructor: func(req *reconcile.Request) logr.Logger {
log := mgr.GetLogger().WithName("ctrlr").WithName("istio")
log := logger
if req != nil {
log = log.WithValues("Istio", req.Name)
}
return log
},
}).
For(&v1alpha1.Istio{}).
Owns(&v1alpha1.IstioRevision{}).
// we use the Watches function instead of For(), so that we can wrap the handler so that events that cause the object to be enqueued are logged
// +lint-watches:ignore: Istio (not found in charts, but this is the main resource watched by this controller)
Watches(&v1alpha1.Istio{}, mainObjectHandler).
Named("istio").
Watches(&v1alpha1.IstioRevision{}, ownedResourceHandler).
Complete(reconciler.NewStandardReconciler[*v1alpha1.Istio](r.Client, r.Reconcile))
}

Expand Down Expand Up @@ -315,3 +329,7 @@ func convertConditionReason(reason v1alpha1.IstioRevisionConditionReason) v1alph
panic(fmt.Sprintf("can't convert IstioRevisionConditionReason: %s", reason))
}
}

func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
return enqueuelogger.WrapIfNecessary(v1alpha1.IstioKind, logger, handler)
}
25 changes: 21 additions & 4 deletions controllers/istiocni/istiocni_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
"github.com/istio-ecosystem/sail-operator/pkg/config"
"github.com/istio-ecosystem/sail-operator/pkg/constants"
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
"github.com/istio-ecosystem/sail-operator/pkg/helm"
"github.com/istio-ecosystem/sail-operator/pkg/istiovalues"
Expand Down Expand Up @@ -196,20 +197,32 @@ func (r *Reconciler) uninstallHelmChart(ctx context.Context, cni *v1alpha1.Istio

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
logger := mgr.GetLogger().WithName("ctrlr").WithName("istiocni")

// mainObjectHandler handles the IstioCNI watch events
mainObjectHandler := wrapEventHandler(logger, &handler.EnqueueRequestForObject{})

// ownedResourceHandler handles resources that are owned by the IstioCNI CR
ownedResourceHandler := handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &v1alpha1.IstioCNI{}, handler.OnlyControllerOwner())
ownedResourceHandler := wrapEventHandler(logger,
handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &v1alpha1.IstioCNI{}, handler.OnlyControllerOwner()))

namespaceHandler := wrapEventHandler(logger, handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequest))

return ctrl.NewControllerManagedBy(mgr).
WithOptions(controller.Options{
LogConstructor: func(req *reconcile.Request) logr.Logger {
log := mgr.GetLogger().WithName("ctrlr").WithName("istiocni")
log := logger
if req != nil {
log = log.WithValues("IstioCNI", req.Name)
}
return log
},
}).
For(&v1alpha1.IstioCNI{}).

// we use the Watches function instead of For(), so that we can wrap the handler so that events that cause the object to be enqueued are logged
// +lint-watches:ignore: IstioCNI (not found in charts, but this is the main resource watched by this controller)
Watches(&v1alpha1.IstioCNI{}, mainObjectHandler).
Named("istiocni").

// namespaced resources
Watches(&corev1.ConfigMap{}, ownedResourceHandler).
Expand All @@ -222,7 +235,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {

// cluster-scoped resources
// +lint-watches:ignore: Namespace (not present in charts, but must be watched to reconcile IstioCni when its namespace is created)
Watches(&corev1.Namespace{}, handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequest)).
Watches(&corev1.Namespace{}, namespaceHandler).
Watches(&rbacv1.ClusterRole{}, ownedResourceHandler).
Watches(&rbacv1.ClusterRoleBinding{}, ownedResourceHandler).
Complete(reconciler.NewStandardReconcilerWithFinalizer[*v1alpha1.IstioCNI](r.Client, r.Reconcile, r.Finalize, constants.FinalizerName))
Expand Down Expand Up @@ -334,3 +347,7 @@ func (r *Reconciler) mapNamespaceToReconcileRequest(ctx context.Context, ns clie
}
return requests
}

func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
return enqueuelogger.WrapIfNecessary(v1alpha1.IstioCNIKind, logger, handler)
}
12 changes: 8 additions & 4 deletions controllers/istiorevision/istiorevision_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
logger := mgr.GetLogger().WithName("ctrlr").WithName("istiorev")

// mainObjectHandler handles the IstioRevision watch events
mainObjectHandler := enqueuelogger.WrapIfNecessary(v1alpha1.IstioRevisionKind, logger, &handler.EnqueueRequestForObject{})
mainObjectHandler := wrapEventHandler(logger, &handler.EnqueueRequestForObject{})

// ownedResourceHandler handles resources that are owned by the IstioRevision CR
ownedResourceHandler := enqueuelogger.WrapIfNecessary(v1alpha1.IstioRevisionKind, logger,
ownedResourceHandler := wrapEventHandler(logger,
handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &v1alpha1.IstioRevision{}, handler.OnlyControllerOwner()))

// nsHandler triggers reconciliation in two cases:
Expand All @@ -218,11 +218,11 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
// - when a namespace that references the IstioRevision CR via the istio.io/rev
// or istio-injection labels is updated, so that the InUse condition of
// the IstioRevision CR is updated.
nsHandler := enqueuelogger.WrapIfNecessary(v1alpha1.IstioRevisionKind, logger, handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequest))
nsHandler := wrapEventHandler(logger, handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequest))

// podHandler handles pods that reference the IstioRevision CR via the istio.io/rev or sidecar.istio.io/inject labels.
// The handler triggers the reconciliation of the referenced IstioRevision CR so that its InUse condition is updated.
podHandler := enqueuelogger.WrapIfNecessary(v1alpha1.IstioRevisionKind, logger, handler.EnqueueRequestsFromMapFunc(r.mapPodToReconcileRequest))
podHandler := wrapEventHandler(logger, handler.EnqueueRequestsFromMapFunc(r.mapPodToReconcileRequest))

return ctrl.NewControllerManagedBy(mgr).
WithOptions(controller.Options{
Expand Down Expand Up @@ -611,3 +611,7 @@ func clearIgnoredFields(obj client.Object) {
func badIstioRevisionType(rev *v1alpha1.IstioRevision) string {
return fmt.Sprintf("unknown IstioRevisionType: %s", rev.Spec.Type)
}

func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
return enqueuelogger.WrapIfNecessary(v1alpha1.IstioRevisionKind, logger, handler)
}
19 changes: 17 additions & 2 deletions controllers/webhook/webhook_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/go-logr/logr"
"github.com/istio-ecosystem/sail-operator/api/v1alpha1"
"github.com/istio-ecosystem/sail-operator/pkg/constants"
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
"github.com/istio-ecosystem/sail-operator/pkg/reconciler"
admissionv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -36,6 +37,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -158,17 +160,26 @@ func getReadinessProbeURL(config admissionv1.WebhookClientConfig) (string, error

// SetupWithManager sets up the controller with the Manager.
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
logger := mgr.GetLogger().WithName("ctrlr").WithName("webhook")

// objectHandler handles the MutatingWebhookConfiguration watch events
objectHandler := wrapEventHandler(logger, &handler.EnqueueRequestForObject{})

return ctrl.NewControllerManagedBy(mgr).
WithOptions(controller.Options{
LogConstructor: func(req *reconcile.Request) logr.Logger {
log := mgr.GetLogger().WithName("ctrlr").WithName("webhook")
log := logger
if req != nil {
log = log.WithValues("MutatingWebhookConfiguration", req.Name)
}
return log
},
}).
For(&admissionv1.MutatingWebhookConfiguration{}, builder.WithPredicates(ownedByRemoteIstioPredicate(mgr.GetClient()))).

// we use the Watches function instead of For(), so that we can wrap the handler so that events that cause the object to be enqueued are logged
// +lint-watches:ignore: IstioRevision (not found in charts, but this is the main resource watched by this controller)
Watches(&admissionv1.MutatingWebhookConfiguration{}, objectHandler, builder.WithPredicates(ownedByRemoteIstioPredicate(mgr.GetClient()))).
Named("mutatingwebhookconfiguration").
Complete(reconciler.NewStandardReconciler[*admissionv1.MutatingWebhookConfiguration](r.Client, r.Reconcile))
}

Expand Down Expand Up @@ -221,3 +232,7 @@ func getTimeout(webhook *admissionv1.MutatingWebhookConfiguration) time.Duration
}
return defaultTimeoutSeconds * time.Second
}

func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
return enqueuelogger.WrapIfNecessary("MutatingWebhookConfiguration", logger, handler)
}

0 comments on commit 62d9c90

Please sign in to comment.