diff --git a/controllers/consoleplugin.go b/controllers/consoleplugin.go index 85de4a528..80673fdd2 100644 --- a/controllers/consoleplugin.go +++ b/controllers/consoleplugin.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "reflect" + "sort" argocommon "github.com/argoproj-labs/argocd-operator/common" argocdutil "github.com/argoproj-labs/argocd-operator/controllers/argoutil" @@ -16,6 +17,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/redhat-developer/gitops-operator/common" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" resourcev1 "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -267,6 +269,74 @@ func pluginConfigMap() *corev1.ConfigMap { return cm } +// normalizeContainerDefaults sets Kubernetes default values for container fields that are +// automatically populated by the API server. This ensures consistent comparison between +// existing containers (from etcd) and new containers (from operator). +func normalizeContainerDefaults(container *corev1.Container) { + if container.TerminationMessagePath == "" { + container.TerminationMessagePath = "/dev/termination-log" + } + if container.TerminationMessagePolicy == "" { + container.TerminationMessagePolicy = corev1.TerminationMessageReadFile + } +} + +// sortContainers creates a sorted copy of containers by name, and nested fields +// (Env, Ports, VolumeMounts) to handle non-deterministic ordering from etcd +func sortContainers(containers []corev1.Container) []corev1.Container { + if len(containers) == 0 { + return containers + } + sorted := make([]corev1.Container, len(containers)) + for i := range containers { + sorted[i] = *containers[i].DeepCopy() + normalizeContainerDefaults(&sorted[i]) + sort.Slice(sorted[i].Env, func(a, b int) bool { + return sorted[i].Env[a].Name < sorted[i].Env[b].Name + }) + sort.Slice(sorted[i].Ports, func(a, b int) bool { + if sorted[i].Ports[a].ContainerPort != sorted[i].Ports[b].ContainerPort { + return sorted[i].Ports[a].ContainerPort < sorted[i].Ports[b].ContainerPort + } + return sorted[i].Ports[a].Name < sorted[i].Ports[b].Name + }) + sort.Slice(sorted[i].VolumeMounts, func(a, b int) bool { + return sorted[i].VolumeMounts[a].Name < sorted[i].VolumeMounts[b].Name + }) + } + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Name < sorted[j].Name + }) + return sorted +} + +// sortVolumes creates a sorted copy of volumes by name +func sortVolumes(volumes []corev1.Volume) []corev1.Volume { + sorted := make([]corev1.Volume, len(volumes)) + copy(sorted, volumes) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Name < sorted[j].Name + }) + return sorted +} + +// sortTolerations creates a sorted copy of tolerations by key, operator, and effect +func sortTolerations(tolerations []corev1.Toleration) []corev1.Toleration { + sorted := make([]corev1.Toleration, len(tolerations)) + copy(sorted, tolerations) + sort.Slice(sorted, func(i, j int) bool { + a, b := sorted[i], sorted[j] + if a.Key != b.Key { + return a.Key < b.Key + } + if a.Operator != b.Operator { + return string(a.Operator) < string(b.Operator) + } + return string(a.Effect) < string(b.Effect) + }) + return sorted +} + func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.GitopsService, request reconcile.Request) (reconcile.Result, error) { reqLogger := logs.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name) newPluginDeployment := pluginDeployment(cr.Spec.ImagePullPolicy) @@ -309,17 +379,19 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop } else { existingSpecTemplate := &existingPluginDeployment.Spec.Template newSpecTemplate := newPluginDeployment.Spec.Template - changed := !reflect.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) || - !reflect.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) || - !reflect.DeepEqual(existingSpecTemplate.Spec.Containers, newSpecTemplate.Spec.Containers) || - !reflect.DeepEqual(existingSpecTemplate.Spec.Volumes, newSpecTemplate.Spec.Volumes) || - !reflect.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) || - !reflect.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) || - !reflect.DeepEqual(existingPluginDeployment.Spec.Template.Spec.Tolerations, newPluginDeployment.Spec.Template.Spec.Tolerations) || - !reflect.DeepEqual(existingSpecTemplate.Spec.SecurityContext, existingSpecTemplate.Spec.SecurityContext) || !reflect.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources) + // Sort list fields before comparing to handle non-deterministic ordering + changed := !equality.Semantic.DeepEqual(existingPluginDeployment.Labels, newPluginDeployment.Labels) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) || + !equality.Semantic.DeepEqual(sortContainers(existingSpecTemplate.Spec.Containers), sortContainers(newSpecTemplate.Spec.Containers)) || + !equality.Semantic.DeepEqual(sortVolumes(existingSpecTemplate.Spec.Volumes), sortVolumes(newSpecTemplate.Spec.Volumes)) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.DNSPolicy, newSpecTemplate.Spec.DNSPolicy) || + !equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Template.Spec.NodeSelector, newPluginDeployment.Spec.Template.Spec.NodeSelector) || + !equality.Semantic.DeepEqual(sortTolerations(existingPluginDeployment.Spec.Template.Spec.Tolerations), sortTolerations(newPluginDeployment.Spec.Template.Spec.Tolerations)) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.SecurityContext, newSpecTemplate.Spec.SecurityContext) || + !equality.Semantic.DeepEqual(existingSpecTemplate.Spec.Containers[0].Resources, newSpecTemplate.Spec.Containers[0].Resources) if changed { reqLogger.Info("Reconciling plugin deployment", "Namespace", existingPluginDeployment.Namespace, "Name", existingPluginDeployment.Name) diff --git a/controllers/consoleplugin_test.go b/controllers/consoleplugin_test.go index 267a907f4..c6f3743e9 100644 --- a/controllers/consoleplugin_test.go +++ b/controllers/consoleplugin_test.go @@ -1656,3 +1656,207 @@ func TestPlug_reconcileConfigMap_changedLabels(t *testing.T) { }) } } + +// Tests that reconciliation does NOT trigger an update when containers are returned in different order from etcd. +func TestReconcileDeployment_NoUpdateWhenContainersOrderDiffers(t *testing.T) { + s := scheme.Scheme + addKnownTypesToScheme(s) + + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(newGitopsService()).Build() + reconciler := newReconcileGitOpsService(fakeClient, s) + instance := &pipelinesv1alpha1.GitopsService{} + + // Create deployment + _, err := reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Get the deployment and capture initial ResourceVersion and Generation + deployment := &appsv1.Deployment{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + initialRV := deployment.ResourceVersion + initialGen := deployment.Generation + + // Simulate etcd returning containers in different order + if len(deployment.Spec.Template.Spec.Containers) >= 2 { + containers := deployment.Spec.Template.Spec.Containers + reversedContainers := make([]corev1.Container, len(containers)) + for i := range containers { + reversedContainers[len(containers)-1-i] = containers[i] + } + deployment.Spec.Template.Spec.Containers = reversedContainers + err = fakeClient.Update(context.TODO(), deployment) + assertNoError(t, err) + } + + // Reconcile again - should NOT trigger an update + _, err = reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Verify no update was triggered + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + assert.Equal(t, deployment.ResourceVersion, initialRV, + "ResourceVersion should NOT change when only container order differs") + assert.Equal(t, deployment.Generation, initialGen, + "Generation should NOT change when only container order differs") +} + +// Tests that reconciliation does NOT trigger an update when volumes are returned in different order from etcd. +func TestReconcileDeployment_NoUpdateWhenVolumesOrderDiffers(t *testing.T) { + s := scheme.Scheme + addKnownTypesToScheme(s) + + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(newGitopsService()).Build() + reconciler := newReconcileGitOpsService(fakeClient, s) + instance := &pipelinesv1alpha1.GitopsService{} + + // Create deployment + _, err := reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Get the deployment + deployment := &appsv1.Deployment{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + // Simulate etcd returning volumes in different order + if len(deployment.Spec.Template.Spec.Volumes) >= 2 { + volumes := deployment.Spec.Template.Spec.Volumes + reversedVolumes := make([]corev1.Volume, len(volumes)) + for i := range volumes { + reversedVolumes[len(volumes)-1-i] = volumes[i] + } + deployment.Spec.Template.Spec.Volumes = reversedVolumes + err = fakeClient.Update(context.TODO(), deployment) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + rvAfterManualUpdate := deployment.ResourceVersion + genAfterManualUpdate := deployment.Generation + + // Reconcile again - should NOT trigger an update + _, err = reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Verify no update was triggered + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + assert.Equal(t, deployment.ResourceVersion, rvAfterManualUpdate, + "ResourceVersion should NOT change when only volume order differs") + assert.Equal(t, deployment.Generation, genAfterManualUpdate, + "Generation should NOT change when only volume order differs") + } else { + t.Skip("Skipping test: deployment has less than 2 volumes, cannot test order difference") + } +} + +// Tests that reconciliation does NOT trigger an update when tolerations are returned in different order from etcd. +func TestReconcileDeployment_NoUpdateWhenTolerationsOrderDiffers(t *testing.T) { + s := scheme.Scheme + addKnownTypesToScheme(s) + + // Create GitopsService with tolerations + gitopsService := &pipelinesv1alpha1.GitopsService{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceName, + }, + Spec: pipelinesv1alpha1.GitopsServiceSpec{ + Tolerations: []corev1.Toleration{ + { + Key: "key-a", + Operator: corev1.TolerationOpEqual, + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "key-b", + Operator: corev1.TolerationOpEqual, + Effect: corev1.TaintEffectNoSchedule, + }, + }, + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(gitopsService).Build() + reconciler := newReconcileGitOpsService(fakeClient, s) + + // Create deployment + _, err := reconciler.reconcileDeployment(gitopsService, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Get the deployment + deployment := &appsv1.Deployment{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + // Simulate etcd returning tolerations in different order + if len(deployment.Spec.Template.Spec.Tolerations) >= 2 { + tolerations := deployment.Spec.Template.Spec.Tolerations + reversedTolerations := make([]corev1.Toleration, len(tolerations)) + for i := range tolerations { + reversedTolerations[len(tolerations)-1-i] = tolerations[i] + } + deployment.Spec.Template.Spec.Tolerations = reversedTolerations + err = fakeClient.Update(context.TODO(), deployment) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + rvAfterManualUpdate := deployment.ResourceVersion + genAfterManualUpdate := deployment.Generation + + // Reconcile again - should NOT trigger an update + _, err = reconciler.reconcileDeployment(gitopsService, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Verify no update was triggered + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + assert.Equal(t, deployment.ResourceVersion, rvAfterManualUpdate, + "ResourceVersion should NOT change when only toleration order differs") + assert.Equal(t, deployment.Generation, genAfterManualUpdate, + "Generation should NOT change when only toleration order differs") + } else { + t.Skip("Skipping test: deployment has less than 2 tolerations, cannot test order difference") + } +} + +// Tests that legitimate changes do trigger updates. +func TestReconcileDeployment_UpdateWhenActualChange(t *testing.T) { + s := scheme.Scheme + addKnownTypesToScheme(s) + + fakeClient := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(newGitopsService()).Build() + reconciler := newReconcileGitOpsService(fakeClient, s) + instance := &pipelinesv1alpha1.GitopsService{} + + // Create deployment + _, err := reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Get the deployment and capture initial ResourceVersion and Generation + deployment := &appsv1.Deployment{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + initialRV := deployment.ResourceVersion + + // Make an actual change + deployment.Spec.Template.Spec.Containers[0].Image = "different-image:tag" + err = fakeClient.Update(context.TODO(), deployment) + assertNoError(t, err) + + // Reconcile again - should trigger an update + _, err = reconciler.reconcileDeployment(instance, newRequest(serviceNamespace, gitopsPluginName)) + assertNoError(t, err) + + // Verify update was triggered + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment) + assertNoError(t, err) + + assert.Assert(t, deployment.ResourceVersion != initialRV, + "ResourceVersion SHOULD change when actual change is made") +} diff --git a/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go b/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go new file mode 100644 index 000000000..801b87066 --- /dev/null +++ b/test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go @@ -0,0 +1,266 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sequential + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + gitopsoperatorv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1" + "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" + deploymentFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/deployment" + gitopsserviceFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/gitopsservice" + k8sFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/k8s" + fixtureUtils "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const testReconciliationTriggerAnnotation = "test-reconciliation-trigger" +const gitopsPluginDeploymentName = "gitops-plugin" +const openshiftGitopsNamespace = "openshift-gitops" + +var _ = Describe("GitOps Operator Sequential E2E Tests", func() { + + Context("1-123_validate_list_order_comparison", func() { + + var ( + k8sClient client.Client + ctx context.Context + runDebug struct { + initialGen, genAfterOrderChange, finalGen int64 + expectedImage, lastPluginImage string + } + ) + + BeforeEach(func() { + fixture.EnsureSequentialCleanSlate() + k8sClient, _ = fixtureUtils.GetE2ETestKubeClient() + ctx = context.Background() + runDebug = struct { + initialGen, genAfterOrderChange, finalGen int64 + expectedImage, lastPluginImage string + }{} + }) + + AfterEach(func() { + if CurrentSpecReport().Failed() { + GinkgoWriter.Println("++++ 1-123 failure debug start ++++") + kubeClient, err := fixtureUtils.GetE2ETestKubeClient() + if err != nil { + GinkgoWriter.Println(fmt.Sprintf("could not get kube client: %v", err)) + } else { + c := context.Background() + gs := &gitopsoperatorv1alpha1.GitopsService{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + gsErr := kubeClient.Get(c, client.ObjectKeyFromObject(gs), gs) + pluginDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: gitopsPluginDeploymentName, Namespace: openshiftGitopsNamespace}} + pluginErr := kubeClient.Get(c, client.ObjectKeyFromObject(pluginDepl), pluginDepl) + readyReplicas, observedGen, deplGen := int32(0), int64(0), int64(0) + if pluginErr == nil { + readyReplicas = pluginDepl.Status.ReadyReplicas + observedGen = pluginDepl.Status.ObservedGeneration + deplGen = pluginDepl.Generation + } + GinkgoWriter.Println(fmt.Sprintf("gs=%v plugin=%v ready=%d gen=%d obs=%d", + gsErr == nil, pluginErr == nil, readyReplicas, deplGen, observedGen)) + if runDebug.finalGen != 0 || runDebug.genAfterOrderChange != 0 { + GinkgoWriter.Println(fmt.Sprintf("list-order: initial=%d afterOrder=%d final=%d (want %d)", + runDebug.initialGen, runDebug.genAfterOrderChange, runDebug.finalGen, runDebug.genAfterOrderChange)) + } + if runDebug.expectedImage != "" { + GinkgoWriter.Println(fmt.Sprintf("image: expected=%q last=%q", + runDebug.expectedImage, runDebug.lastPluginImage)) + } + } + GinkgoWriter.Println("++++ 1-123 failure debug end ++++") + } + fixture.OutputDebugOnFail(openshiftGitopsNamespace) + }) + + It("Should not trigger updates when only list order differs", func() { + gitopsService := &gitopsoperatorv1alpha1.GitopsService{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(gitopsService), gitopsService)).To(Succeed()) + + pluginDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: gitopsPluginDeploymentName, + Namespace: openshiftGitopsNamespace, + }, + } + Eventually(pluginDeployment, "5m", "5s").Should(k8sFixture.ExistByName(), + "deployment %s never showed in %s (5m)", gitopsPluginDeploymentName, openshiftGitopsNamespace) + Eventually(pluginDeployment, "60s", "5s").Should(deploymentFixture.HaveReadyReplicas(1), + "deployment %s in %s not ready after 60s", gitopsPluginDeploymentName, openshiftGitopsNamespace) + + By("capturing initial state before simulating etcd order change") + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment)).To(Succeed()) + initialGen := pluginDeployment.Generation + runDebug.initialGen = initialGen + + hasMultipleContainers := len(pluginDeployment.Spec.Template.Spec.Containers) >= 2 + hasMultipleVolumes := len(pluginDeployment.Spec.Template.Spec.Volumes) >= 2 + hasMultipleTolerations := len(pluginDeployment.Spec.Template.Spec.Tolerations) >= 2 + + if !hasMultipleContainers && !hasMultipleVolumes && !hasMultipleTolerations { + Skip("Deployment does not have multiple containers, volumes, or tolerations to test order differences") + } + + By("simulating etcd returning lists in different order") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment); err != nil { + return err + } + + if hasMultipleContainers { + containers := pluginDeployment.Spec.Template.Spec.Containers + reversed := make([]corev1.Container, len(containers)) + for i := range containers { + reversed[len(containers)-1-i] = containers[i] + } + pluginDeployment.Spec.Template.Spec.Containers = reversed + } + + if hasMultipleVolumes { + volumes := pluginDeployment.Spec.Template.Spec.Volumes + reversed := make([]corev1.Volume, len(volumes)) + for i := range volumes { + reversed[len(volumes)-1-i] = volumes[i] + } + pluginDeployment.Spec.Template.Spec.Volumes = reversed + } + + if hasMultipleTolerations { + tolerations := pluginDeployment.Spec.Template.Spec.Tolerations + reversed := make([]corev1.Toleration, len(tolerations)) + for i := range tolerations { + reversed[len(tolerations)-1-i] = tolerations[i] + } + pluginDeployment.Spec.Template.Spec.Tolerations = reversed + } + + return k8sClient.Update(ctx, pluginDeployment) + }) + Expect(err).ToNot(HaveOccurred()) + + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment)).To(Succeed()) + genAfterManualOrderChange := pluginDeployment.Generation + runDebug.genAfterOrderChange = genAfterManualOrderChange + + By("triggering reconciliation by updating GitopsService CR") + gitopsserviceFixture.Update(gitopsService, func(gs *gitopsoperatorv1alpha1.GitopsService) { + if gs.Annotations == nil { + gs.Annotations = make(map[string]string) + } + gs.Annotations[testReconciliationTriggerAnnotation] = "list-order-test" + }) + time.Sleep(10 * time.Second) + + gitopsserviceFixture.Update(gitopsService, func(gs *gitopsoperatorv1alpha1.GitopsService) { + if gs.Annotations != nil { + delete(gs.Annotations, testReconciliationTriggerAnnotation) + } + }) + time.Sleep(10 * time.Second) + + By("verifying no unnecessary update was triggered") + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment)).To(Succeed()) + finalGen := pluginDeployment.Generation + runDebug.finalGen = finalGen + + Expect(finalGen).To(Equal(genAfterManualOrderChange), + fmt.Sprintf("Generation should not change when only list order differs. Initial: %d, AfterManualOrderChange: %d, Final: %d", initialGen, genAfterManualOrderChange, finalGen)) + + }) + + It("Should trigger updates when actual changes are made", func() { + gitopsService := &gitopsoperatorv1alpha1.GitopsService{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(gitopsService), gitopsService)).To(Succeed()) + + pluginDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: gitopsPluginDeploymentName, + Namespace: openshiftGitopsNamespace, + }, + } + Eventually(pluginDeployment, "5m", "5s").Should(k8sFixture.ExistByName(), + "deployment %s never showed in %s (5m)", gitopsPluginDeploymentName, openshiftGitopsNamespace) + Eventually(pluginDeployment, "60s", "5s").Should(deploymentFixture.HaveReadyReplicas(1), + "deployment %s in %s not ready after 60s", gitopsPluginDeploymentName, openshiftGitopsNamespace) + + By("capturing initial state before making actual change") + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment)).To(Succeed()) + initialGen := pluginDeployment.Generation + pluginContainer := deploymentFixture.GetTemplateSpecContainerByName(gitopsPluginDeploymentName, *pluginDeployment) + Expect(pluginContainer).ToNot(BeNil(), "deployment should have container %q", gitopsPluginDeploymentName) + expectedImage := pluginContainer.Image + + By("making an actual change to the deployment") + deploymentFixture.Update(pluginDeployment, func(d *appsv1.Deployment) { + for i := range d.Spec.Template.Spec.Containers { + if d.Spec.Template.Spec.Containers[i].Name == gitopsPluginDeploymentName { + d.Spec.Template.Spec.Containers[i].Image = "wrong-image:wrong-tag" + break + } + } + }) + + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment)).To(Succeed()) + genAfterChange := pluginDeployment.Generation + Expect(genAfterChange).ToNot(Equal(initialGen)) + + time.Sleep(15 * time.Second) + + By("triggering reconciliation") + gitopsserviceFixture.Update(gitopsService, func(gs *gitopsoperatorv1alpha1.GitopsService) { + if gs.Annotations == nil { + gs.Annotations = make(map[string]string) + } + gs.Annotations[testReconciliationTriggerAnnotation] = "actual-change-test" + }) + time.Sleep(15 * time.Second) + + gitopsserviceFixture.Update(gitopsService, func(gs *gitopsoperatorv1alpha1.GitopsService) { + if gs.Annotations != nil { + delete(gs.Annotations, testReconciliationTriggerAnnotation) + } + }) + + By("verifying operator corrected the image back to the expected image") + Eventually(func() bool { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(pluginDeployment), pluginDeployment); err != nil { + return false + } + c := deploymentFixture.GetTemplateSpecContainerByName(gitopsPluginDeploymentName, *pluginDeployment) + if c == nil { + return false + } + runDebug.lastPluginImage = c.Image + return c.Image == expectedImage + }, "5m", "5s").Should(BeTrue(), "Operator should restore the image of container %q to %q within 5m", gitopsPluginDeploymentName, expectedImage) + }) + }) +})