Merge pull request #605 from kyledong-suse/feature

feat: modify startup probe with INFO and final sync failure error

Author: kyledong-suse

charts/runtime-enforcer/templates/agent/daemonset.yaml

@@ -92,8 +92,8 @@ spec:
           httpGet:
             path: /readyz
             port: 8081
-          # we give 45s (5s delay + 8 attempts every 5s) for the startup probe to succeed
-          initialDelaySeconds: 5
+          # we give 50s (10s delay + 8 attempts every 5s) for the startup probe to succeed
+          initialDelaySeconds: 10
           periodSeconds: 5
           failureThreshold: 8
         resources: {{- toYaml .Values.agent.resources | nindent 10 }}

cmd/agent/main.go

@@ -39,6 +39,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
+const wpSyncInProgressMsg = "waiting for WorkloadPolicy synchronization to complete"
+
 type Config struct {
 	learningNamespaceSelector string
 	nriSocketPath             string
@@ -95,25 +97,30 @@ func setupWorkloadPolicyHandler(
 	ctrlMgr manager.Manager,
 	logger *slog.Logger,
 	resolver *resolver.Resolver,
-) error {
+) (*workloadpolicyhandler.WorkloadPolicyHandler, error) {
 	wpHandler := workloadpolicyhandler.NewWorkloadPolicyHandler(ctrlMgr.GetClient(), logger, resolver)
 	err := wpHandler.SetupWithManager(ctrlMgr)
 	if err != nil {
-		return fmt.Errorf("unable to set up WorkloadPolicy handler: %w", err)
+		return nil, fmt.Errorf("unable to set up WorkloadPolicy handler: %w", err)
 	}
 	// controller-runtime doesn't support a separate startup probe, so we use the readiness probe instead.
 	// See https://github.com/kubernetes-sigs/controller-runtime/issues/2644 for more details.
 	if err = ctrlMgr.AddReadyzCheck("policy readyz", func(req *http.Request) error {
 		if syncErr := wpHandler.HasSynced(req.Context()); syncErr != nil {
-			logger.ErrorContext(req.Context(), "WorkloadPolicy handler is not synced", "error", syncErr)
-			return fmt.Errorf("WorkloadPolicy handler is not synced: %w", syncErr)
+			logger.InfoContext(
+				req.Context(),
+				wpSyncInProgressMsg,
+				"error",
+				syncErr,
+			)
+			return fmt.Errorf("%s: %w", wpSyncInProgressMsg, syncErr)
 		}
 		return nil
 	}); err != nil {
-		return fmt.Errorf("failed to add policy readiness probe: %w", err)
+		return nil, fmt.Errorf("failed to add policy readiness probe: %w", err)
 	}
 
-	return nil
+	return wpHandler, nil
 }
 
 func waitForMutatingAdmissionWebhook(ctx context.Context) error {
@@ -239,7 +246,8 @@ func startAgent(ctx context.Context, logger *slog.Logger, config Config) error {
 		return fmt.Errorf("failed to create resolver: %w", err)
 	}
 
-	if err = setupWorkloadPolicyHandler(ctrlMgr, logger, resolver); err != nil {
+	wpHandler, err := setupWorkloadPolicyHandler(ctrlMgr, logger, resolver)
+	if err != nil {
 		return err
 	}
 
@@ -298,6 +306,12 @@ func startAgent(ctx context.Context, logger *slog.Logger, config Config) error {
 
 	logger.InfoContext(ctx, "starting manager")
 	if err = ctrlMgr.Start(ctx); err != nil {
+		if !resolver.IsNRISynchronized() || !wpHandler.IsSynchronized() {
+			logger.ErrorContext(ctx, "agent terminated before startup synchronization completed",
+				"nriSynchronized", resolver.IsNRISynchronized(),
+				"workloadPoliciesSynchronized", wpHandler.IsSynchronized(),
+			)
+		}
 		return fmt.Errorf("failed to start manager: %w", err)
 	}
 

internal/nri/plugin.go

@@ -14,6 +14,8 @@ import (
 	"github.com/rancher-sandbox/runtime-enforcer/internal/types/workloadkind"
 )
 
+const nriSyncRetryMsg = "NRI pod/container sync not ready yet, will retry"
+
 type plugin struct {
 	stub            stub.Stub
 	logger          *slog.Logger
@@ -163,8 +165,8 @@ func (p *plugin) Synchronize(
 		)
 		if err := p.resolver.AddPodContainerFromNri(podData); err != nil {
 			// This could be recoverable. Returning an error so we can retry.
-			podLogger.ErrorContext(ctx, "failed to add pod container from NRI", "error", err)
-			return nil, fmt.Errorf("failed to add pod container from NRI: %w", err)
+			podLogger.InfoContext(ctx, nriSyncRetryMsg, "error", err)
+			return nil, fmt.Errorf("%s: %w", nriSyncRetryMsg, err)
 		}
 	}
 	// Mark resolver as synchronized, so old agent can be safely removed.

internal/resolver/nri_api.go

@@ -8,6 +8,8 @@ import (
 	"github.com/rancher-sandbox/runtime-enforcer/internal/bpf"
 )
 
+const nriSyncInProgressMsg = "waiting for NRI synchronization to complete"
+
 func convertPodData(pod PodInput) *podEntry {
 	return &podEntry{
 		meta:       &pod.Meta,
@@ -113,10 +115,14 @@ func (r *Resolver) NRISynchronized() {
 	r.nriSynchronized.Store(true)
 }
 
+func (r *Resolver) IsNRISynchronized() bool {
+	return r.nriSynchronized.Load()
+}
+
 func (r *Resolver) Ping(req *http.Request) error {
 	if !r.nriSynchronized.Load() {
-		r.logger.WarnContext(req.Context(), "NRI handler has not yet synchronized")
-		return errors.New("NRI handler has not yet synchronized")
+		r.logger.InfoContext(req.Context(), nriSyncInProgressMsg)
+		return errors.New(nriSyncInProgressMsg)
 	}
 	return nil
 }

internal/workloadpolicyhandler/workloadpolicy_handler.go

@@ -111,6 +111,10 @@ func (r *WorkloadPolicyHandler) HasSynced(ctx context.Context) error {
 	return nil
 }
 
+func (r *WorkloadPolicyHandler) IsSynchronized() bool {
+	return r.hasSynced.Load()
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *WorkloadPolicyHandler) SetupWithManager(mgr ctrl.Manager) error {
 	err := ctrl.NewControllerManagedBy(mgr).