diff --git a/controllers/apps/apimanager_controller.go b/controllers/apps/apimanager_controller.go index 4e29a63dd..0849dac79 100644 --- a/controllers/apps/apimanager_controller.go +++ b/controllers/apps/apimanager_controller.go @@ -160,7 +160,7 @@ func (r *APIManagerReconciler) Reconcile(ctx context.Context, req ctrl.Request) return res, nil } - specResult, specErr := r.reconcileAPIManagerLogic(instance) + specResult, specErr := r.reconcileAPIManagerLogic(r.BaseReconciler, instance) statusResult, statusErr := r.reconcileAPIManagerStatus(instance, preflightChecksError) if statusErr != nil { return ctrl.Result{}, statusErr @@ -402,8 +402,8 @@ func (r *APIManagerReconciler) setAPIManagerDefaults(cr *appsv1alpha1.APIManager return ctrl.Result{Requeue: updated}, err } -func (r *APIManagerReconciler) reconcileAPIManagerLogic(cr *appsv1alpha1.APIManager) (reconcile.Result, error) { - baseAPIManagerLogicReconciler := operator.NewBaseAPIManagerLogicReconciler(r.BaseReconciler, cr) +func (r *APIManagerReconciler) reconcileAPIManagerLogic(b *reconcilers.BaseReconciler, cr *appsv1alpha1.APIManager) (reconcile.Result, error) { + baseAPIManagerLogicReconciler := operator.NewBaseAPIManagerLogicReconciler(b, cr) imageReconciler := operator.NewAMPImagesReconciler(baseAPIManagerLogicReconciler) result, err := imageReconciler.Reconcile() if err != nil || result.Requeue { diff --git a/go.mod b/go.mod index 8003c19be..e01e873f0 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( github.com/spf13/cobra v1.7.0 github.com/spf13/viper v1.7.0 github.com/stretchr/testify v1.11.1 + go.uber.org/zap v1.26.0 golang.org/x/mod v0.27.0 k8s.io/api v0.29.0 k8s.io/apimachinery v0.29.0 @@ -101,7 +102,6 @@ require ( github.com/subosito/gotenv v1.2.0 // indirect go.mongodb.org/mongo-driver v1.14.0 // indirect go.uber.org/multierr v1.11.0 // indirect - go.uber.org/zap v1.26.0 // indirect golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/net v0.43.0 // indirect golang.org/x/oauth2 v0.16.0 // indirect diff --git a/pkg/3scale/amp/component/backend.go b/pkg/3scale/amp/component/backend.go index 0722ac353..432878fc6 100644 --- a/pkg/3scale/amp/component/backend.go +++ b/pkg/3scale/amp/component/backend.go @@ -15,6 +15,7 @@ import ( policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) const ( @@ -256,6 +257,9 @@ func (backend *Backend) CronDeployment(ctx context.Context, k8sclient client.Cli }, InitialDelaySeconds: 30, PeriodSeconds: 5, + TimeoutSeconds: 1, + SuccessThreshold: 1, + FailureThreshold: 3, }, }, }, @@ -330,10 +334,10 @@ func (backend *Backend) ListenerDeployment(ctx context.Context, k8sclient client }, }, InitialDelaySeconds: 30, - TimeoutSeconds: 0, + TimeoutSeconds: 1, PeriodSeconds: 10, - SuccessThreshold: 0, - FailureThreshold: 0, + SuccessThreshold: 1, + FailureThreshold: 3, }, ReadinessProbe: &v1.Probe{ ProbeHandler: v1.ProbeHandler{ @@ -343,13 +347,14 @@ func (backend *Backend) ListenerDeployment(ctx context.Context, k8sclient client Type: intstr.Int, IntVal: 3000, }, + Scheme: v1.URISchemeHTTP, }, }, InitialDelaySeconds: 30, TimeoutSeconds: 5, - PeriodSeconds: 0, - SuccessThreshold: 0, - FailureThreshold: 0, + PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, }, ImagePullPolicy: v1.PullIfNotPresent, }, @@ -668,7 +673,7 @@ func (backend *Backend) QueuesRedisTLSEnvVars() []v1.EnvVar { } func (backend *Backend) backendVolumes() []v1.Volume { - res := []v1.Volume{} + var res []v1.Volume if backend.Options.BackendRedisTLS.Enabled { items := []v1.KeyToPath{} if backend.Options.BackendRedisTLS.HasCA() { @@ -682,8 +687,9 @@ func (backend *Backend) backendVolumes() []v1.Volume { Name: "backend-redis-tls", VolumeSource: v1.VolumeSource{ Secret: &v1.SecretVolumeSource{ - SecretName: BackendSecretBackendRedisSecretName, - Items: items, + SecretName: BackendSecretBackendRedisSecretName, + Items: items, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } @@ -703,8 +709,9 @@ func (backend *Backend) backendVolumes() []v1.Volume { Name: "queues-redis-tls", VolumeSource: v1.VolumeSource{ Secret: &v1.SecretVolumeSource{ - SecretName: BackendSecretBackendRedisSecretName, - Items: items, + SecretName: BackendSecretBackendRedisSecretName, + Items: items, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } @@ -725,7 +732,7 @@ func (backend *Backend) backendListenerRunArgs() []string { } func (backend *Backend) backendContainerVolumeMounts() []v1.VolumeMount { - res := []v1.VolumeMount{} + var res []v1.VolumeMount if backend.Options.BackendRedisTLS.Enabled { res = append(res, backend.backendRedisContainerVolumeMounts()) } diff --git a/pkg/3scale/amp/component/backend_redis_tls_test.go b/pkg/3scale/amp/component/backend_redis_tls_test.go index 5b0c49298..732a0f9cd 100644 --- a/pkg/3scale/amp/component/backend_redis_tls_test.go +++ b/pkg/3scale/amp/component/backend_redis_tls_test.go @@ -5,6 +5,7 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" "github.com/3scale/3scale-operator/pkg/helper" ) @@ -163,7 +164,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { BackendRedisTLS: TLSConfig{Enabled: false}, BackendRedisQueuesTLS: TLSConfig{Enabled: false}, }, - []v1.Volume{}, + nil, }, { "StorageOnly_OneWayTLS", @@ -183,6 +184,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_CA", Path: "backend-redis-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -206,6 +208,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_QUEUES_CA", Path: "backend-redis-queues-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -238,6 +241,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_CERT", Path: "backend-redis-client.crt"}, {Key: "REDIS_SSL_KEY", Path: "backend-redis-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -251,6 +255,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_QUEUES_CERT", Path: "backend-redis-queues-client.crt"}, {Key: "REDIS_SSL_QUEUES_KEY", Path: "backend-redis-queues-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -281,6 +286,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_CERT", Path: "backend-redis-client.crt"}, {Key: "REDIS_SSL_KEY", Path: "backend-redis-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -292,6 +298,7 @@ func TestBackendComponentRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_QUEUES_CA", Path: "backend-redis-queues-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -322,7 +329,7 @@ func TestBackendComponentRedisTLSVolumeMounts(t *testing.T) { BackendRedisTLS: TLSConfig{Enabled: false}, BackendRedisQueuesTLS: TLSConfig{Enabled: false}, }, - []v1.VolumeMount{}, + nil, }, { "StorageOnly", diff --git a/pkg/3scale/amp/component/system.go b/pkg/3scale/amp/component/system.go index e06777d64..37285d0b7 100644 --- a/pkg/3scale/amp/component/system.go +++ b/pkg/3scale/amp/component/system.go @@ -14,6 +14,7 @@ import ( policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "github.com/3scale/3scale-operator/apis/apps" "github.com/3scale/3scale-operator/pkg/helper" @@ -594,6 +595,7 @@ func (system *System) appPodVolumes() []v1.Volume { Path: "tls.key", // Map the secret key to the tls.key file in the container }, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } @@ -624,6 +626,7 @@ func (system *System) appPodVolumes() []v1.Volume { }, }, }, + DefaultMode: ptr.To(v1.ProjectedVolumeSourceDefaultMode), }, }, } @@ -988,6 +991,7 @@ func (system *System) SidekiqPodVolumes() []v1.Volume { Path: "tls.key", // Map the secret key to the tls.key file in the container }, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } @@ -1015,6 +1019,7 @@ func (system *System) SidekiqPodVolumes() []v1.Volume { }, }, }, + DefaultMode: ptr.To(v1.ProjectedVolumeSourceDefaultMode), }, }, } @@ -1505,6 +1510,9 @@ func (system *System) systemInit(containerImage string) []v1.Container { "-c", "cp /tls/* /writable-tls/ && chmod 0600 /writable-tls/*", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, VolumeMounts: []v1.VolumeMount{ { Name: "tls-secret", @@ -1535,7 +1543,10 @@ func (system *System) sidekiqInit(containerImage string) []v1.Container { "-c", "bundle exec sh -c \"until rake boot:redis && curl --output /dev/null --silent --fail --head http://system-master:3000/status; do sleep $SLEEP_SECONDS; done\"", }, - Env: append(system.SystemRedisEnvVars(), helper.EnvVarFromValue("SLEEP_SECONDS", "1")), + Env: append(system.SystemRedisEnvVars(), helper.EnvVarFromValue("SLEEP_SECONDS", "1")), + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, } // Append Redis TLS volume mounts if Redis TLS is enabled @@ -1552,6 +1563,9 @@ func (system *System) sidekiqInit(containerImage string) []v1.Container { "-c", "cp /tls/* /writable-tls/ && chmod 0600 /writable-tls/*", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, VolumeMounts: []v1.VolumeMount{ { Name: "tls-secret", @@ -1683,8 +1697,9 @@ func (system *System) redisTLSVolumes() []v1.Volume { Name: "system-redis-tls", VolumeSource: v1.VolumeSource{ Secret: &v1.SecretVolumeSource{ - SecretName: SystemSecretSystemRedisSecretName, - Items: items, + SecretName: SystemSecretSystemRedisSecretName, + Items: items, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } @@ -1704,8 +1719,9 @@ func (system *System) redisTLSVolumes() []v1.Volume { Name: "backend-redis-tls", VolumeSource: v1.VolumeSource{ Secret: &v1.SecretVolumeSource{ - SecretName: BackendSecretBackendRedisSecretName, - Items: items, + SecretName: BackendSecretBackendRedisSecretName, + Items: items, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, } diff --git a/pkg/3scale/amp/component/system_redis_tls_test.go b/pkg/3scale/amp/component/system_redis_tls_test.go index 60faa16fe..1865c999f 100644 --- a/pkg/3scale/amp/component/system_redis_tls_test.go +++ b/pkg/3scale/amp/component/system_redis_tls_test.go @@ -5,6 +5,7 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" "github.com/3scale/3scale-operator/pkg/helper" ) @@ -187,6 +188,7 @@ func TestRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_CA", Path: "system-redis-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -210,6 +212,7 @@ func TestRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_CA", Path: "backend-redis-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -242,6 +245,7 @@ func TestRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_CERT", Path: "system-redis-client.crt"}, {Key: "REDIS_SSL_KEY", Path: "system-redis-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -255,6 +259,7 @@ func TestRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_CERT", Path: "backend-redis-client.crt"}, {Key: "REDIS_SSL_KEY", Path: "backend-redis-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -285,6 +290,7 @@ func TestRedisTLSVolumes(t *testing.T) { {Key: "REDIS_SSL_CERT", Path: "system-redis-client.crt"}, {Key: "REDIS_SSL_KEY", Path: "system-redis-private.key"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -296,6 +302,7 @@ func TestRedisTLSVolumes(t *testing.T) { Items: []v1.KeyToPath{ {Key: "REDIS_SSL_CA", Path: "backend-redis-ca.crt"}, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, diff --git a/pkg/3scale/amp/component/system_searchd.go b/pkg/3scale/amp/component/system_searchd.go index b0bf23190..16dd9fd1a 100644 --- a/pkg/3scale/amp/component/system_searchd.go +++ b/pkg/3scale/amp/component/system_searchd.go @@ -101,7 +101,10 @@ func (s *SystemSearchd) Deployment(ctx context.Context, k8sclient client.Client, }, }, InitialDelaySeconds: 60, + TimeoutSeconds: 1, PeriodSeconds: 10, + SuccessThreshold: 1, + FailureThreshold: 3, }, ReadinessProbe: &v1.Probe{ ProbeHandler: v1.ProbeHandler{ diff --git a/pkg/3scale/amp/component/zync.go b/pkg/3scale/amp/component/zync.go index 38764d75a..326c295de 100644 --- a/pkg/3scale/amp/component/zync.go +++ b/pkg/3scale/amp/component/zync.go @@ -11,6 +11,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -642,6 +643,9 @@ func (zync *Zync) zyncInit(containerImage string) []v1.Container { "-c", "cp /tls/* /writable-tls/ && chmod 0600 /writable-tls/*", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, VolumeMounts: []v1.VolumeMount{ { Name: "tls-secret", @@ -663,6 +667,9 @@ func (zync *Zync) zyncInit(containerImage string) []v1.Container { "-c", "bundle exec sh -c \"until rake boot:db; do sleep $SLEEP_SECONDS; done\"", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, Env: []v1.EnvVar{ { Name: "SLEEP_SECONDS", @@ -704,6 +711,9 @@ func (zync *Zync) zyncInit(containerImage string) []v1.Container { "-c", "bundle exec sh -c \"until rake boot:db; do sleep $SLEEP_SECONDS; done\"", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, Env: []v1.EnvVar{ { Name: "SLEEP_SECONDS", @@ -763,6 +773,7 @@ func (zync *Zync) zyncVolume() []v1.Volume { Path: "tls.key", // Map the secret key to the tls.key file in the container }, }, + DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode), }, }, }, @@ -789,6 +800,9 @@ func (zync *Zync) zyncQueInit(containerImage string) []v1.Container { "-c", "cp /tls/* /writable-tls/ && chmod 0600 /writable-tls/*", }, + ImagePullPolicy: v1.PullIfNotPresent, + TerminationMessagePath: v1.TerminationMessagePathDefault, + TerminationMessagePolicy: v1.TerminationMessageReadFile, VolumeMounts: []v1.VolumeMount{ { Name: "tls-secret", diff --git a/test/integration/apimanager_controller_test.go b/test/integration/apimanager_controller_test.go index b0b0c4bd8..7b53f9735 100644 --- a/test/integration/apimanager_controller_test.go +++ b/test/integration/apimanager_controller_test.go @@ -211,6 +211,14 @@ var _ = Describe("APIManager controller", func() { waitForAPIManagerAvailableCondition(5*time.Second, 15*time.Minute, apimanager, GinkgoWriter) fmt.Fprintf(GinkgoWriter, "APIManager 'Available' condition is true\n") + // Verify no perpetual reconciliation occurred during initial deployment. + verifyNoDeploymentUpdates(apimanager.Namespace, apimanager.Name, 0, GinkgoWriter) + + // Trigger a synthetic change and confirm the operator corrects it exactly once. + triggerSyntheticDeploymentUpdate(testNamespace, GinkgoWriter) + time.Sleep(settlingPeriod) + verifyNoDeploymentUpdates(apimanager.Namespace, apimanager.Name, 1, GinkgoWriter) + elapsed := time.Since(start) fmt.Fprintf(GinkgoWriter, "APIManager creation and availability took '%s'\n", elapsed) }) @@ -1053,6 +1061,36 @@ func testCustomEnvironmentContent() string { ` } +// triggerSyntheticDeploymentUpdate patches a monitored deployment with a dummy +// image tag, then waits for the operator to reconcile it back. This guarantees +// at least one Deployment UPDATE in the ReconcileCounter, proving the counter +// is wired correctly and not silently counting nothing. +func triggerSyntheticDeploymentUpdate(namespace string, w io.Writer) { + const deploymentName = "system-memcache" + + dep := &appsv1.Deployment{} + Expect(testK8sClient.Get(context.Background(), + types.NamespacedName{Name: deploymentName, Namespace: namespace}, dep)).To(Succeed()) + + originalImage := dep.Spec.Template.Spec.Containers[0].Image + dep.Spec.Template.Spec.Containers[0].Image = originalImage + "-synthetic-test-trigger" + Expect(testK8sClient.Update(context.Background(), dep)).To(Succeed()) + fmt.Fprintf(w, "Synthetic image change applied to %s; waiting for operator to reconcile back\n", deploymentName) + + // Use testK8sAPIClient (direct API server read) to avoid the cached client + // returning the pre-update image before the operator has processed the change. + Eventually(func() bool { + d := &appsv1.Deployment{} + if err := testK8sAPIClient.Get(context.Background(), + types.NamespacedName{Name: deploymentName, Namespace: namespace}, d); err != nil { + return false + } + return d.Spec.Template.Spec.Containers[0].Image == originalImage + }, 2*time.Minute, 2*time.Second).Should(BeTrue(), + fmt.Sprintf("operator did not revert synthetic image change on %s within 2 minutes", deploymentName)) + fmt.Fprintf(w, "Operator reconciled %s back to desired image\n", deploymentName) +} + func testGetCustomEnvironmentSecret(namespace string) *corev1.Secret { customEnvironmentSecret := corev1.Secret{ TypeMeta: metav1.TypeMeta{ diff --git a/test/integration/apimanager_suite_test.go b/test/integration/apimanager_suite_test.go index 3427c6e48..fe7f69442 100644 --- a/test/integration/apimanager_suite_test.go +++ b/test/integration/apimanager_suite_test.go @@ -25,6 +25,8 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + uberzap "go.uber.org/zap" + "go.uber.org/zap/zapcore" "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -57,6 +59,7 @@ var ( testK8sClient client.Client testK8sAPIClient client.Reader testEnv *envtest.Environment + reconcileCounter *ReconcileCounter ) func TestAPIManager(t *testing.T) { @@ -65,7 +68,38 @@ func TestAPIManager(t *testing.T) { } var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + // List of operator-managed deployments to monitor for updates + monitoredDeployments := []string{ + "apicast-production", + "apicast-staging", + "backend-cron", + "backend-listener", + "backend-worker", + "system-app", + "system-memcache", + "system-sidekiq", + "system-searchd", + "zync", + "zync-que", + "zync-database", + } + + // Build a base zapcore.Core writing to GinkgoWriter so ReconcileCounter can wrap it. + baseCore := zapcore.NewCore( + zapcore.NewConsoleEncoder(uberzap.NewDevelopmentEncoderConfig()), + zapcore.AddSync(GinkgoWriter), + zapcore.DebugLevel, + ) + reconcileCounter = NewReconcileCounter(baseCore, monitoredDeployments) + + // Inject the wrapped core into the controller-runtime logger via RawZapOpts. + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + zap.RawZapOpts(uberzap.WrapCore(func(_ zapcore.Core) zapcore.Core { + return reconcileCounter + })), + )) By("bootstrapping test environment") diff --git a/test/integration/reconcile_counter_test.go b/test/integration/reconcile_counter_test.go new file mode 100644 index 000000000..9d0cb93f8 --- /dev/null +++ b/test/integration/reconcile_counter_test.go @@ -0,0 +1,172 @@ +package integration + +import ( + "fmt" + "strings" + "sync" + + "go.uber.org/zap/zapcore" +) + +// counterState is the single shared mutable core of a ReconcileCounter tree. +// All ReconcileCounter instances produced by With() point to the same counterState, +// so counts accumulate correctly regardless of which logger variant does the write. +// mu protects updateCounts; deploymentNames is read-only after construction. +type counterState struct { + deploymentNames map[string]bool + // updateCounts is keyed by "namespace/name" (the APIManager CR instance), + // then by deployment name. This allows parallel specs targeting different + // CR instances to accumulate counts independently. + updateCounts map[string]map[string]int + mu sync.Mutex +} + +// ReconcileCounter wraps a zapcore.Core to count deployment update events per +// APIManager CR instance. It overrides With and Check so the counter survives +// logger.WithName/WithValues chains, and captures the "namespace" and "name" +// fields that controller-runtime adds to the reconciler logger context. +type ReconcileCounter struct { + zapcore.Core + state *counterState + namespace string // captured from With() chain + crName string // captured from With() chain +} + +func (rc *ReconcileCounter) crKey() string { + if rc.namespace == "" || rc.crName == "" { + return "" + } + return rc.namespace + "/" + rc.crName +} + +// NewReconcileCounter creates a new ReconcileCounter wrapping the given core. +func NewReconcileCounter(core zapcore.Core, deploymentNames []string) *ReconcileCounter { + nameMap := make(map[string]bool) + for _, name := range deploymentNames { + nameMap[name] = true + } + return &ReconcileCounter{ + Core: core, + state: &counterState{ + deploymentNames: nameMap, + updateCounts: make(map[string]map[string]int), + }, + } +} + +// With returns a new ReconcileCounter wrapping the inner core-with-fields, +// sharing the same counter state. It captures "namespace" and "name" fields +// so Write can key counts by CR instance. +func (rc *ReconcileCounter) With(fields []zapcore.Field) zapcore.Core { + ns := rc.namespace + name := rc.crName + for _, f := range fields { + if f.Type == zapcore.StringType { + switch f.Key { + case "namespace": + ns = f.String + case "name": + name = f.String + } + } + } + return &ReconcileCounter{ + Core: rc.Core.With(fields), + state: rc.state, + namespace: ns, + crName: name, + } +} + +// Check ensures rc.Write is called (not the inner core's Write) for entries +// that pass the level check. +func (rc *ReconcileCounter) Check(entry zapcore.Entry, ce *zapcore.CheckedEntry) *zapcore.CheckedEntry { + if rc.Core.Enabled(entry.Level) { + return ce.AddCore(entry, rc) + } + return ce +} + +// Write intercepts log entries and counts deployment updates keyed by CR instance. +// The key is resolved in priority order: +// 1. With() chain (context logger): controller-runtime injects "namespace"/"name" before Reconcile. +// 2. Explicit fields on the log entry: UpdateResource logs obj.GetNamespace() as "namespace" and +// the APIManager owner reference name as "name". +func (rc *ReconcileCounter) Write(entry zapcore.Entry, fields []zapcore.Field) error { + if !strings.Contains(entry.Message, "Updated object 'v1.Deployment/") { + return rc.Core.Write(entry, fields) + } + parts := strings.Split(entry.Message, "v1.Deployment/") + if len(parts) == 2 { + deploymentName := strings.TrimSuffix(parts[1], "'") + if rc.state.deploymentNames[deploymentName] { + key := rc.crKey() + if key == "" { + var ns, name string + for _, f := range fields { + if f.Type == zapcore.StringType { + switch f.Key { + case "namespace": + ns = f.String + case "name": + name = f.String + } + } + } + if ns != "" && name != "" { + key = ns + "/" + name + } + } + if key != "" { + rc.state.mu.Lock() + if rc.state.updateCounts[key] == nil { + rc.state.updateCounts[key] = make(map[string]int) + } + rc.state.updateCounts[key][deploymentName]++ + rc.state.mu.Unlock() + } + } + } + return rc.Core.Write(entry, fields) +} + +// GetUpdateCounts returns a copy of per-deployment counts for the given CR instance. +func (rc *ReconcileCounter) GetUpdateCounts(namespace, name string) map[string]int { + key := namespace + "/" + name + rc.state.mu.Lock() + defer rc.state.mu.Unlock() + counts := make(map[string]int) + for k, v := range rc.state.updateCounts[key] { + counts[k] = v + } + return counts +} + +// GetTotalUpdates returns the total deployment update count for the given CR instance. +func (rc *ReconcileCounter) GetTotalUpdates(namespace, name string) int { + key := namespace + "/" + name + rc.state.mu.Lock() + defer rc.state.mu.Unlock() + total := 0 + for _, count := range rc.state.updateCounts[key] { + total += count + } + return total +} + +// GetReport returns a formatted breakdown for the given CR instance. +func (rc *ReconcileCounter) GetReport(namespace, name string) string { + key := namespace + "/" + name + rc.state.mu.Lock() + defer rc.state.mu.Unlock() + counts := rc.state.updateCounts[key] + if len(counts) == 0 { + return fmt.Sprintf("No deployment updates detected for %s", key) + } + var sb strings.Builder + fmt.Fprintf(&sb, "Deployment update counts for %s:\n", key) + for depName, count := range counts { + fmt.Fprintf(&sb, " %s: %d updates\n", depName, count) + } + return sb.String() +} diff --git a/test/integration/verify_no_perpetual_reconciliation_test.go b/test/integration/verify_no_perpetual_reconciliation_test.go new file mode 100644 index 000000000..33dc2a829 --- /dev/null +++ b/test/integration/verify_no_perpetual_reconciliation_test.go @@ -0,0 +1,61 @@ +package integration + +import ( + "fmt" + "io" + "sort" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// settlingPeriod is the time to wait after the synthetic update is reconciled +// to confirm no further updates occur. +const settlingPeriod = 30 * time.Second + +// verifyNoDeploymentUpdates asserts that the reconcile counter recorded exactly +// the expected number of deployment updates. Before the synthetic trigger this +// should be 0 (no perpetual reconcile during initial deployment). After the +// synthetic trigger it should be exactly 1 (the operator corrected the drift +// and stopped). +func verifyNoDeploymentUpdates(namespace, name string, expected int, w io.Writer) { + updateCounts := reconcileCounter.GetUpdateCounts(namespace, name) + totalUpdates := reconcileCounter.GetTotalUpdates(namespace, name) + + fmt.Fprintf(w, "\n=== Deployment Update Report (%s/%s, expected %d) ===\n", + namespace, name, expected) + fmt.Fprintf(w, "Total: %d\n", totalUpdates) + + names := make([]string, 0, len(updateCounts)) + for n := range updateCounts { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + fmt.Fprintf(w, " %s: %d\n", n, updateCounts[n]) + } + fmt.Fprintf(w, "=============================================================\n\n") + + Expect(totalUpdates).To(Equal(expected), + deploymentUpdateDetail(namespace, name, updateCounts, totalUpdates, expected)) +} + +// deploymentUpdateDetail builds a human-readable breakdown for use in a Gomega +// failure message so the offending deployments are immediately visible. +func deploymentUpdateDetail(namespace, name string, counts map[string]int, total, expected int) string { + var sb string + sb = fmt.Sprintf("%s/%s: total deployment updates %d, expected %d; per-deployment breakdown:\n", + namespace, name, total, expected) + names := make([]string, 0, len(counts)) + for n := range counts { + names = append(names, n) + } + sort.Strings(names) + for _, n := range names { + sb += fmt.Sprintf(" %s: %d\n", n, counts[n]) + } + return sb +} + +var _ = Describe // suppress unused import lint for ginkgo dot-import