diff --git a/CLAUDE.md b/CLAUDE.md index 843650f..815f943 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ Reloader is a Kubernetes operator that automatically triggers rolling restarts o - **Duplicate reload suppression**: If a workload references both a ConfigMap and a Secret that are updated in the same controller reconcile cycle, it may get reloaded twice. Could be solved with a per-workload debounce map keyed by namespace/name/resourceVersion, flushed after a short TTL. - **CronJob/Job reload is destructive**: Jobs are deleted and recreated on change, which loses run history. Could instead only annotate the CronJob template without spawning a new Job. - **No per-resource reload rate limiting**: A rapid-fire ConfigMap update (e.g., from a CI pipeline) can trigger many restarts. A cooldown window per resource would help. -- **CSI integration gap**: CSI volumes are watched at the `SecretProviderClassPodStatus` level but the link back to the workload is indirect and may miss edge cases. Needs a direct map from SecretProviderClass → workloads that mount it. +- **CSI integration gap**: CSI volumes are watched at the `SecretProviderClassPodStatus` level, but the link back to the workload is indirect and may miss edge cases. Needs a direct map from SecretProviderClass → workloads that mount it. --- @@ -272,7 +272,7 @@ The `reloader.stakater.com/search` annotation on a workload pairs with `reloader - **Exact `ShouldReload()` precedence**: The code in `pkg/common/common.go` checks annotations in a specific order. The exact tie-breaking when both workload-level and pod-template-level annotations are set should be verified by reading that function fully before making annotation behavior changes. - **CSI → workload mapping**: How exactly does Reloader map a `SecretProviderClassPodStatus` change back to workloads? Is it via the SecretProviderClass name matching an annotation on the workload, or via volume reference scanning? Needs confirmation before adding CSI-related features. -- **`ContainerPatchPathFunc` field**: `RollingUpgradeFuncs` has a `ContainerPatchPathFunc` field but it is not documented — unclear if/how it differs from `ContainersFunc` in patch scenarios. +- **`ContainerPatchPathFunc` field**: `RollingUpgradeFuncs` has a `ContainerPatchPathFunc` field, but it is not documented — unclear if/how it differs from `ContainersFunc` in patch scenarios. - **Webhook vs alert**: `--webhook-url` replaces reloading with a POST request. `ALERT_WEBHOOK_URL` env var sends an alert *after* reloading. These are two different mechanisms; the naming is confusing and easy to conflate. - **Load test scenarios S7–S13**: Only S1, S4, and S6 are confirmed from CI. The behavior and coverage of the remaining scenarios is unknown without reading `test/loadtest/` in full. - **`SyncAfterRestart` semantics**: Flag docs say it "syncs add events after restart" but only if `ReloadOnCreate` is also true. The interaction between these two flags in HA mode (where controllers restart on leader change) needs verification. diff --git a/internal/pkg/util/interface.go b/internal/pkg/util/interface.go index a137873..ba04de2 100644 --- a/internal/pkg/util/interface.go +++ b/internal/pkg/util/interface.go @@ -5,7 +5,6 @@ import ( "strconv" "github.com/sirupsen/logrus" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // InterfaceSlice converts an interface to an interface array @@ -24,20 +23,6 @@ func InterfaceSlice(slice interface{}) []interface{} { return ret } -type ObjectMeta struct { - metav1.ObjectMeta -} - -func ToObjectMeta(kubernetesObject interface{}) ObjectMeta { - objectValue := reflect.ValueOf(kubernetesObject) - fieldName := reflect.TypeOf((*metav1.ObjectMeta)(nil)).Elem().Name() - field, _ := objectValue.FieldByName(fieldName).Interface().(metav1.ObjectMeta) - - return ObjectMeta{ - ObjectMeta: field, - } -} - // ParseBool returns result in bool format after parsing func ParseBool(value interface{}) bool { if reflect.Bool == reflect.TypeOf(value).Kind() { diff --git a/test/e2e/README.md b/test/e2e/README.md index 4629f10..eae94ac 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -12,7 +12,7 @@ make e2e-cleanup # Teardown ## Prerequisites -- Go 1.25+ +- Go 1.26+ - Docker or Podman - [Kind](https://kind.sigs.k8s.io/) 0.20+ - kubectl diff --git a/test/e2e/utils/accessors.go b/test/e2e/utils/accessors.go index 514de99..445f86a 100644 --- a/test/e2e/utils/accessors.go +++ b/test/e2e/utils/accessors.go @@ -28,7 +28,8 @@ var ( if d.Spec.Replicas == nil { return false } - return d.Status.ReadyReplicas == *d.Spec.Replicas && + return d.Status.ObservedGeneration >= d.Generation && + d.Status.ReadyReplicas == *d.Spec.Replicas && d.Status.UpdatedReplicas == *d.Spec.Replicas && d.Status.AvailableReplicas == *d.Spec.Replicas } @@ -46,8 +47,10 @@ var ( return d.Spec.Template.Spec.Containers } DaemonSetIsReady StatusAccessor[*appsv1.DaemonSet] = func(d *appsv1.DaemonSet) bool { - return d.Status.DesiredNumberScheduled > 0 && - d.Status.NumberReady == d.Status.DesiredNumberScheduled + return d.Status.ObservedGeneration >= d.Generation && + d.Status.DesiredNumberScheduled > 0 && + d.Status.NumberReady == d.Status.DesiredNumberScheduled && + d.Status.UpdatedNumberScheduled == d.Status.DesiredNumberScheduled } ) @@ -66,7 +69,9 @@ var ( if s.Spec.Replicas == nil { return false } - return s.Status.ReadyReplicas == *s.Spec.Replicas + return s.Status.ObservedGeneration >= s.Generation && + s.Status.ReadyReplicas == *s.Spec.Replicas && + s.Status.UpdatedReplicas == *s.Spec.Replicas } ) diff --git a/test/e2e/utils/conditions.go b/test/e2e/utils/conditions.go index cd374ce..5736b02 100644 --- a/test/e2e/utils/conditions.go +++ b/test/e2e/utils/conditions.go @@ -39,6 +39,27 @@ func HasPodTemplateAnnotation[T any](accessor PodTemplateAccessor[T], key string } } +// HasPodTemplateAnnotationChanged returns a condition that checks the pod template annotation +// is present AND its value differs from priorValue. If priorValue is empty, any non-empty value +// satisfies the condition (equivalent to HasPodTemplateAnnotation). +// Use this in WaitReloaded to correctly detect a reload after a prior reload has already set the annotation. +func HasPodTemplateAnnotationChanged[T any](accessor PodTemplateAccessor[T], key, priorValue string) Condition[T] { + return func(obj T) bool { + template := accessor(obj) + if template == nil || template.Annotations == nil { + return false + } + v, ok := template.Annotations[key] + if !ok { + return false + } + if priorValue == "" { + return true + } + return v != priorValue + } +} + // HasAnnotation returns a condition that checks for an annotation on the resource. func HasAnnotation[T any](accessor AnnotationAccessor[T], key string) Condition[T] { return func(obj T) bool { @@ -78,6 +99,55 @@ func HasEnvVarPrefix[T any](accessor ContainerAccessor[T], prefix string) Condit } } +// HasEnvVarNamed returns a condition that checks for an env var with exactly the given name. +func HasEnvVarNamed[T any](accessor ContainerAccessor[T], name string) Condition[T] { + return func(obj T) bool { + containers := accessor(obj) + for _, container := range containers { + for _, env := range container.Env { + if env.Name == name { + return true + } + } + } + return false + } +} + +// HasEnvVarPrefixChanged returns a condition that checks for an env var with the given prefix +// whose value has changed from priorValue. If priorValue is empty, any matching env var satisfies +// the condition (equivalent to HasEnvVarPrefix). +// Use this in WaitEnvVar to correctly detect a reload after a prior reload already set the env var. +func HasEnvVarPrefixChanged[T any](accessor ContainerAccessor[T], prefix, priorValue string) Condition[T] { + return func(obj T) bool { + containers := accessor(obj) + for _, container := range containers { + for _, env := range container.Env { + if strings.HasPrefix(env.Name, prefix) { + if priorValue == "" { + return true + } + return env.Value != priorValue + } + } + } + return false + } +} + +// GetEnvVarValueByPrefix returns the value of the first env var with the given prefix +// found across the given containers. Returns empty string if not found. +func GetEnvVarValueByPrefix(containers []corev1.Container, prefix string) string { + for _, c := range containers { + for _, env := range c.Env { + if strings.HasPrefix(env.Name, prefix) { + return env.Value + } + } + } + return "" +} + // IsReady returns a condition that checks if the resource is ready. func IsReady[T any](accessor StatusAccessor[T]) Condition[T] { return func(obj T) bool { diff --git a/test/e2e/utils/helm.go b/test/e2e/utils/helm.go index a2ba2c9..28deb5c 100644 --- a/test/e2e/utils/helm.go +++ b/test/e2e/utils/helm.go @@ -6,7 +6,6 @@ import ( "os/exec" "path/filepath" "strings" - "time" ) // Helm-related constants. @@ -121,18 +120,17 @@ func UndeployReloader(namespace, releaseName string) error { return nil } -// waitForReloaderGone waits for the Reloader deployment to be fully removed. +// waitForReloaderGone waits for the Reloader deployment to be fully removed using kubectl wait. +// This is watch-based (kubectl wait --for=delete) rather than a polling loop. func waitForReloaderGone(namespace, releaseName string) { deploymentName := ReloaderDeploymentName(releaseName) - - for i := 0; i < 30; i++ { - cmd := exec.Command("kubectl", "get", "deployment", deploymentName, "-n", namespace, "--ignore-not-found", "-o", "name") - output, _ := Run(cmd) - if strings.TrimSpace(output) == "" { - return - } - time.Sleep(1 * time.Second) - } + cmd := exec.Command("kubectl", "wait", + "deployment/"+deploymentName, + "--for=delete", + "--namespace", namespace, + "--timeout=120s", + ) + _, _ = Run(cmd) } // cleanupClusterResources removes cluster-scoped resources that might be left over @@ -154,8 +152,6 @@ func cleanupClusterResources(releaseName string) { cmd := exec.Command("kubectl", "delete", res.kind, res.name, "--ignore-not-found", "--wait=true") _, _ = Run(cmd) } - - time.Sleep(500 * time.Millisecond) } // GetTestImage returns the test image from environment or the default. @@ -166,30 +162,41 @@ func GetTestImage() string { return DefaultTestImage } -// GetImageRepository extracts the repository (without tag) from a full image reference. -// Example: "ghcr.io/stakater/reloader:v1.0.0" -> "ghcr.io/stakater/reloader" +// GetImageRepository extracts the repository (without tag or digest) from a full image reference. +// Examples: +// +// "ghcr.io/stakater/reloader:v1.0.0" -> "ghcr.io/stakater/reloader" +// "ghcr.io/stakater/reloader@sha256:abc123" -> "ghcr.io/stakater/reloader" func GetImageRepository(image string) string { - for i := len(image) - 1; i >= 0; i-- { - if image[i] == ':' { - return image[:i] - } - if image[i] == '/' { - break + // Digest-based: repo@sha256:hash — split at '@' + if idx := strings.Index(image, "@"); idx != -1 { + return image[:idx] + } + // Tag-based: repo:tag — split at last ':' only if it comes after the last '/' + if lastColon := strings.LastIndex(image, ":"); lastColon != -1 { + if lastSlash := strings.LastIndex(image, "/"); lastSlash < lastColon { + return image[:lastColon] } } return image } // GetImageTag extracts the tag from a full image reference. -// Example: "ghcr.io/stakater/reloader:v1.0.0" -> "v1.0.0" -// Returns "latest" if no tag is found. +// Examples: +// +// "ghcr.io/stakater/reloader:v1.0.0" -> "v1.0.0" +// "ghcr.io/stakater/reloader@sha256:abc123" -> "sha256:abc123" +// +// Returns "latest" if no tag or digest is found. func GetImageTag(image string) string { - for i := len(image) - 1; i >= 0; i-- { - if image[i] == ':' { - return image[i+1:] - } - if image[i] == '/' { - break + // Digest-based: return everything after '@' + if idx := strings.Index(image, "@"); idx != -1 { + return image[idx+1:] + } + // Tag-based: return everything after last ':' (only if it comes after the last '/') + if lastColon := strings.LastIndex(image, ":"); lastColon != -1 { + if lastSlash := strings.LastIndex(image, "/"); lastSlash < lastColon { + return image[lastColon+1:] } } return "latest" diff --git a/test/e2e/utils/podspec.go b/test/e2e/utils/podspec.go index e843112..263bed9 100644 --- a/test/e2e/utils/podspec.go +++ b/test/e2e/utils/podspec.go @@ -132,24 +132,36 @@ func AddCSIVolume(spec *corev1.PodSpec, containerIdx int, spcName string) { } // AddCSIInitContainer adds an init container that mounts a CSI SecretProviderClass volume. +// The init container is named "init-csi-{spcName}" to avoid collisions when multiple CSI +// volumes are mounted. The volume is only added if not already present (idempotent). // This is distinct from AddCSIVolume which mounts into a regular container. func AddCSIInitContainer(spec *corev1.PodSpec, spcName string) { volumeName := "csi-" + spcName mountPath := "/mnt/secrets-store/" + spcName - spec.Volumes = append(spec.Volumes, corev1.Volume{ - Name: volumeName, - VolumeSource: corev1.VolumeSource{ - CSI: &corev1.CSIVolumeSource{ - Driver: CSIDriverName, - ReadOnly: ptr.To(true), - VolumeAttributes: map[string]string{ - "secretProviderClass": spcName, + + hasVolume := false + for _, v := range spec.Volumes { + if v.Name == volumeName { + hasVolume = true + break + } + } + if !hasVolume { + spec.Volumes = append(spec.Volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + CSI: &corev1.CSIVolumeSource{ + Driver: CSIDriverName, + ReadOnly: ptr.To(true), + VolumeAttributes: map[string]string{ + "secretProviderClass": spcName, + }, }, }, - }, - }) + }) + } spec.InitContainers = append(spec.InitContainers, corev1.Container{ - Name: "init-csi", + Name: "init-csi-" + spcName, Image: DefaultImage, Command: []string{"sh", "-c", "echo init done"}, VolumeMounts: []corev1.VolumeMount{ diff --git a/test/e2e/utils/resources.go b/test/e2e/utils/resources.go index 47ca2b0..7f0fa94 100644 --- a/test/e2e/utils/resources.go +++ b/test/e2e/utils/resources.go @@ -548,44 +548,7 @@ func WithCSIVolume(spcName string) DeploymentOption { // WithInitContainerCSIVolume adds an init container with a CSI volume mount. func WithInitContainerCSIVolume(spcName string) DeploymentOption { return func(d *appsv1.Deployment) { - volumeName := csiVolumeName(spcName) - mountPath := csiMountPath(spcName) - - hasCSIVolume := false - for _, v := range d.Spec.Template.Spec.Volumes { - if v.Name == volumeName { - hasCSIVolume = true - break - } - } - if !hasCSIVolume { - d.Spec.Template.Spec.Volumes = append(d.Spec.Template.Spec.Volumes, corev1.Volume{ - Name: volumeName, - VolumeSource: corev1.VolumeSource{ - CSI: &corev1.CSIVolumeSource{ - Driver: CSIDriverName, - ReadOnly: ptr.To(true), - VolumeAttributes: map[string]string{ - "secretProviderClass": spcName, - }, - }, - }, - }) - } - - initContainer := corev1.Container{ - Name: fmt.Sprintf("init-csi-%s", spcName), - Image: DefaultImage, - Command: []string{"sh", "-c", "echo init done"}, - VolumeMounts: []corev1.VolumeMount{ - { - Name: volumeName, - MountPath: mountPath, - ReadOnly: true, - }, - }, - } - d.Spec.Template.Spec.InitContainers = append(d.Spec.Template.Spec.InitContainers, initContainer) + AddCSIInitContainer(&d.Spec.Template.Spec, spcName) } } diff --git a/test/e2e/utils/testenv.go b/test/e2e/utils/testenv.go index b9d5dd1..063d449 100644 --- a/test/e2e/utils/testenv.go +++ b/test/e2e/utils/testenv.go @@ -32,10 +32,13 @@ type TestEnvironment struct { } // SetupTestEnvironment creates a new test environment with kubernetes clients. -// It creates a unique namespace with the given prefix. +// It creates a unique namespace with the given prefix. The returned env.Cancel must be +// called (e.g., in AfterSuite) to release the child context after env.Cleanup() completes. func SetupTestEnvironment(ctx context.Context, namespacePrefix string) (*TestEnvironment, error) { + childCtx, cancel := context.WithCancel(ctx) env := &TestEnvironment{ - Ctx: ctx, + Ctx: childCtx, + Cancel: cancel, TestImage: GetTestImage(), } diff --git a/test/e2e/utils/watch.go b/test/e2e/utils/watch.go index 4643f84..d380bb8 100644 --- a/test/e2e/utils/watch.go +++ b/test/e2e/utils/watch.go @@ -66,6 +66,9 @@ func WatchUntil[T runtime.Object](ctx context.Context, watchFunc WatchFunc, name opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", name).String() } + const maxReconnectDelay = 5 * time.Second + reconnectDelay := 100 * time.Millisecond + for { select { case <-ctx.Done(): @@ -80,7 +83,10 @@ func WatchUntil[T runtime.Object](ctx context.Context, watchFunc WatchFunc, name select { case <-ctx.Done(): return zero, ErrWatchTimeout - case <-time.After(100 * time.Millisecond): + case <-time.After(reconnectDelay): + if reconnectDelay < maxReconnectDelay { + reconnectDelay *= 2 + } } } } @@ -146,6 +152,9 @@ func WatchUntilDeleted( ResourceVersion: "0", } + const maxReconnectDelay = 5 * time.Second + reconnectDelay := 100 * time.Millisecond + for { select { case <-ctx.Done(): @@ -160,7 +169,10 @@ func WatchUntilDeleted( select { case <-ctx.Done(): return ErrWatchTimeout - case <-time.After(100 * time.Millisecond): + case <-time.After(reconnectDelay): + if reconnectDelay < maxReconnectDelay { + reconnectDelay *= 2 + } } } } diff --git a/test/e2e/utils/workload_adapter.go b/test/e2e/utils/workload_adapter.go index d40700a..bc7f80c 100644 --- a/test/e2e/utils/workload_adapter.go +++ b/test/e2e/utils/workload_adapter.go @@ -156,11 +156,19 @@ func (r *AdapterRegistry) GetStandardWorkloads() []WorkloadType { } } -// GetAllWorkloads returns all registered workload types. +// GetAllWorkloads returns all registered workload types in a canonical, deterministic order. +// Map iteration order in Go is non-deterministic, so this uses a fixed ordering to ensure +// consistent test parameterization across runs. func (r *AdapterRegistry) GetAllWorkloads() []WorkloadType { + canonical := []WorkloadType{ + WorkloadDeployment, WorkloadDaemonSet, WorkloadStatefulSet, + WorkloadCronJob, WorkloadJob, WorkloadArgoRollout, WorkloadDeploymentConfig, + } result := make([]WorkloadType, 0, len(r.adapters)) - for wt := range r.adapters { - result = append(result, wt) + for _, wt := range canonical { + if _, ok := r.adapters[wt]; ok { + result = append(result, wt) + } } return result } diff --git a/test/e2e/utils/workload_argo.go b/test/e2e/utils/workload_argo.go index 24cbcf4..69d5163 100644 --- a/test/e2e/utils/workload_argo.go +++ b/test/e2e/utils/workload_argo.go @@ -55,20 +55,27 @@ func (a *ArgoRolloutAdapter) WaitReady(ctx context.Context, namespace, name stri } // WaitReloaded waits for the Argo Rollout to have the reload annotation using watches. +// Captures the current annotation value first to avoid false positives from prior reloads. func (a *ArgoRolloutAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(RolloutPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(RolloutPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } // WaitEnvVar waits for the Argo Rollout to have a STAKATER_ env var using watches. +// Captures the current env var value first to avoid false positives from prior reloads. func (a *ArgoRolloutAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { + priorValue := "" + if r, err := a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { + priorValue = GetEnvVarValueByPrefix(r.Spec.Template.Spec.Containers, prefix) + } watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.rolloutsClient.ArgoprojV1alpha1().Rollouts(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefix(RolloutContainers, prefix), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefixChanged(RolloutContainers, prefix, priorValue), timeout) return HandleWatchResult(err) } diff --git a/test/e2e/utils/workload_cronjob.go b/test/e2e/utils/workload_cronjob.go index b77cddc..c681cce 100644 --- a/test/e2e/utils/workload_cronjob.go +++ b/test/e2e/utils/workload_cronjob.go @@ -47,11 +47,13 @@ func (a *CronJobAdapter) WaitReady(ctx context.Context, namespace, name string, } // WaitReloaded waits for the CronJob pod template to have the reload annotation using watches. +// Captures the current annotation value first to avoid false positives from prior reloads. func (a *CronJobAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.BatchV1().CronJobs(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(CronJobPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(CronJobPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } diff --git a/test/e2e/utils/workload_daemonset.go b/test/e2e/utils/workload_daemonset.go index d80ce79..4a7a2b1 100644 --- a/test/e2e/utils/workload_daemonset.go +++ b/test/e2e/utils/workload_daemonset.go @@ -47,20 +47,27 @@ func (a *DaemonSetAdapter) WaitReady(ctx context.Context, namespace, name string } // WaitReloaded waits for the DaemonSet to have the reload annotation using watches. +// Captures the current annotation value first to avoid false positives from prior reloads. func (a *DaemonSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(DaemonSetPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(DaemonSetPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } // WaitEnvVar waits for the DaemonSet to have a STAKATER_ env var using watches. +// Captures the current env var value first to avoid false positives from prior reloads. func (a *DaemonSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { + priorValue := "" + if ds, err := a.client.AppsV1().DaemonSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { + priorValue = GetEnvVarValueByPrefix(ds.Spec.Template.Spec.Containers, prefix) + } watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().DaemonSets(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefix(DaemonSetContainers, prefix), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefixChanged(DaemonSetContainers, prefix, priorValue), timeout) return HandleWatchResult(err) } diff --git a/test/e2e/utils/workload_deployment.go b/test/e2e/utils/workload_deployment.go index 1b967b8..f7ef5e3 100644 --- a/test/e2e/utils/workload_deployment.go +++ b/test/e2e/utils/workload_deployment.go @@ -47,20 +47,29 @@ func (a *DeploymentAdapter) WaitReady(ctx context.Context, namespace, name strin } // WaitReloaded waits for the Deployment to have the reload annotation using watches. +// It captures the current annotation value before watching so that a prior reload's annotation +// does not cause a false positive — the condition triggers only when the value changes. func (a *DeploymentAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(DeploymentPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(DeploymentPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } // WaitEnvVar waits for the Deployment to have a STAKATER_ env var using watches. +// It captures the current env var value before watching so that a prior reload's value does not +// cause a false positive — the condition triggers only when the value appears or changes. func (a *DeploymentAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { + priorValue := "" + if d, err := a.client.AppsV1().Deployments(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { + priorValue = GetEnvVarValueByPrefix(d.Spec.Template.Spec.Containers, prefix) + } watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().Deployments(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefix(DeploymentContainers, prefix), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefixChanged(DeploymentContainers, prefix, priorValue), timeout) return HandleWatchResult(err) } diff --git a/test/e2e/utils/workload_openshift.go b/test/e2e/utils/workload_openshift.go index 091f03a..6f758bf 100644 --- a/test/e2e/utils/workload_openshift.go +++ b/test/e2e/utils/workload_openshift.go @@ -57,20 +57,27 @@ func (a *DeploymentConfigAdapter) WaitReady(ctx context.Context, namespace, name } // WaitReloaded waits for the DeploymentConfig to have the reload annotation using watches. +// Captures the current annotation value first to avoid false positives from prior reloads. func (a *DeploymentConfigAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(DeploymentConfigPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(DeploymentConfigPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } // WaitEnvVar waits for the DeploymentConfig to have a STAKATER_ env var using watches. +// Captures the current env var value first to avoid false positives from prior reloads. func (a *DeploymentConfigAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { + priorValue := "" + if dc, err := a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil && dc.Spec.Template != nil { + priorValue = GetEnvVarValueByPrefix(dc.Spec.Template.Spec.Containers, prefix) + } watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.openshiftClient.AppsV1().DeploymentConfigs(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefix(DeploymentConfigContainers, prefix), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefixChanged(DeploymentConfigContainers, prefix, priorValue), timeout) return HandleWatchResult(err) } diff --git a/test/e2e/utils/workload_statefulset.go b/test/e2e/utils/workload_statefulset.go index 53f6fd7..d071678 100644 --- a/test/e2e/utils/workload_statefulset.go +++ b/test/e2e/utils/workload_statefulset.go @@ -47,20 +47,27 @@ func (a *StatefulSetAdapter) WaitReady(ctx context.Context, namespace, name stri } // WaitReloaded waits for the StatefulSet to have the reload annotation using watches. +// Captures the current annotation value first to avoid false positives from prior reloads. func (a *StatefulSetAdapter) WaitReloaded(ctx context.Context, namespace, name, annotationKey string, timeout time.Duration) (bool, error) { + priorValue, _ := a.GetPodTemplateAnnotation(ctx, namespace, name, annotationKey) watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotation(StatefulSetPodTemplate, annotationKey), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasPodTemplateAnnotationChanged(StatefulSetPodTemplate, annotationKey, priorValue), timeout) return HandleWatchResult(err) } // WaitEnvVar waits for the StatefulSet to have a STAKATER_ env var using watches. +// Captures the current env var value first to avoid false positives from prior reloads. func (a *StatefulSetAdapter) WaitEnvVar(ctx context.Context, namespace, name, prefix string, timeout time.Duration) (bool, error) { + priorValue := "" + if sts, err := a.client.AppsV1().StatefulSets(namespace).Get(ctx, name, metav1.GetOptions{}); err == nil { + priorValue = GetEnvVarValueByPrefix(sts.Spec.Template.Spec.Containers, prefix) + } watchFunc := func(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { return a.client.AppsV1().StatefulSets(namespace).Watch(ctx, opts) } - _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefix(StatefulSetContainers, prefix), timeout) + _, err := WatchUntil(ctx, watchFunc, name, HasEnvVarPrefixChanged(StatefulSetContainers, prefix, priorValue), timeout) return HandleWatchResult(err) }