Ignoring the same events that are not older than 2 seconds + updating…

… the status sub-resource

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

config/crd/bases/kremser.dev_rbacnegotiations.yaml

@@ -20,18 +20,33 @@ spec:
   scope: Namespaced
   versions:
   - additionalPrinterColumns:
-    - jsonPath: .spec.for.kind
-      name: kind
+    - description: For which kind the rbac negotiation was requested
+      jsonPath: .spec.for.kind
+      name: for kind
       type: string
-    - jsonPath: .spec.for.name
-      name: name
+    - description: Name of the {kind}
+      jsonPath: .spec.for.name
+      name: for name
       type: string
-    - jsonPath: .spec.role.name
+    - description: State of the negotiation
+      jsonPath: .status.status
+      name: status
+      type: string
+    - description: Name of the associated role
+      jsonPath: .spec.role.name
       name: role
+      priority: 10
       type: string
-    - jsonPath: .spec.status.status
-      name: status
+    - description: How many RBAC entries have been added to the role
+      jsonPath: .status.entriesAdded
+      name: entries
+      priority: 12
       type: string
+    - description: When the last reconciliation was done
+      jsonPath: .status.lastCheck
+      name: checked
+      priority: 13
+      type: date
     name: v1
     schema:
       openAPIV3Schema:
@@ -102,13 +117,31 @@ spec:
                 type: object
               serviceAccountName:
                 description: this can override the real service account that's specified
-                  in the deployment,rs,ds or ss
+                  in the deployment,rs,ds or ss the calculated value will be stored
+                  in status sub-resource
                 type: string
             required:
             - for
             type: object
           status:
             description: RbacNegotiationStatus defines the observed state of RbacNegotiation
+            properties:
+              entriesAdded:
+                format: int32
+                minimum: 0
+                type: integer
+              lastCheck:
+                format: date-time
+                type: string
+              status:
+                default: Requested
+                enum:
+                - Requested
+                - InProgress
+                - Error
+                - NoChange
+                - Synced
+                type: string
             type: object
         type: object
         x-kubernetes-preserve-unknown-fields: true

controllers/rbac_event_handler.go

@@ -24,6 +24,7 @@ import (
 	"io"
 	apps "k8s.io/api/apps/v1"
 	"k8s.io/client-go/tools/record"
+	"reflect"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"strings"
 	"time"
@@ -70,7 +71,7 @@ func (r *RbacEventHandler) Setup(ctx context.Context) {
 	span.End()
 }
 
-func (r *RbacEventHandler) handleResource(ctx context.Context, resource kremserv1.RbacNegotiation) ctrl.Result {
+func (r *RbacEventHandler) handleResource(ctx context.Context, resource *kremserv1.RbacNegotiation) ctrl.Result {
 	// tracing
 	newCtx, span := r.Tracer.Start(ctx, "handleResource")
 	span.SetAttributes(attribute.String("resource.name", resource.Name))
@@ -81,7 +82,9 @@ func (r *RbacEventHandler) handleResource(ctx context.Context, resource kremserv
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())
-		log.Log.Error(err, "Unable to get logs from underlying pod.")
+		if !strings.Contains(fmt.Sprint(err), "ContainerCreating") {
+			UpdateStatus(r.Client, ctx, resource, true, false)
+		}
 		return ctrl.Result{
 			Requeue:      true,
 			RequeueAfter: time.Duration(r.Config.Controller.SyncIntervalAfterNoLogsSeconds) * time.Second,
@@ -96,26 +99,31 @@ func (r *RbacEventHandler) handleResource(ctx context.Context, resource kremserv
 		err := r.addMissingRbacEntry(newCtx, resource.Spec.For.Namespace, appInfo.serviceAccount, missingRbacEntry, resource.Spec.Role)
 		if err != nil {
 			log.Log.Error(err, "Unable to add missing rbac entry")
+			UpdateStatus(r.Client, ctx, resource, true, false)
+
 			return ctrl.Result{
 				Requeue:      true,
 				RequeueAfter: time.Duration(r.Config.Controller.SyncIntervalAfterErrorMinutes) * time.Minute,
 			}
 		}
-		r.emitEvent(resource, missingRbacEntry)
+		r.emitEvent(*resource, missingRbacEntry)
 		tryAgainInSeconds := r.Config.Controller.SyncIntervalAfterPodRestartSeconds
 		if r.Config.Controller.ShouldRestartAppPods {
 			r.restartPods(newCtx, appInfo.livePods)
 		} else {
-			// pod is going to be restarted anyway, but exp backoff can make this quite a long process
+			// pod is going to be restarted anyway, but the default exp backoff can make this quite a long process
 			tryAgainInSeconds *= 4
 		}
+		UpdateStatus(r.Client, ctx, resource, false, true)
 		return ctrl.Result{
 			Requeue:      true,
 			RequeueAfter: time.Duration(tryAgainInSeconds) * time.Second,
 		}
 	}
 	retryMinutes := r.Config.Controller.SyncIntervalAfterNoRbacEntryMinutes
 	log.Log.Info(fmt.Sprintf("No rbac related stuff has been found in the logs. Will try again in %d minutes..", retryMinutes))
+
+	UpdateStatus(r.Client, ctx, resource, false, false)
 	return ctrl.Result{
 		Requeue:      true,
 		RequeueAfter: time.Duration(retryMinutes) * time.Minute,
@@ -294,9 +302,11 @@ func (r *RbacEventHandler) getAppInfo(ctx context.Context, resource kremserv1.Rb
 	podLogs, err := req.Stream(ctx)
 	if err != nil {
 		if strings.Contains(fmt.Sprint(err), "waiting to start: ContainerCreating") {
+			log.Log.Info(fmt.Sprintf("Unable to get logs from underlying pod. Pod %s is still starting (ContainerCreating)", podName))
 			return nil, fmt.Errorf("pod %s is still starting (ContainerCreating)", podName)
 		}
 		log.Log.V(1).Info("Check the ReplicaSet if the service account isn't missing.")
+		log.Log.Error(err, "Unable to get logs from underlying pod.")
 		return nil, err
 	}
 	defer podLogs.Close()
@@ -354,7 +364,7 @@ func (r *RbacEventHandler) getSelectorAndSA(ctx context.Context, resource kremse
 
 func (r *RbacEventHandler) getObject(ctx context.Context, obj client.Object, nsName client.ObjectKey) (map[string]string, string) {
 	if err := r.Client.Get(ctx, nsName, obj); err != nil {
-		log.Log.Error(err, fmt.Sprintf("Cannot get k8s object %s with name %v ", obj.GetObjectKind(), nsName))
+		log.Log.Error(err, fmt.Sprintf("Cannot get %v resource with name '%v' ", reflect.TypeOf(obj), nsName))
 		return nil, ""
 	}
 	switch casted := obj.(type) {

controllers/rbacnegotiation_controller.go

@@ -52,19 +52,24 @@ type RbacNegotiationReconciler struct {
 func (r *RbacNegotiationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	_ = log.FromContext(ctx)
 
-	log.Log.Info("fetching RbacNegotiation resource")
+	//log.Log.Info("fetching RbacNegotiation resource")
 	rbacNeg := kremserv1.RbacNegotiation{}
 	if err := r.Client.Get(ctx, req.NamespacedName, &rbacNeg); err != nil {
 		log.Log.Info(fmt.Sprintf("Failed to get RbacNegotiation '%s/%s'. It was probably deleted.", req.NamespacedName.Namespace, req.NamespacedName.Name))
 		// Ignore NotFound errors as they will be retried automatically if the
 		// resource is created in the future.
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
+
+	// ignore the same events that are not older than 2 sec
+	if IsNotOlderThan(&rbacNeg, 2) {
+		return ctrl.Result{}, nil
+	}
 	if rbacNeg.Spec.For.Namespace == "" {
 		rbacNeg.Spec.For.Namespace = req.Namespace
 	}
 	log.Log.Info(fmt.Sprintf("New rbac negotiation event: for %s '%s'", strings.ToLower(rbacNeg.Spec.For.Kind), rbacNeg.Spec.For.Name))
-	result := r.handler.handleResource(ctx, rbacNeg)
+	result := r.handler.handleResource(ctx, &rbacNeg)
 	return result, nil
 }