From b529f3a7277fb627c38ffb8ddc862bef0e528bde Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Fri, 2 Feb 2024 13:39:22 +0000 Subject: [PATCH] Add Tests and CI config Adds all DNS related unit and integration tests from MGC repo (There aren't many :-/) Add configuration for running tests locally and via CI: * Add test-unit and test-integration make targets(Tests must be tagged correctly unit vs integration) * Use actions/setup-go@v5 (Uses node20 and avoids warnings) * Add patch to `make bundle` to avoid changing createdAt even when nothing else has updated. * Add act tool install and make targets to help verify GH actions locally. * Add integration_test_suite job to CI workflow. --- .github/workflows/ci.yaml | 26 +++-- Makefile | 30 ++++- .../dnsheathcheckprobe_controller_test.go | 98 +++++++++++++++++ internal/controller/helper_test.go | 48 ++++++++ .../controller/managedzone_controller_test.go | 104 ++++++++++++++++++ internal/controller/suite_test.go | 81 ++++++++++++++ internal/provider/google/google_test.go | 2 +- make/act.mk | 32 ++++++ 8 files changed, 411 insertions(+), 10 deletions(-) create mode 100644 internal/controller/dnsheathcheckprobe_controller_test.go create mode 100644 internal/controller/helper_test.go create mode 100644 internal/controller/managedzone_controller_test.go create mode 100644 make/act.mk diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ef79f423..88588d5f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -28,7 +28,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-go@v4 + - uses: actions/setup-go@v5 with: go-version: v1.21.x cache: false @@ -42,7 +42,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Set up Go 1.21.x - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false @@ -57,7 +57,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Set up Go 1.21.x - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false @@ -72,7 +72,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Set up Go 1.21.x - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false @@ -87,7 +87,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Set up Go 1.21.x - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false @@ -102,10 +102,22 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-go@v4 + - uses: actions/setup-go@v5 with: go-version: v1.21 cache: false - name: Run suite run: | - make test + make test-unit + integration_test_suite: + name: Integration Test Suite + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: v1.21 + cache: false + - name: Run suite + run: | + make test-integration diff --git a/Makefile b/Makefile index 11c649e2..eec2cee4 100644 --- a/Makefile +++ b/Makefile @@ -120,8 +120,15 @@ imports: openshift-goimports ## Run openshift goimports against code. $(OPENSHIFT_GOIMPORTS) -m github.com/kuadrant/kuadrant-dns-operator -i github.com/kuadrant/kuadrant-operator .PHONY: test -test: manifests generate fmt vet envtest ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out +test: test-unit test-integration ## Run tests. + +.PHONY: test-unit +test-unit: manifests generate fmt vet ## Run unit tests. + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -tags=unit -coverprofile cover-unit.out + +.PHONY: test-integration +test-integration: manifests generate fmt vet envtest ## Run integration tests. + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./internal/controller... -tags=integration -coverprofile cover-integration.out .PHONY: local-setup local-setup: $(KIND) ## Setup local development kind cluster and dependencies @@ -206,12 +213,14 @@ CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest OPENSHIFT_GOIMPORTS ?= $(LOCALBIN)/openshift-goimports KIND = $(LOCALBIN)/kind +ACT = $(LOCALBIN)/act ## Tool Versions KUSTOMIZE_VERSION ?= v5.0.1 CONTROLLER_TOOLS_VERSION ?= v0.12.0 OPENSHIFT_GOIMPORTS_VERSION ?= c70783e636f2213cac683f6865d88c5edace3157 KIND_VERSION = v0.20.0 +ACT_VERSION = latest .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. If wrong version is installed, it will be removed before downloading. @@ -260,12 +269,29 @@ kind: $(KIND) ## Download kind locally if necessary. $(KIND): $(LOCALBIN) GOBIN=$(LOCALBIN) go install sigs.k8s.io/kind@$(KIND_VERSION) +.PHONY: act +act: $(ACT) +$(ACT): $(LOCALBIN) ## Download act locally if necessary. + GOBIN=$(LOCALBIN) go install github.com/nektos/act@$(ACT_VERSION) + .PHONY: bundle bundle: manifests kustomize operator-sdk ## Generate bundle manifests and metadata, then validate generated files. $(OPERATOR_SDK) generate kustomize manifests -q cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG) $(KUSTOMIZE) build config/manifests | $(OPERATOR_SDK) generate bundle $(BUNDLE_GEN_FLAGS) $(OPERATOR_SDK) bundle validate ./bundle + $(MAKE) bundle-ignore-createdAt + +# Since operator-sdk 1.26.0, `make bundle` changes the `createdAt` field from the bundle +# even if it is patched: +# https://github.com/operator-framework/operator-sdk/pull/6136 +# This code checks if only the createdAt field. If is the only change, it is ignored. +# Else, it will do nothing. +# https://github.com/operator-framework/operator-sdk/issues/6285#issuecomment-1415350333 +# https://github.com/operator-framework/operator-sdk/issues/6285#issuecomment-1532150678 +.PHONY: bundle-ignore-createdAt +bundle-ignore-createdAt: + git diff --quiet -I'^ createdAt: ' ./bundle && git checkout ./bundle || true .PHONY: bundle-build bundle-build: ## Build the bundle image. diff --git a/internal/controller/dnsheathcheckprobe_controller_test.go b/internal/controller/dnsheathcheckprobe_controller_test.go new file mode 100644 index 00000000..6a5ac7fa --- /dev/null +++ b/internal/controller/dnsheathcheckprobe_controller_test.go @@ -0,0 +1,98 @@ +//go:build integration + +package controller + +import ( + "context" + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/kuadrant-dns-operator/api/v1alpha1" +) + +var _ = Describe("DNSHealthCheckProbe controller", func() { + const ( + ProbeName = "test-probe" + ProbeNamespace = "default" + + timeout = time.Second * 10 + duration = time.Second * 10 + interval = time.Millisecond * 250 + ) + + Context("When creating DNSHealthCheckProbe", func() { + It("Should update health status to healthy", func() { + By("Performing health check") + + ctx := context.Background() + probeObj := &v1alpha1.DNSHealthCheckProbe{ + ObjectMeta: metav1.ObjectMeta{ + Name: ProbeName, + Namespace: ProbeNamespace, + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + Host: "localhost", + Address: "0.0.0.0", + Port: 3333, + Interval: metav1.Duration{Duration: time.Second * 10}, + Path: "/healthy", + }, + } + + Expect(k8sClient.Create(ctx, probeObj)).Should(Succeed()) + + Eventually(func() error { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(probeObj), probeObj) + if err != nil { + return err + } + if probeObj.Status.LastCheckedAt.Time == (time.Time{}) { + return fmt.Errorf("expected probeObj.Status.LastCheckedAt to be non-zero %s, %s", probeObj.Status.LastCheckedAt.Time, (metav1.Time{}).Time) + } + return nil + }, timeout+(time.Second*20), interval).Should(BeNil()) + + GinkgoWriter.Print(probeObj) + + Expect(*probeObj.Status.Healthy).Should(BeTrue()) + Expect(probeObj.Status.LastCheckedAt).Should(Not(BeZero())) + }) + It("Should update health status to unhealthy", func() { + By("Updating to unhealthy endpoint") + + ctx := context.Background() + probeObj := &v1alpha1.DNSHealthCheckProbe{} + + err := k8sClient.Get(ctx, client.ObjectKey{ + Name: ProbeName, + Namespace: ProbeNamespace, + }, probeObj) + Expect(err).NotTo(HaveOccurred()) + + patch := client.MergeFrom(probeObj.DeepCopy()) + lastUpdate := probeObj.Status.LastCheckedAt + probeObj.Spec.Path = "/unhealthy" + Expect(k8sClient.Patch(ctx, probeObj, patch)).To(BeNil()) + + Eventually(func() error { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(probeObj), probeObj) + if err != nil { + return err + } + if !probeObj.Status.LastCheckedAt.Time.After(lastUpdate.Time) { + return fmt.Errorf("expected probeObj.Status.LastCheckedAt to be after lastUpdate") + } + return nil + }, timeout+(time.Second*20), interval).Should(BeNil()) + + Expect(*probeObj.Status.Healthy).Should(BeFalse()) + Expect(probeObj.Status.Reason).Should(Equal("Status code: 500")) + }) + }) +}) diff --git a/internal/controller/helper_test.go b/internal/controller/helper_test.go new file mode 100644 index 00000000..c9a2ca86 --- /dev/null +++ b/internal/controller/helper_test.go @@ -0,0 +1,48 @@ +//go:build integration + +package controller + +import ( + "context" + "fmt" + "net/http" + "time" +) + +const ( + TestTimeoutMedium = time.Second * 10 + TestTimeoutLong = time.Second * 30 + TestRetryIntervalMedium = time.Millisecond * 250 + defaultNS = "default" + providerCredential = "secretname" +) + +type testHealthServer struct { + Port int +} + +func (s *testHealthServer) Start(ctx context.Context) error { + mux := http.NewServeMux() + + endpoint := func(expectedCode int) func(http.ResponseWriter, *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(expectedCode) + } + } + + mux.HandleFunc("/healthy", endpoint(200)) + mux.HandleFunc("/unhealthy", endpoint(500)) + + errCh := make(chan error) + + go func() { + errCh <- http.ListenAndServe(fmt.Sprintf(":%d", s.Port), mux) + }() + + select { + case <-ctx.Done(): + return ctx.Err() + case err := <-errCh: + return err + } +} diff --git a/internal/controller/managedzone_controller_test.go b/internal/controller/managedzone_controller_test.go new file mode 100644 index 00000000..8cdb2cb0 --- /dev/null +++ b/internal/controller/managedzone_controller_test.go @@ -0,0 +1,104 @@ +//go:build integration + +/* +Copyright 2024. + +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 controller + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/kuadrant-dns-operator/api/v1alpha1" + //+kubebuilder:scaffold:imports +) + +var _ = Describe("ManagedZoneReconciler", func() { + Context("testing ManagedZone controller", func() { + var managedZone *v1alpha1.ManagedZone + var ctx context.Context + + BeforeEach(func() { + ctx = context.Background() + managedZone = &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example.com", + Namespace: defaultNS, + }, + Spec: v1alpha1.ManagedZoneSpec{ + ID: "example.com", + DomainName: "example.com", + SecretRef: v1alpha1.ProviderRef{ + Name: providerCredential, + }, + }, + } + }) + + AfterEach(func() { + // Clean up managedZones + mzList := &v1alpha1.ManagedZoneList{} + err := k8sClient.List(ctx, mzList, client.InNamespace(defaultNS)) + Expect(err).NotTo(HaveOccurred()) + for _, mz := range mzList.Items { + err = k8sClient.Delete(ctx, &mz) + Expect(client.IgnoreNotFound(err)).NotTo(HaveOccurred()) + } + }) + + It("should accept a managed zone for this controller and allow deletion", func() { + Expect(k8sClient.Create(ctx, managedZone)).To(BeNil()) + + createdMZ := &v1alpha1.ManagedZone{} + + Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Namespace: managedZone.Namespace, Name: managedZone.Name}, createdMZ) + }, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred()) + + Expect(k8sClient.Delete(ctx, managedZone)).To(BeNil()) + + Eventually(func() error { + err := k8sClient.Get(ctx, client.ObjectKey{Namespace: managedZone.Namespace, Name: managedZone.Name}, createdMZ) + if err != nil && !errors.IsNotFound(err) { + return err + } + return nil + }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) + }) + + It("should reject a managed zone with an invalid domain name", func() { + invalidDomainNameManagedZone := &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid_domain", + Namespace: defaultNS, + }, + Spec: v1alpha1.ManagedZoneSpec{ + ID: "invalid_domain", + DomainName: "invalid_domain", + }, + } + err := k8sClient.Create(ctx, invalidDomainNameManagedZone) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.domainName in body should match")) + }) + }) +}) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index c6f1cc33..011540a8 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -1,3 +1,5 @@ +//go:build integration + /* Copyright 2024. @@ -17,20 +19,29 @@ limitations under the License. package controller import ( + "context" "path/filepath" "testing" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" kuadrantiov1alpha1 "github.com/kuadrant/kuadrant-dns-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-dns-operator/internal/health" + "github.com/kuadrant/kuadrant-dns-operator/internal/provider" + _ "github.com/kuadrant/kuadrant-dns-operator/internal/provider/aws" + providerFake "github.com/kuadrant/kuadrant-dns-operator/internal/provider/fake" + _ "github.com/kuadrant/kuadrant-dns-operator/internal/provider/google" //+kubebuilder:scaffold:imports ) @@ -40,6 +51,26 @@ import ( var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment +var ctx context.Context +var cancel context.CancelFunc +var dnsProviderFactory = &providerFake.Factory{ + ProviderForFunc: func(ctx context.Context, pa kuadrantiov1alpha1.ProviderAccessor) (provider.Provider, error) { + return &providerFake.Provider{ + EnsureFunc: func(record *kuadrantiov1alpha1.DNSRecord, zone *kuadrantiov1alpha1.ManagedZone) error { + return nil + }, + DeleteFunc: func(record *kuadrantiov1alpha1.DNSRecord, zone *kuadrantiov1alpha1.ManagedZone) error { + return nil + }, + EnsureManagedZoneFunc: func(zone *kuadrantiov1alpha1.ManagedZone) (provider.ManagedZoneOutput, error) { + return provider.ManagedZoneOutput{}, nil + }, + DeleteManagedZoneFunc: func(zone *kuadrantiov1alpha1.ManagedZone) error { + return nil + }, + }, nil + }, +} func TestControllers(t *testing.T) { RegisterFailHandler(Fail) @@ -50,6 +81,7 @@ func TestControllers(t *testing.T) { var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + ctx, cancel = context.WithCancel(ctrl.SetupSignalHandler()) By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, @@ -71,10 +103,59 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + HealthProbeBindAddress: "0", + Metrics: metricsserver.Options{BindAddress: "0"}, + }) + Expect(err).ToNot(HaveOccurred()) + + healthQueue := health.NewRequestQueue(1 * time.Second) + err = mgr.Add(healthQueue) + Expect(err).ToNot(HaveOccurred()) + + monitor := health.NewMonitor() + err = mgr.Add(monitor) + Expect(err).ToNot(HaveOccurred()) + + healthServer := &testHealthServer{ + Port: 3333, + } + err = mgr.Add(healthServer) + Expect(err).ToNot(HaveOccurred()) + + err = (&ManagedZoneReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ProviderFactory: dnsProviderFactory, + }).SetupWithManager(mgr) + Expect(err).ToNot(HaveOccurred()) + + err = (&DNSRecordReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ProviderFactory: dnsProviderFactory, + }).SetupWithManager(mgr) + Expect(err).ToNot(HaveOccurred()) + + err = (&DNSHealthCheckProbeReconciler{ + Client: mgr.GetClient(), + HealthMonitor: monitor, + Queue: healthQueue, + }).SetupWithManager(mgr) + Expect(err).ToNot(HaveOccurred()) + + go func() { + defer GinkgoRecover() + err = mgr.Start(ctx) + Expect(err).ToNot(HaveOccurred()) + }() + }) var _ = AfterSuite(func() { By("tearing down the test environment") + cancel() err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) }) diff --git a/internal/provider/google/google_test.go b/internal/provider/google/google_test.go index e7f449f8..3a4244fd 100644 --- a/internal/provider/google/google_test.go +++ b/internal/provider/google/google_test.go @@ -1,4 +1,4 @@ -// //go:build unit +//go:build unit package google diff --git a/make/act.mk b/make/act.mk new file mode 100644 index 00000000..80b784a8 --- /dev/null +++ b/make/act.mk @@ -0,0 +1,32 @@ + +##@ GitHub Actions + +## Targets to help test GitHub Actions locally using act https://github.com/nektos/act + +.PHONY: act-test-unit-tests +act-test-unit-tests: act ## Run unit tests job. + $(ACT) -j unit_test_suite + +.PHONY: act-test-integration-tests +act-test-integration-tests: act ## Run integration tests job. + $(ACT) -j integration_test_suite --privileged + +.PHONY: act-test-verify-manifests +act-test-verify-manifests: act ## Run verify manifests job. + $(ACT) -j verify-manifests + +.PHONY: act-test-verify-bundle +act-test-verify-bundle: act ## Run verify bundle job. + $(ACT) -j verify-bundle + +.PHONY: act-test-verify-code +act-test-verify-code: act ## Run verify code job. + $(ACT) -j verify-code + +.PHONY: act-test-lint +act-test-lint: act ## Run lint job. + $(ACT) -j lint + +.PHONY: act-test-verify-imports +act-test-verify-imports: act ## Run verify-imports job. + $(ACT) -j verify-imports