Skip to content

Commit 2efbfb9

Browse files
borisurbanikclaude
borisurbanik
and
claude
committed
Prevent perpetual reconcile by hardcoding K8s-defaulted fields in component specs
The K8s API server fills zero-value fields with defaults when a resource is written. Mutators using reflect.DeepEqual then see a mismatch between the desired spec (Go zero values) and the live spec (K8s-filled defaults), triggering an update on every reconcile cycle. Explicitly set all K8s-defaulted fields in every component Deployment spec: - Probe fields: Scheme, TimeoutSeconds, PeriodSeconds, SuccessThreshold, FailureThreshold - Container / init-container fields: TerminationMessagePath, TerminationMessagePolicy, ImagePullPolicy (init containers are most sensitive — DeploymentPodInitContainerMutator does a full struct DeepEqual) - Pod spec fields: RestartPolicy, DNSPolicy, SecurityContext, TerminationGracePeriodSeconds, SchedulerName - Volume source fields: DefaultMode on Secret, ConfigMap, and Projected volume sources - Use nil (not []T{}) for optional volume/volumemount slices so reflect.DeepEqual treats K8s-normalised absent and locally-absent the same Extend UpdateResource to log the object's namespace and APIManager owner name as structured fields, enabling the integration test's ReconcileCounter to attribute each Deployment update to the correct CR instance. Add ReconcileCounter and verifyNoDeploymentUpdates to the integration test suite to assert ≤50 total Deployment update calls per APIManager install, providing a regression test for this class of bug. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8f7ce52 commit 2efbfb9

15 files changed

Lines changed: 726 additions & 227 deletions

‎controllers/apps/apimanager_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func (r *APIManagerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
160160
return res, nil
161161
}
162162

163-
specResult, specErr := r.reconcileAPIManagerLogic(instance)
163+
specResult, specErr := r.reconcileAPIManagerLogic(r.WithRequest(req), instance)
164164
statusResult, statusErr := r.reconcileAPIManagerStatus(instance, preflightChecksError)
165165
if statusErr != nil {
166166
return ctrl.Result{}, statusErr
@@ -402,8 +402,8 @@ func (r *APIManagerReconciler) setAPIManagerDefaults(cr *appsv1alpha1.APIManager
402402
return ctrl.Result{Requeue: updated}, err
403403
}
404404

405-
func (r *APIManagerReconciler) reconcileAPIManagerLogic(cr *appsv1alpha1.APIManager) (reconcile.Result, error) {
406-
baseAPIManagerLogicReconciler := operator.NewBaseAPIManagerLogicReconciler(r.BaseReconciler, cr)
405+
func (r *APIManagerReconciler) reconcileAPIManagerLogic(b *reconcilers.BaseReconciler, cr *appsv1alpha1.APIManager) (reconcile.Result, error) {
406+
baseAPIManagerLogicReconciler := operator.NewBaseAPIManagerLogicReconciler(b, cr)
407407
imageReconciler := operator.NewAMPImagesReconciler(baseAPIManagerLogicReconciler)
408408
result, err := imageReconciler.Reconcile()
409409
if err != nil || result.Requeue {

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ require (
2828
github.com/spf13/cobra v1.7.0
2929
github.com/spf13/viper v1.7.0
3030
github.com/stretchr/testify v1.9.0
31+
go.uber.org/zap v1.26.0
3132
golang.org/x/mod v0.19.0
3233
k8s.io/api v0.29.0
3334
k8s.io/apimachinery v0.29.0
@@ -101,7 +102,6 @@ require (
101102
github.com/subosito/gotenv v1.2.0 // indirect
102103
go.mongodb.org/mongo-driver v1.14.0 // indirect
103104
go.uber.org/multierr v1.11.0 // indirect
104-
go.uber.org/zap v1.26.0 // indirect
105105
golang.org/x/crypto v0.31.0 // indirect
106106
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
107107
golang.org/x/net v0.33.0 // indirect

‎pkg/3scale/amp/component/apicast.go‎

Lines changed: 83 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
policyv1 "k8s.io/api/policy/v1"
1818
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1919
"k8s.io/apimachinery/pkg/util/intstr"
20+
"k8s.io/utils/ptr"
2021
"sigs.k8s.io/controller-runtime/pkg/client"
2122
)
2223

@@ -139,36 +140,49 @@ func (apicast *Apicast) StagingDeployment(ctx context.Context, k8sclient client.
139140
Annotations: apicast.stagingPodAnnotations(watchedSecretAnnotations),
140141
},
141142
Spec: v1.PodSpec{
142-
Affinity: apicast.Options.StagingAffinity,
143-
Tolerations: apicast.Options.StagingTolerations,
144-
ServiceAccountName: "amp",
145-
Volumes: apicast.stagingVolumes(),
143+
Affinity: apicast.Options.StagingAffinity,
144+
Tolerations: apicast.Options.StagingTolerations,
145+
ServiceAccountName: "amp",
146+
RestartPolicy: v1.RestartPolicyAlways,
147+
DNSPolicy: v1.DNSClusterFirst,
148+
SecurityContext: &v1.PodSecurityContext{},
149+
TerminationGracePeriodSeconds: ptr.To(int64(v1.DefaultTerminationGracePeriodSeconds)),
150+
SchedulerName: v1.DefaultSchedulerName,
151+
Volumes: apicast.stagingVolumes(),
146152
Containers: []v1.Container{
147153
{
148-
Ports: apicast.stagingContainerPorts(),
149-
Env: apicast.buildApicastStagingEnv(),
150-
Image: containerImage,
151-
ImagePullPolicy: v1.PullIfNotPresent,
152-
Name: ApicastStagingName,
153-
Resources: apicast.Options.StagingResourceRequirements,
154-
VolumeMounts: apicast.stagingVolumeMounts(),
154+
Ports: apicast.stagingContainerPorts(),
155+
Env: apicast.buildApicastStagingEnv(),
156+
Image: containerImage,
157+
ImagePullPolicy: v1.PullIfNotPresent,
158+
Name: ApicastStagingName,
159+
Resources: apicast.Options.StagingResourceRequirements,
160+
VolumeMounts: apicast.stagingVolumeMounts(),
161+
TerminationMessagePath: v1.TerminationMessagePathDefault,
162+
TerminationMessagePolicy: v1.TerminationMessageReadFile,
155163
LivenessProbe: &v1.Probe{
156164
ProbeHandler: v1.ProbeHandler{HTTPGet: &v1.HTTPGetAction{
157-
Path: "/status/live",
158-
Port: intstr.FromInt32(8090),
165+
Path: "/status/live",
166+
Port: intstr.FromInt32(8090),
167+
Scheme: v1.URISchemeHTTP,
159168
}},
160169
InitialDelaySeconds: 10,
161170
TimeoutSeconds: 5,
162171
PeriodSeconds: 10,
172+
SuccessThreshold: 1,
173+
FailureThreshold: 3,
163174
},
164175
ReadinessProbe: &v1.Probe{
165176
ProbeHandler: v1.ProbeHandler{HTTPGet: &v1.HTTPGetAction{
166-
Path: "/status/ready",
167-
Port: intstr.FromInt32(8090),
177+
Path: "/status/ready",
178+
Port: intstr.FromInt32(8090),
179+
Scheme: v1.URISchemeHTTP,
168180
}},
169181
InitialDelaySeconds: 15,
170182
TimeoutSeconds: 5,
171183
PeriodSeconds: 30,
184+
SuccessThreshold: 1,
185+
FailureThreshold: 3,
172186
},
173187
},
174188
},
@@ -219,15 +233,23 @@ func (apicast *Apicast) ProductionDeployment(ctx context.Context, k8sclient clie
219233
Annotations: apicast.productionPodAnnotations(watchedSecretAnnotations),
220234
},
221235
Spec: v1.PodSpec{
222-
Affinity: apicast.Options.ProductionAffinity,
223-
Tolerations: apicast.Options.ProductionTolerations,
224-
ServiceAccountName: "amp",
225-
Volumes: apicast.productionVolumes(),
236+
Affinity: apicast.Options.ProductionAffinity,
237+
Tolerations: apicast.Options.ProductionTolerations,
238+
ServiceAccountName: "amp",
239+
RestartPolicy: v1.RestartPolicyAlways,
240+
DNSPolicy: v1.DNSClusterFirst,
241+
SecurityContext: &v1.PodSecurityContext{},
242+
TerminationGracePeriodSeconds: ptr.To(int64(v1.DefaultTerminationGracePeriodSeconds)),
243+
SchedulerName: v1.DefaultSchedulerName,
244+
Volumes: apicast.productionVolumes(),
226245
InitContainers: []v1.Container{
227246
{
228-
Name: ApicastProductionInitContainerName,
229-
Image: containerImage,
230-
Command: []string{"sh", "-c", "until $(curl --output /dev/null --silent --fail --head http://system-master:3000/status); do sleep $SLEEP_SECONDS; done"},
247+
Name: ApicastProductionInitContainerName,
248+
Image: containerImage,
249+
ImagePullPolicy: v1.PullIfNotPresent,
250+
Command: []string{"sh", "-c", "until $(curl --output /dev/null --silent --fail --head http://system-master:3000/status); do sleep $SLEEP_SECONDS; done"},
251+
TerminationMessagePath: v1.TerminationMessagePathDefault,
252+
TerminationMessagePolicy: v1.TerminationMessageReadFile,
231253
Env: []v1.EnvVar{
232254
{
233255
Name: "SLEEP_SECONDS",
@@ -238,30 +260,38 @@ func (apicast *Apicast) ProductionDeployment(ctx context.Context, k8sclient clie
238260
},
239261
Containers: []v1.Container{
240262
{
241-
Ports: apicast.productionContainerPorts(),
242-
Env: apicast.buildApicastProductionEnv(),
243-
Image: containerImage,
244-
ImagePullPolicy: v1.PullIfNotPresent,
245-
Name: ApicastProductionName,
246-
Resources: apicast.Options.ProductionResourceRequirements,
247-
VolumeMounts: apicast.productionVolumeMounts(),
263+
Ports: apicast.productionContainerPorts(),
264+
Env: apicast.buildApicastProductionEnv(),
265+
Image: containerImage,
266+
ImagePullPolicy: v1.PullIfNotPresent,
267+
Name: ApicastProductionName,
268+
Resources: apicast.Options.ProductionResourceRequirements,
269+
VolumeMounts: apicast.productionVolumeMounts(),
270+
TerminationMessagePath: v1.TerminationMessagePathDefault,
271+
TerminationMessagePolicy: v1.TerminationMessageReadFile,
248272
LivenessProbe: &v1.Probe{
249273
ProbeHandler: v1.ProbeHandler{HTTPGet: &v1.HTTPGetAction{
250-
Path: "/status/live",
251-
Port: intstr.FromInt32(8090),
274+
Path: "/status/live",
275+
Port: intstr.FromInt32(8090),
276+
Scheme: v1.URISchemeHTTP,
252277
}},
253278
InitialDelaySeconds: 10,
254279
TimeoutSeconds: 5,
255280
PeriodSeconds: 10,
281+
SuccessThreshold: 1,
282+
FailureThreshold: 3,
256283
},
257284
ReadinessProbe: &v1.Probe{
258285
ProbeHandler: v1.ProbeHandler{HTTPGet: &v1.HTTPGetAction{
259-
Path: "/status/ready",
260-
Port: intstr.FromInt32(8090),
286+
Path: "/status/ready",
287+
Port: intstr.FromInt32(8090),
288+
Scheme: v1.URISchemeHTTP,
261289
}},
262290
InitialDelaySeconds: 15,
263291
TimeoutSeconds: 5,
264292
PeriodSeconds: 30,
293+
SuccessThreshold: 1,
294+
FailureThreshold: 3,
265295
},
266296
},
267297
},
@@ -618,7 +648,8 @@ func (apicast *Apicast) productionVolumes() []v1.Volume {
618648
Name: customPolicy.VolumeName(),
619649
VolumeSource: v1.VolumeSource{
620650
Secret: &v1.SecretVolumeSource{
621-
SecretName: customPolicy.Secret.Name,
651+
SecretName: customPolicy.Secret.Name,
652+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
622653
},
623654
},
624655
})
@@ -636,6 +667,7 @@ func (apicast *Apicast) productionVolumes() []v1.Volume {
636667
Path: apicast.Options.ProductionTracingConfig.VolumeName(),
637668
},
638669
},
670+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
639671
},
640672
},
641673
})
@@ -646,8 +678,9 @@ func (apicast *Apicast) productionVolumes() []v1.Volume {
646678
Name: OpentelemetryConfigurationVolumeName,
647679
VolumeSource: v1.VolumeSource{
648680
Secret: &v1.SecretVolumeSource{
649-
SecretName: apicast.Options.ProductionOpentelemetry.Secret.Name,
650-
Optional: &[]bool{false}[0],
681+
SecretName: apicast.Options.ProductionOpentelemetry.Secret.Name,
682+
Optional: &[]bool{false}[0],
683+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
651684
},
652685
},
653686
})
@@ -658,7 +691,8 @@ func (apicast *Apicast) productionVolumes() []v1.Volume {
658691
Name: customEnvVolumeName(customEnvSecret),
659692
VolumeSource: v1.VolumeSource{
660693
Secret: &v1.SecretVolumeSource{
661-
SecretName: customEnvSecret.GetName(),
694+
SecretName: customEnvSecret.GetName(),
695+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
662696
},
663697
},
664698
})
@@ -669,7 +703,8 @@ func (apicast *Apicast) productionVolumes() []v1.Volume {
669703
Name: HTTPSCertificatesVolumeName,
670704
VolumeSource: v1.VolumeSource{
671705
Secret: &v1.SecretVolumeSource{
672-
SecretName: *apicast.Options.ProductionHTTPSCertificateSecretName,
706+
SecretName: *apicast.Options.ProductionHTTPSCertificateSecretName,
707+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
673708
},
674709
},
675710
})
@@ -686,7 +721,8 @@ func (apicast *Apicast) stagingVolumes() []v1.Volume {
686721
Name: customPolicy.VolumeName(),
687722
VolumeSource: v1.VolumeSource{
688723
Secret: &v1.SecretVolumeSource{
689-
SecretName: customPolicy.Secret.Name,
724+
SecretName: customPolicy.Secret.Name,
725+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
690726
},
691727
},
692728
})
@@ -704,6 +740,7 @@ func (apicast *Apicast) stagingVolumes() []v1.Volume {
704740
Path: apicast.Options.StagingTracingConfig.VolumeName(),
705741
},
706742
},
743+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
707744
},
708745
},
709746
})
@@ -714,8 +751,9 @@ func (apicast *Apicast) stagingVolumes() []v1.Volume {
714751
Name: OpentelemetryConfigurationVolumeName,
715752
VolumeSource: v1.VolumeSource{
716753
Secret: &v1.SecretVolumeSource{
717-
SecretName: apicast.Options.StagingOpentelemetry.Secret.Name,
718-
Optional: &[]bool{false}[0],
754+
SecretName: apicast.Options.StagingOpentelemetry.Secret.Name,
755+
Optional: &[]bool{false}[0],
756+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
719757
},
720758
},
721759
})
@@ -726,7 +764,8 @@ func (apicast *Apicast) stagingVolumes() []v1.Volume {
726764
Name: customEnvVolumeName(customEnvSecret),
727765
VolumeSource: v1.VolumeSource{
728766
Secret: &v1.SecretVolumeSource{
729-
SecretName: customEnvSecret.GetName(),
767+
SecretName: customEnvSecret.GetName(),
768+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
730769
},
731770
},
732771
})
@@ -737,7 +776,8 @@ func (apicast *Apicast) stagingVolumes() []v1.Volume {
737776
Name: HTTPSCertificatesVolumeName,
738777
VolumeSource: v1.VolumeSource{
739778
Secret: &v1.SecretVolumeSource{
740-
SecretName: *apicast.Options.StagingHTTPSCertificateSecretName,
779+
SecretName: *apicast.Options.StagingHTTPSCertificateSecretName,
780+
DefaultMode: ptr.To(v1.SecretVolumeSourceDefaultMode),
741781
},
742782
},
743783
})

0 commit comments

Comments
 (0)