From 85f89072aa5d630397e54cbb19c47080c54484c5 Mon Sep 17 00:00:00 2001 From: Adam Kaplan Date: Wed, 1 Nov 2023 17:14:27 -0400 Subject: [PATCH] Updates from breaking dependency changes - Utilize ginkgo/v2 SpecContext to handle graceful termination. A separate context was created to isolate EnvTest from any "client" contexts used during tests and avoid deadlocks. - Use v1.26 for EnvTest-driven tests. - Re-generate manifests and bundle --- Makefile | 2 +- ...erator.shipwright.io_shipwrightbuilds.yaml | 4 +-- ...erator.shipwright.io_shipwrightbuilds.yaml | 4 +-- controllers/default_test.go | 27 ++++++++++--------- controllers/suite_test.go | 21 +++++++-------- pkg/common/util_test.go | 3 ++- 6 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index ecf22560..feee2e4f 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ endif # Image URL to use all building/pushing image targets IMG ?= $(IMAGE_TAG_BASE):$(TAG) # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. -ENVTEST_K8S_VERSION = 1.23 +ENVTEST_K8S_VERSION = 1.26 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set) ifeq (,$(shell go env GOBIN)) diff --git a/bundle/manifests/operator.shipwright.io_shipwrightbuilds.yaml b/bundle/manifests/operator.shipwright.io_shipwrightbuilds.yaml index 0e9bb7ff..1bb04b7d 100644 --- a/bundle/manifests/operator.shipwright.io_shipwrightbuilds.yaml +++ b/bundle/manifests/operator.shipwright.io_shipwrightbuilds.yaml @@ -51,8 +51,8 @@ spec: description: "Condition contains details for one aspect of the current state of this API Resource. --- This struct is intended for direct use as an array at the field path .status.conditions. For example, - type FooStatus struct{ // Represents the observations of a foo's - current state. // Known .status.conditions.type are: \"Available\", + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge // +listType=map // +listMapKey=type Conditions []metav1.Condition `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" diff --git a/config/crd/bases/operator.shipwright.io_shipwrightbuilds.yaml b/config/crd/bases/operator.shipwright.io_shipwrightbuilds.yaml index 55fbddff..e2fc4e34 100644 --- a/config/crd/bases/operator.shipwright.io_shipwrightbuilds.yaml +++ b/config/crd/bases/operator.shipwright.io_shipwrightbuilds.yaml @@ -52,8 +52,8 @@ spec: description: "Condition contains details for one aspect of the current state of this API Resource. --- This struct is intended for direct use as an array at the field path .status.conditions. For example, - type FooStatus struct{ // Represents the observations of a foo's - current state. // Known .status.conditions.type are: \"Available\", + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge // +listType=map // +listMapKey=type Conditions []metav1.Condition `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" diff --git a/controllers/default_test.go b/controllers/default_test.go index caaa8275..bc09594b 100644 --- a/controllers/default_test.go +++ b/controllers/default_test.go @@ -1,9 +1,11 @@ package controllers import ( - g "github.com/onsi/ginkgo" + "context" + + g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" - "github.com/shipwright-io/operator/pkg/common" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -14,11 +16,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/shipwright-io/operator/api/v1alpha1" + "github.com/shipwright-io/operator/pkg/common" "github.com/shipwright-io/operator/test" ) // createNamespace creates the namespace informed. -func createNamespace(name string) { +func createNamespace(ctx context.Context, name string) { ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}} err := k8sClient.Get(ctx, types.NamespacedName{Name: ns.Name}, ns) if errors.IsNotFound(err) { @@ -60,9 +63,9 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { } truePtr := true - g.BeforeEach(func() { + g.BeforeEach(func(ctx g.SpecContext) { // setting up the namespaces, where Shipwright Controller will be deployed - createNamespace(namespace) + createNamespace(ctx, namespace) g.By("does tekton taskrun crd exist") err := k8sClient.Get(ctx, types.NamespacedName{Name: "taskruns.tekton.dev"}, &crdv1.CustomResourceDefinition{}) @@ -144,7 +147,7 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { test.EventuallyContainFinalizer(ctx, k8sClient, build, FinalizerAnnotation) }) - g.AfterEach(func() { + g.AfterEach(func(ctx g.SpecContext) { g.By("deleting the ShipwrightBuild instance") namespacedName := types.NamespacedName{Namespace: namespace, Name: build.Name} err := k8sClient.Get(ctx, namespacedName, build) @@ -170,7 +173,7 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { g.When("a ShipwrightBuild object is created", func() { - g.It("creates RBAC for the Shipwright build controller", func() { + g.It("creates RBAC for the Shipwright build controller", func(ctx g.SpecContext) { expectedClusterRole := baseClusterRole.DeepCopy() test.EventuallyExists(ctx, k8sClient, expectedClusterRole) @@ -181,12 +184,12 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { test.EventuallyExists(ctx, k8sClient, expectedServiceAccount) }) - g.It("creates a deployment for the Shipwright build controller", func() { + g.It("creates a deployment for the Shipwright build controller", func(ctx g.SpecContext) { expectedDeployment := baseDeployment.DeepCopy() test.EventuallyExists(ctx, k8sClient, expectedDeployment) }) - g.It("creates custom resource definitions for the Shipwright build APIs", func() { + g.It("creates custom resource definitions for the Shipwright build APIs", func(ctx g.SpecContext) { test.CRDEventuallyExists(ctx, k8sClient, "builds.shipwright.io") test.CRDEventuallyExists(ctx, k8sClient, "buildruns.shipwright.io") test.CRDEventuallyExists(ctx, k8sClient, "buildstrategies.shipwright.io") @@ -196,7 +199,7 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { g.When("a ShipwrightBuild object is deleted", func() { - g.It("deletes the RBAC for the Shipwright build controller", func() { + g.It("deletes the RBAC for the Shipwright build controller", func(ctx g.SpecContext) { expectedClusterRole := baseClusterRole.DeepCopy() expectedClusterRoleBinding := baseClusterRoleBinding.DeepCopy() expectedServiceAccount := baseServiceAccount.DeepCopy() @@ -216,7 +219,7 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { test.EventuallyRemoved(ctx, k8sClient, expectedServiceAccount) }) - g.It("deletes the deployment for the Shipwright build controller", func() { + g.It("deletes the deployment for the Shipwright build controller", func(ctx g.SpecContext) { expectedDeployment := baseDeployment.DeepCopy() // Setup - ensure the objects we want exist test.EventuallyExists(ctx, k8sClient, expectedDeployment) @@ -230,7 +233,7 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() { }) // TODO: Do not delete the CRDs! This is something only a cluster admin should do. - g.It("deletes the custom resource definitions for the Shipwright build APIs", func() { + g.It("deletes the custom resource definitions for the Shipwright build APIs", func(ctx g.SpecContext) { test.CRDEventuallyExists(ctx, k8sClient, "builds.shipwright.io") test.CRDEventuallyExists(ctx, k8sClient, "buildruns.shipwright.io") test.CRDEventuallyExists(ctx, k8sClient, "buildstrategies.shipwright.io") diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 0d5bbbee..c894b2c6 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -10,21 +10,20 @@ import ( "testing" "time" - tektonoperatorv1alpha1client "github.com/tektoncd/operator/pkg/client/clientset/versioned/typed/operator/v1alpha1" - crdclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" - - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + crdclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/envtest/printer" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + tektonoperatorv1alpha1client "github.com/tektoncd/operator/pkg/client/clientset/versioned/typed/operator/v1alpha1" + operatorv1alpha1 "github.com/shipwright-io/operator/api/v1alpha1" // +kubebuilder:scaffold:imports ) @@ -45,16 +44,12 @@ func TestAPIs(t *testing.T) { RegisterFailHandler(Fail) SetDefaultEventuallyTimeout(restTimeout) SetDefaultEventuallyPollingInterval(restRetry) - RunSpecsWithDefaultAndCustomReporters(t, - "Controller Suite", - []Reporter{printer.NewlineReporter{}}) + RunSpecs(t, "Controller Suite") } var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - ctx, cancel = context.WithCancel(context.TODO()) - By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, @@ -91,6 +86,9 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) go func() { + // The controller needs its own context to run in a separate goroutine + // This needs to be lifecycled independently of the context that ginkgo/v2 passes in + ctx, cancel = context.WithCancel(context.Background()) err := mgr.Start(ctx) Expect(err).NotTo(HaveOccurred()) }() @@ -98,8 +96,7 @@ var _ = BeforeSuite(func() { k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) - -}, 60) +}) var _ = AfterSuite(func() { cancel() diff --git a/pkg/common/util_test.go b/pkg/common/util_test.go index 4f2876cd..e1cb515f 100644 --- a/pkg/common/util_test.go +++ b/pkg/common/util_test.go @@ -5,8 +5,9 @@ import ( "testing" mf "github.com/manifestival/manifestival" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime"