diff --git a/.github/workflows/e2e-aks-ci.yml b/.github/workflows/e2e-aks-ci.yml index e4ae9bd26b..387f5d7ad3 100644 --- a/.github/workflows/e2e-aks-ci.yml +++ b/.github/workflows/e2e-aks-ci.yml @@ -137,7 +137,7 @@ jobs: FLEET_E2E_NS: fleet-local run: | export KUBECONFIG="$GITHUB_WORKSPACE/kubeconfig-fleet-ci" - ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources e2e/drift + ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources - name: Acceptance Tests for Examples env: diff --git a/.github/workflows/e2e-ci.yml b/.github/workflows/e2e-ci.yml index e4a47c60e0..59cf16c25f 100644 --- a/.github/workflows/e2e-ci.yml +++ b/.github/workflows/e2e-ci.yml @@ -83,7 +83,7 @@ jobs: env: FLEET_E2E_NS: fleet-local run: | - ginkgo --github-output --label-filter='!infra-setup && !sharding' e2e/single-cluster e2e/keep-resources e2e/drift + ginkgo --github-output --label-filter='!infra-setup && !sharding' e2e/single-cluster e2e/keep-resources - name: E2E Sharding/Metrics Tests if: ${{ matrix.test_type.name == 'sharding' }} diff --git a/.github/workflows/e2e-gke-ci.yml b/.github/workflows/e2e-gke-ci.yml index ed1466afb7..831f43cddd 100644 --- a/.github/workflows/e2e-gke-ci.yml +++ b/.github/workflows/e2e-gke-ci.yml @@ -140,7 +140,7 @@ jobs: env: FLEET_E2E_NS: fleet-local run: | - ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources e2e/drift + ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources - name: Acceptance Tests for Examples env: diff --git a/.github/workflows/e2e-nightly-ci.yml b/.github/workflows/e2e-nightly-ci.yml index 8854c93c74..e22d634f45 100644 --- a/.github/workflows/e2e-nightly-ci.yml +++ b/.github/workflows/e2e-nightly-ci.yml @@ -94,7 +94,7 @@ jobs: export CI_OCI_CERTS_DIR="$(git rev-parse --show-toplevel)/FleetCI-RootCA" # 1. Run test cases not needing infra - ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources e2e/drift + ginkgo --github-output --label-filter='!infra-setup' e2e/single-cluster e2e/keep-resources # 2. Run tests for metrics ginkgo --github-output e2e/metrics diff --git a/e2e/assets/drift/correction-disabled/gitrepo.yaml b/e2e/assets/drift/correction-disabled/gitrepo.yaml deleted file mode 100644 index ef752ca100..0000000000 --- a/e2e/assets/drift/correction-disabled/gitrepo.yaml +++ /dev/null @@ -1,10 +0,0 @@ -kind: GitRepo -apiVersion: fleet.cattle.io/v1alpha1 -metadata: - name: drift-test -spec: - repo: https://github.com/rancher/fleet-test-data - branch: master - paths: - - drift - diff --git a/e2e/assets/drift/correction-enabled/gitrepo.yaml b/e2e/assets/drift/correction-enabled/gitrepo.yaml deleted file mode 100644 index f7cb2a42c4..0000000000 --- a/e2e/assets/drift/correction-enabled/gitrepo.yaml +++ /dev/null @@ -1,12 +0,0 @@ -kind: GitRepo -apiVersion: fleet.cattle.io/v1alpha1 -metadata: - name: drift-correction-test -spec: - repo: https://github.com/rancher/fleet-test-data - branch: master - correctDrift: - enabled: true - paths: - - drift - - drift-ignore-status diff --git a/e2e/assets/drift/force/gitrepo.yaml b/e2e/assets/drift/force/gitrepo.yaml deleted file mode 100644 index df1f6e9a0a..0000000000 --- a/e2e/assets/drift/force/gitrepo.yaml +++ /dev/null @@ -1,13 +0,0 @@ -kind: GitRepo -apiVersion: fleet.cattle.io/v1alpha1 -metadata: - name: drift-force-test -spec: - repo: https://github.com/rancher/fleet-test-data - branch: master - correctDrift: - enabled: true - force: true - paths: - - drift - diff --git a/e2e/drift/drift_test.go b/e2e/drift/drift_test.go deleted file mode 100644 index e342b87bf0..0000000000 --- a/e2e/drift/drift_test.go +++ /dev/null @@ -1,284 +0,0 @@ -package examples_test - -import ( - "encoding/json" - "fmt" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/rancher/fleet/e2e/testenv" - "github.com/rancher/fleet/e2e/testenv/kubectl" - fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" -) - -var _ = Describe("Drift", Ordered, func() { - var ( - asset string - k kubectl.Command - namespace string - bundleName string - ) - - BeforeEach(func() { - k = env.Kubectl.Namespace(env.Namespace) - namespace = "drift" - }) - - JustBeforeEach(func() { - out, err := k.Namespace(env.Namespace).Apply("-f", testenv.AssetPath(asset)) - Expect(err).ToNot(HaveOccurred(), out) - - var bundle fleet.Bundle - Eventually(func() int { - bundle = getBundle(bundleName, k) - return bundle.Status.Summary.Ready - }).Should(Equal(1), fmt.Sprintf("Summary: %+v", bundle.Status.Summary)) - - defer func() { - if r := recover(); r != nil { - bundle := getBundle(bundleName, k) - GinkgoWriter.Printf("bundle status: %v", bundle.Status) - } - }() - }) - - AfterEach(func() { - out, err := k.Namespace(env.Namespace).Delete("-f", testenv.AssetPath(asset), "--wait") - Expect(err).ToNot(HaveOccurred(), out) - }) - - AfterAll(func() { - _, _ = k.Delete("ns", namespace) - _, _ = k.Delete("ns", "drift-ignore-status") - }) - - When("Drift correction is not enabled", func() { - BeforeEach(func() { - asset = "drift/correction-disabled/gitrepo.yaml" - bundleName = "drift-test-drift" - }) - - Context("Modifying externalName in service resource", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "service", "drift-dummy-service", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/spec/externalName", "value": "modified"}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - It("Bundle is modified", func() { - Eventually(func() bool { - b := getBundle(bundleName, k) - return b.Status.Summary.Modified == 1 - }).Should(BeTrue()) - By("Changes haven't been rolled back") - kw := k.Namespace(namespace) - out, _ := kw.Get("services", "drift-dummy-service", "-o=json") - var service corev1.Service - _ = json.Unmarshal([]byte(out), &service) - Expect(service.Spec.ExternalName).Should(Equal("modified")) - }) - }) - }) - - When("Drift correction is enabled without force", func() { - BeforeEach(func() { - asset = "drift/correction-enabled/gitrepo.yaml" - bundleName = "drift-correction-test-drift" - }) - Context("Modifying externalName in service", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "service", "drift-dummy-service", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/spec/externalName", "value": "modified"}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - It("Drift is corrected", func() { - Eventually(func() bool { - b := getBundle(bundleName, k) - return b.Status.Summary.Ready == 1 - }).Should(BeTrue()) - Eventually(func() bool { - kw := k.Namespace(namespace) - out, _ := kw.Get("services", "drift-dummy-service", "-o=json") - var service corev1.Service - _ = json.Unmarshal([]byte(out), &service) - return service.Spec.ExternalName == "drift-dummy" - }).Should(BeTrue()) - }) - }) - - Context("Modifying image in deployment", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "deployment", "drift-dummy-deployment", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/spec/template/spec/containers/0/image", "value": "foo:modified"}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - It("Drift is corrected", func() { - Eventually(func() bool { - b := getBundle(bundleName, k) - return b.Status.Summary.Ready == 1 - }).Should(BeTrue()) - Eventually(func() bool { - kw := k.Namespace(namespace) - out, _ := kw.Get("deployment", "drift-dummy-deployment", "-o=json") - var deployment appsv1.Deployment - _ = json.Unmarshal([]byte(out), &deployment) - return deployment.Spec.Template.Spec.Containers[0].Image == "k8s.gcr.io/pause" - }).Should(BeTrue()) - }) - }) - - Context("Modifying data in configmap", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "configmap", "configmap", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/data/foo", "value": "modified"}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - It("Drift is corrected", func() { - Eventually(func() bool { - b := getBundle(bundleName, k) - return b.Status.Summary.Ready == 1 - }).Should(BeTrue()) - Eventually(func() bool { - kw := k.Namespace(namespace) - out, _ := kw.Get("configmap", "configmap", "-o=json") - var configMap corev1.ConfigMap - _ = json.Unmarshal([]byte(out), &configMap) - return configMap.Data["foo"] == "bar" - }).Should(BeTrue()) - - kw := k.Namespace(namespace) - out, err := kw.Get( - "secrets", - "--field-selector=type=helm.sh/release.v1", - `-o=go-template={{printf "%d" (len .items)}}`, - ) - Expect(err).ToNot(HaveOccurred(), out) - Expect(out).To(Equal("2")) // Max Helm history - }) - }) - - // Helm rollback uses three-way merge by default (without force), which fails when trying to rollback a change made on an item in the ports array. - Context("Modifying port in service", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "service", "drift-dummy-service", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/spec/ports/0/port", "value": 1234}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - // Note: more accurate checks on status changes are now done in integration tests. - It("Corrects drift when drift correction is set to force", func() { - Eventually(func() string { - out, _ := k.Namespace(env.Namespace).Get("bundles", bundleName, "-o=jsonpath={.status.conditions[*].message}") - return out - }).Should(ContainSubstring(`service.v1 drift/drift-dummy-service modified`)) - - out, err := k.Patch( - "gitrepo", - "drift-correction-test", - "--type=merge", - "-p", - `{"spec":{"correctDrift":{"force": true}}}`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - Eventually(func() string { - out, _ := k.Namespace(env.Namespace).Get("bundles", bundleName, "-o=jsonpath={.status.conditions[*].message}") - return out - }).ShouldNot(ContainSubstring(`drift-dummy-service modified`)) - }) - }) - - Context("Resource manifests containing status fields", func() { - // Status must be ignored for drift correction, despite being part of the manifests - It("Is marked as ready", func() { - bundleName := "drift-correction-test-drift-ignore-status" - - var bundle fleet.Bundle - Eventually(func() int { - bundle = getBundle(bundleName, k) - return bundle.Status.Summary.Ready - }).Should(Equal(1), fmt.Sprintf("Summary: %+v", bundle.Status.Summary)) - }) - }) - }) - - When("Drift correction is enabled with force", func() { - BeforeEach(func() { - asset = "drift/force/gitrepo.yaml" - bundleName = "drift-force-test-drift" - }) - - //Helm rollback does a PUT to override all resources when --force is used. - Context("Modifying port in service", func() { - JustBeforeEach(func() { - kw := k.Namespace(namespace) - out, err := kw.Patch( - "service", "drift-dummy-service", - "-o=json", - "--type=json", - "-p", `[{"op": "replace", "path": "/spec/ports/0/port", "value": 5678}]`, - ) - Expect(err).ToNot(HaveOccurred(), out) - GinkgoWriter.Print(out) - }) - - It("Bundle Status is Ready, and changes are rolled back", func() { - var bundle fleet.Bundle - Eventually(func() int { - bundle = getBundle(bundleName, k) - return bundle.Status.Summary.Ready - }).Should(Equal(1), fmt.Sprintf("Summary: %+v", bundle.Status.Summary)) - Eventually(func() bool { - kw := k.Namespace(namespace) - out, _ := kw.Get("services", "drift-dummy-service", "-o=json") - var service corev1.Service - _ = json.Unmarshal([]byte(out), &service) - return service.Spec.Ports[0].Port == 80 - }).Should(BeTrue()) - }) - }) - }) -}) - -func getBundle(bundleName string, k kubectl.Command) fleet.Bundle { - out, _ := k.Namespace(env.Namespace).Get("bundles", bundleName, "-o=json") - var bundle fleet.Bundle - _ = json.Unmarshal([]byte(out), &bundle) - - return bundle -} diff --git a/e2e/drift/suite_test.go b/e2e/drift/suite_test.go deleted file mode 100644 index 01156ab12c..0000000000 --- a/e2e/drift/suite_test.go +++ /dev/null @@ -1,27 +0,0 @@ -package examples_test - -import ( - "testing" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/rancher/fleet/e2e/testenv" -) - -func TestE2E(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "E2E Suite for drift correction") -} - -var ( - env *testenv.Env -) - -var _ = BeforeSuite(func() { - SetDefaultEventuallyTimeout(testenv.Timeout) - SetDefaultEventuallyPollingInterval(time.Second) - testenv.SetRoot("../..") - - env = testenv.New() -}) diff --git a/e2e/single-cluster/status_test.go b/e2e/single-cluster/status_test.go index 52b48bf47f..83c692891d 100644 --- a/e2e/single-cluster/status_test.go +++ b/e2e/single-cluster/status_test.go @@ -2,7 +2,6 @@ package singlecluster_test import ( "errors" - "fmt" "strings" . "github.com/onsi/ginkgo/v2" @@ -107,53 +106,4 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered }) }) - - When("bundle is deleted", func() { - BeforeEach(func() { - targetNamespace = "my-custom-namespace" - }) - - It("correctly updates the status fields for GitRepos", func() { - Eventually(func(g Gomega) { - out, err := k.Delete("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local") - g.Expect(err).ToNot(HaveOccurred(), out) - }).Should((Succeed())) - - Eventually(func() error { - out, err := k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") - if err != nil { - return err - } - - expectedDesiredReady := "\"desiredReady\":0" - if !strings.Contains(out, expectedDesiredReady) { - return fmt.Errorf("expected %q not found in %q", expectedDesiredReady, out) - } - - expectedReady := "\"ready\":0" - if !strings.Contains(out, expectedReady) { - return fmt.Errorf("expected %q not found in %q", expectedReady, out) - } - - out, err = k.Get( - "gitrepo", - "my-gitrepo", - "-n", - "fleet-local", - "-o", - "jsonpath='{.status.display}'", - ) - if err != nil { - return err - } - - expectedReadyBD := "\"readyBundleDeployments\":\"0/0\"" - if !strings.Contains(out, expectedReadyBD) { - return fmt.Errorf("expected %q not found in %q", expectedReadyBD, out) - } - - return nil - }).ShouldNot(HaveOccurred()) - }) - }) }) diff --git a/integrationtests/agent/assets/deployment-with-deployment.yaml b/integrationtests/agent/assets/deployment-with-deployment.yaml new file mode 100644 index 0000000000..a46f8206ea --- /dev/null +++ b/integrationtests/agent/assets/deployment-with-deployment.yaml @@ -0,0 +1,21 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: drift-dummy-deployment + labels: + app: drift-dummy +spec: + replicas: 1 + selector: + matchLabels: + app: drift-dummy + template: + metadata: + labels: + app: drift-dummy + spec: + containers: + - name: pause + image: k8s.gcr.io/pause + ports: + - containerPort: 80 diff --git a/integrationtests/agent/assets/deployment-with-status.yaml b/integrationtests/agent/assets/deployment-with-status.yaml new file mode 100644 index 0000000000..f2c76f01ca --- /dev/null +++ b/integrationtests/agent/assets/deployment-with-status.yaml @@ -0,0 +1,21 @@ +# This deployment is meant to represent the bug at https://github.com/rancher/fleet/issues/2521 +# It includes: +# - A spec field set to its default value (spec.publishNotReadyAddresses in this case) +# - A non-empty "status", despite being a subresource and not modifiable by apply +apiVersion: v1 +kind: Service +metadata: + name: svc-status-test +spec: + publishNotReadyAddresses: false + selector: + app.kubernetes.io/name: MyApp + ports: + - protocol: TCP + port: 80 + targetPort: 9376 + name: myport +status: + loadBalancer: + ingress: + - hostname: foo.bar diff --git a/integrationtests/agent/bundle_deployment_drift_test.go b/integrationtests/agent/bundle_deployment_drift_test.go index dd421afae6..7990bb2cd1 100644 --- a/integrationtests/agent/bundle_deployment_drift_test.go +++ b/integrationtests/agent/bundle_deployment_drift_test.go @@ -2,6 +2,9 @@ package agent import ( "context" + "fmt" + "os" + "time" "github.com/rancher/fleet/integrationtests/utils" "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" @@ -9,21 +12,46 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" ) +func init() { + withStatus, _ := os.ReadFile(assetsPath + "/deployment-with-status.yaml") + withDeployment, _ := os.ReadFile(assetsPath + "/deployment-with-deployment.yaml") + + resources["with-status"] = []v1alpha1.BundleResource{ + { + Name: "deployment-with-status.yaml", + Content: string(withStatus), + Encoding: "", + }, + } + + resources["with-deployment"] = []v1alpha1.BundleResource{ + { + Name: "deployment-with-deployment.yaml", + Content: string(withDeployment), + Encoding: "", + }, + } +} + var _ = Describe("BundleDeployment drift correction", Ordered, func() { const svcName = "svc-test" var ( - namespace string - name string - env *specEnv + namespace string + name string + deplID string + env *specEnv + correctDrift v1alpha1.CorrectDrift ) createBundleDeployment := func(name string) { @@ -33,12 +61,10 @@ var _ = Describe("BundleDeployment drift correction", Ordered, func() { Namespace: clusterNS, }, Spec: v1alpha1.BundleDeploymentSpec{ - DeploymentID: "v1", + DeploymentID: deplID, Options: v1alpha1.BundleDeploymentOptions{ DefaultNamespace: namespace, - CorrectDrift: &v1alpha1.CorrectDrift{ - Enabled: true, - }, + CorrectDrift: &correctDrift, Helm: &v1alpha1.HelmOptions{ MaxHistory: 2, }, @@ -64,17 +90,56 @@ var _ = Describe("BundleDeployment drift correction", Ordered, func() { return newNs } - When("Simulating drift", func() { + When("Drift correction is not enabled", func() { BeforeAll(func() { namespace = createNamespace() + deplID = "v1" + correctDrift = v1alpha1.CorrectDrift{Enabled: false} + env = &specEnv{namespace: namespace} + + name = "drift-disabled-test" + createBundleDeployment(name) + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + DeferCleanup(func() { - Expect(k8sClient.Delete(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: namespace}})).ToNot(HaveOccurred()) + Expect(k8sClient.Delete(context.TODO(), &v1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNS, Name: name}, + })).ToNot(HaveOccurred()) }) + }) + + Context("Modifying externalName in service resource", func() { + It("Receives a modification on a service", func() { + svc, err := env.getService(svcName) + Expect(err).NotTo(HaveOccurred()) + patchedSvc := svc.DeepCopy() + patchedSvc.Spec.ExternalName = "modified" + Expect(k8sClient.Patch(ctx, patchedSvc, client.StrategicMergeFrom(&svc))).NotTo(HaveOccurred()) + }) + + It("Preserves the modification on the service", func() { + Consistently(func(g Gomega) { + svc, err := env.getService(svcName) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(svc.Spec.ExternalName).Should(Equal("modified")) + }, 2*time.Second, 100*time.Millisecond) + }) + }) + }) + + When("Drift correction is enabled without force", func() { + JustBeforeEach(func() { + correctDrift = v1alpha1.CorrectDrift{Enabled: true} env = &specEnv{namespace: namespace} - name = "drift-test" createBundleDeployment(name) + + // deployment resources cannot be ready as they rely on being able to pull Docker images + if name != "drift-deployment-image-test" { + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + } + DeferCleanup(func() { Expect(k8sClient.Delete(context.TODO(), &v1alpha1.BundleDeployment{ ObjectMeta: metav1.ObjectMeta{Namespace: clusterNS, Name: name}, @@ -82,41 +147,139 @@ var _ = Describe("BundleDeployment drift correction", Ordered, func() { }) }) - It("Deploys a bundle deployment which is not ready while its resources are being deployed", func() { - bd := &v1alpha1.BundleDeployment{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: clusterNS, Name: name}, bd) - Expect(err).NotTo(HaveOccurred()) - Expect(bd.Status.Ready).To(BeFalse()) + Context("Modifying externalName in a service resource", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-service-externalname-test" + deplID = "v1" + }) + + It("Corrects drift", func() { + By("Receiving a modification on a service") + svc, err := env.getService(svcName) + Expect(err).NotTo(HaveOccurred()) + patchedSvc := svc.DeepCopy() + patchedSvc.Spec.ExternalName = "modified" + Expect(k8sClient.Patch(ctx, patchedSvc, client.StrategicMergeFrom(&svc))).NotTo(HaveOccurred()) + + By("Restoring the service resource to its previous state") + Eventually(func(g Gomega) { + svc, err := env.getService(svcName) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(svc.Spec.ExternalName).Should(Equal("svc-test")) + }) + }) }) - It("Updates the bundle deployment which will eventually be ready and non modified", func() { - Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + Context("Modifying image in a deployment resource", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-deployment-image-test" + deplID = "with-deployment" + }) + + It("Corrects drift", func() { + By("Receiving a modification on a deployment") + dpl := appsv1.Deployment{} + nsn := types.NamespacedName{ + Namespace: namespace, + Name: "drift-dummy-deployment", + } + + Eventually(func(g Gomega) { + bd := &v1alpha1.BundleDeployment{} + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: clusterNS, Name: name}, bd, &client.GetOptions{}) + // The bundle deployment will not be ready, because no image can be pulled for + // the deployment in envtest clusters. + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, nsn, &dpl) + g.Expect(err).ToNot(HaveOccurred()) + }).Should(Succeed()) + + patchedDpl := dpl.DeepCopy() + patchedDpl.Spec.Template.Spec.Containers[0].Image = "foo:modified" + Expect(k8sClient.Patch(ctx, patchedDpl, client.StrategicMergeFrom(&dpl))).NotTo(HaveOccurred()) + + By("Restoring the deployment resource to its previous state") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, nsn, &dpl) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(dpl.Spec.Template.Spec.Containers[0].Image).Should(Equal("k8s.gcr.io/pause")) + }) + }) }) - It("Creates resources from the bundle deployment in the cluster", func() { - svc, err := env.getService(svcName) - Expect(err).NotTo(HaveOccurred()) - Expect(svc.Name).NotTo(BeEmpty()) + Context("Modifying data in a config map resource", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-configmap-data-test" + deplID = "v1" + }) + + It("Corrects drift", func() { + By("Receiving a modification on a config map") + nsn := types.NamespacedName{ + Namespace: env.namespace, + Name: "cm-test", + } + cm := corev1.ConfigMap{} + err := k8sClient.Get(ctx, nsn, &cm) + Expect(err).ToNot(HaveOccurred()) + + patchedCM := cm.DeepCopy() + patchedCM.Data["foo"] = "modified" + Expect(k8sClient.Patch(ctx, patchedCM, client.StrategicMergeFrom(&cm))).NotTo(HaveOccurred()) + + By("Restoring the config map resource to its previous state") + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, nsn, &cm) + Expect(err).ToNot(HaveOccurred()) + + g.Expect(cm.Data["foo"]).Should(Equal("bar")) + }) + + By("Leaving at most 2 Helm history items") + secrets := corev1.SecretList{} + err = k8sClient.List( + ctx, + &secrets, + client.MatchingFieldsSelector{ + Selector: fields.SelectorFromSet(map[string]string{"type": "helm.sh/release.v1"}), + }, + client.InNamespace(env.namespace), + ) + Expect(err).ToNot(HaveOccurred()) + Expect(len(secrets.Items) <= 2).To(BeTrue(), fmt.Sprintf("Expected %#v to contain at most 2 items", secrets.Items)) + }) }) - It("Lists deployed resources in the bundle deployment status", func() { - bd := &v1alpha1.BundleDeployment{} - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: clusterNS, Name: name}, bd) - Expect(err).NotTo(HaveOccurred()) - Expect(bd.Status.Resources).To(HaveLen(3)) - ts := bd.Status.Resources[0].CreatedAt - Expect(ts.Time).ToNot(BeZero()) - Expect(bd.Status.Resources).To(ContainElement(v1alpha1.BundleDeploymentResource{ - Kind: "Service", - APIVersion: "v1", - Namespace: namespace, - Name: "svc-test", - CreatedAt: ts, - })) + // Status must be ignored for drift correction, despite being part of the manifests + Context("Resource manifests containing status fields", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-status-ignore-test" + deplID = "with-status" + }) + + It("Marks the bundle deployment as ready", func() { + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + }) }) - Context("A release resource is modified", Ordered, func() { - It("Receives a modification on a service", func() { + // Helm rollback uses three-way merge by default (without force), which fails when trying to rollback a + // change made on an item in the ports array. + Context("Drift correction fails", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-test" + deplID = "v1" + }) + + It("Updates the BundleDeployment status as not Ready, including the error message", func() { + By("Receiving a modification on a service") svc, err := env.getService(svcName) Expect(err).NotTo(HaveOccurred()) patchedSvc := svc.DeepCopy() @@ -124,9 +287,8 @@ var _ = Describe("BundleDeployment drift correction", Ordered, func() { patchedSvc.Spec.Ports[0].Port = 4242 patchedSvc.Spec.Ports[0].Name = "myport" Expect(k8sClient.Patch(ctx, patchedSvc, client.StrategicMergeFrom(&svc))).NotTo(HaveOccurred()) - }) - It("Updates the BundleDeployment status as not Ready, including the error message", func() { + By("Updating the bundle deployment status") Eventually(func(g Gomega) { modifiedStatus := v1alpha1.ModifiedStatus{ Kind: "Service", @@ -144,6 +306,82 @@ var _ = Describe("BundleDeployment drift correction", Ordered, func() { ) g.Expect(isOK).To(BeTrue(), status) }).Should(Succeed()) + + By("Correcting drift once drift correction is set to force") + nsn := types.NamespacedName{Namespace: clusterNS, Name: name} + bd := v1alpha1.BundleDeployment{} + + err = k8sClient.Get(ctx, nsn, &bd, &client.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + + patchedBD := bd.DeepCopy() + patchedBD.Spec.CorrectDrift.Force = true + Expect(k8sClient.Patch(ctx, patchedBD, client.MergeFrom(&bd))).NotTo(HaveOccurred()) + + By("Restoring the service resource to its previous state") + Eventually(func(g Gomega) { + err = k8sClient.Get(ctx, nsn, &bd, &client.GetOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + + svc, err := env.getService(svcName) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(svc.Spec.Ports).ToNot(BeEmpty()) + g.Expect(svc.Spec.Ports[0].Port).Should(Equal(80)) + g.Expect(svc.Spec.Ports[0].TargetPort).Should(Equal(9376)) + g.Expect(svc.Spec.Ports[0].Name).Should(Equal("myport")) + }) + + By("Updating the bundle deployment status to be ready and not modified") + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + }) + }) + }) + + When("Drift correction is enabled with force", func() { + JustBeforeEach(func() { + correctDrift = v1alpha1.CorrectDrift{Enabled: true, Force: true} + env = &specEnv{namespace: namespace} + + createBundleDeployment(name) + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) + + DeferCleanup(func() { + Expect(k8sClient.Delete(context.TODO(), &v1alpha1.BundleDeployment{ + ObjectMeta: metav1.ObjectMeta{Namespace: clusterNS, Name: name}, + })).ToNot(HaveOccurred()) + }) + }) + + Context("Modifying a port in a service resource", func() { + BeforeEach(func() { + namespace = createNamespace() + name = "drift-force-service-port-test" + }) + + It("Corrects drift", func() { + By("Receiving a modification on a service") + svc, err := env.getService(svcName) + Expect(err).NotTo(HaveOccurred()) + patchedSvc := svc.DeepCopy() + patchedSvc.Spec.Ports[0].TargetPort = intstr.FromInt(4242) + patchedSvc.Spec.Ports[0].Port = 4242 + patchedSvc.Spec.Ports[0].Name = "myport" + Expect(k8sClient.Patch(ctx, patchedSvc, client.StrategicMergeFrom(&svc))).NotTo(HaveOccurred()) + + By("Restoring the service resource to its previous state") + Eventually(func(g Gomega) { + svc, err := env.getService(svcName) + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(svc.Spec.Ports).ToNot(BeEmpty()) + g.Expect(svc.Spec.Ports[0].Port).Should(Equal(80)) + g.Expect(svc.Spec.Ports[0].TargetPort).Should(Equal(9376)) + g.Expect(svc.Spec.Ports[0].Name).Should(Equal("myport")) + }) + + By("Updating the bundle deployment status to be ready and not modified") + Eventually(env.isBundleDeploymentReadyAndNotModified).WithArguments(name).Should(BeTrue()) }) }) }) diff --git a/integrationtests/gitjob/controller/controller_test.go b/integrationtests/gitjob/controller/controller_test.go index a7851cc545..3f006a9a8a 100644 --- a/integrationtests/gitjob/controller/controller_test.go +++ b/integrationtests/gitjob/controller/controller_test.go @@ -745,6 +745,7 @@ var _ = Describe("GitRepo Status Fields", func() { It("updates the status fields", func() { bundle := &v1alpha1.Bundle{} + By("Receiving a bundle update") Eventually(func() error { err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) Expect(err).ToNot(HaveOccurred()) @@ -753,10 +754,13 @@ var _ = Describe("GitRepo Status Fields", func() { }).ShouldNot(HaveOccurred()) Expect(bundle.Status.Summary.Ready).ToNot(Equal(1)) + By("Updating the GitRepo status to not ready") err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) Expect(err).ToNot(HaveOccurred()) Expect(gitrepo.Status.Summary.Ready).To(Equal(0)) + // This simulates what the bundle deployment reconciler would do. + By("Updating the Bundle deployment status to ready") bd := &v1alpha1.BundleDeployment{} Eventually(func() error { err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bd) @@ -770,6 +774,7 @@ var _ = Describe("GitRepo Status Fields", func() { return k8sClient.Status().Update(ctx, bd) }).ShouldNot(HaveOccurred()) + By("Updating the GitRepo status to ready") Eventually(func() bool { err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) Expect(err).NotTo(HaveOccurred()) @@ -778,6 +783,18 @@ var _ = Describe("GitRepo Status Fields", func() { err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) Expect(err).ToNot(HaveOccurred()) Expect(gitrepo.Status.Summary.Ready).To(Equal(1)) + + By("Deleting a bundle") + err = k8sClient.Delete(ctx, bundle) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(gitrepo.Status.Summary.Ready).To(Equal(0)) + g.Expect(gitrepo.Status.Summary.DesiredReady).To(Equal(0)) + g.Expect(gitrepo.Status.Display.ReadyBundleDeployments).To(Equal("0/0")) + }).Should(Succeed()) }) }) }) diff --git a/internal/cmd/controller/reconciler/bundle_controller.go b/internal/cmd/controller/reconciler/bundle_controller.go index c6a7a5637c..72ae6ce040 100644 --- a/internal/cmd/controller/reconciler/bundle_controller.go +++ b/internal/cmd/controller/reconciler/bundle_controller.go @@ -261,7 +261,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // DependsOn with the bundle's DependsOn (pure function) and replacing // the labels with the bundle's labels for _, target := range matchedTargets { - bd, err := r.createBundle(ctx, logger, target, contentsInOCI, manifestID) + bd, err := r.createBundleDeployment(ctx, logger, target, contentsInOCI, manifestID) if err != nil { return ctrl.Result{}, err } @@ -310,7 +310,7 @@ func upper(op controllerutil.OperationResult) string { } } -func (r *BundleReconciler) createBundle( +func (r *BundleReconciler) createBundleDeployment( ctx context.Context, logger logr.Logger, target *target.Target,