From d74d5106d0c500c90040ccc8e73132a525bb7b51 Mon Sep 17 00:00:00 2001 From: Tim Usner Date: Thu, 25 Feb 2021 18:30:51 +0100 Subject: [PATCH] Fix golangci-lint findings --- Makefile | 2 +- .../crd/bases/druid.gardener.cloud_etcds.yaml | 1 + controllers/etcd_controller_test.go | 112 +++++++++--------- controllers/suite_test.go | 8 +- main.go | 5 +- pkg/predicate/predicate.go | 6 - .../{OpenAPIv2 => openapiv2}/OpenAPIv2.go | 0 .../{OpenAPIv2 => openapiv2}/OpenAPIv2.pb.go | 0 .../{OpenAPIv2 => openapiv2}/OpenAPIv2.proto | 0 .../{OpenAPIv2 => openapiv2}/README.md | 0 .../{OpenAPIv2 => openapiv2}/document.go | 0 .../{OpenAPIv2 => openapiv2}/openapi-2.0.json | 0 12 files changed, 68 insertions(+), 66 deletions(-) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/OpenAPIv2.go (100%) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/OpenAPIv2.pb.go (100%) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/OpenAPIv2.proto (100%) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/README.md (100%) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/document.go (100%) rename vendor/github.com/googleapis/gnostic/{OpenAPIv2 => openapiv2}/openapi-2.0.json (100%) diff --git a/Makefile b/Makefile index 3cfee974c..5c1ac6780 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ fmt: # Check packages .PHONY: check check: - @$(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/check.sh --golangci-lint-config=./.golangci.yaml . ./api/... ./pkg/... ./controllers/... + @$(REPO_ROOT)/vendor/github.com/gardener/gardener/hack/check.sh --golangci-lint-config=./.golangci.yaml ./api/... ./pkg/... ./controllers/... # Generate code .PHONY: generate diff --git a/config/crd/bases/druid.gardener.cloud_etcds.yaml b/config/crd/bases/druid.gardener.cloud_etcds.yaml index 24214ca29..e47acad94 100644 --- a/config/crd/bases/druid.gardener.cloud_etcds.yaml +++ b/config/crd/bases/druid.gardener.cloud_etcds.yaml @@ -1,3 +1,4 @@ + --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition diff --git a/controllers/etcd_controller_test.go b/controllers/etcd_controller_test.go index 86a09f963..8f8ba44d3 100644 --- a/controllers/etcd_controller_test.go +++ b/controllers/etcd_controller_test.go @@ -20,6 +20,8 @@ import ( "os" "time" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/gardener/etcd-druid/pkg/common" "github.com/gardener/gardener/pkg/utils/imagevector" "github.com/ghodss/yaml" @@ -50,23 +52,6 @@ const ( WithOwnerAnnotation StatefulSetInitializer = 2 ) -const ( - aws = "aws" - azure = "azure" - gcp = "gcp" - alicloud = "alicloud" - openstack = "openstack" -) - -const ( - s3 = "S3" - abs = "ABS" - gcs = "GCS" - oss = "OSS" - swift = "Swift" - local = "Local" -) - const ( timeout = time.Minute * 2 pollingInterval = time.Second * 2 @@ -168,15 +153,16 @@ var _ = Describe("Druid", func() { Name: instance.Namespace, }, } - c.Create(context.TODO(), &ns) + _, err = controllerutil.CreateOrUpdate(context.TODO(), c, &ns, func() error { return nil }) + Expect(err).To(Not(HaveOccurred())) + storeSecret := instance.Spec.Backup.Store.SecretRef.Name errors := createSecrets(c, instance.Namespace, storeSecret) Expect(len(errors)).Should(BeZero()) }) It("should create and adopt statefulset", func() { defer WithWd("..")() - err = c.Create(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(context.TODO(), instance)).To(Succeed()) ss := appsv1.StatefulSet{} Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, &ss) }, timeout, pollingInterval).Should(BeNil()) setStatefulSetReady(&ss) @@ -184,7 +170,7 @@ var _ = Describe("Druid", func() { Expect(err).NotTo(HaveOccurred()) }) AfterEach(func() { - c.Delete(context.TODO(), instance) + Expect(c.Delete(context.TODO(), instance)).To(Succeed()) }) }) @@ -193,6 +179,21 @@ var _ = Describe("Druid", func() { var ss *appsv1.StatefulSet instance := getEtcd(name, "default", false) c := mgr.GetClient() + Expect(c.Create(context.TODO(), instance)).To(Succeed()) + Eventually(func() error { + if err := c.Get(context.TODO(), client.ObjectKeyFromObject(instance), instance); err != nil { + return err + } + if instance.UID != "" { + instance.TypeMeta = metav1.TypeMeta{ + APIVersion: "druid.gardener.cloud/v1alpha1", + Kind: "etcd", + } + return nil + } + return fmt.Errorf("etcd object not yet created") + }, timeout, pollingInterval).Should(BeNil()) + switch setupStatefulSet { case WithoutOwner: ss = createStatefulset(name, "default", instance.Spec.Labels) @@ -209,17 +210,13 @@ var _ = Describe("Druid", func() { storeSecret := instance.Spec.Backup.Store.SecretRef.Name errors := createSecrets(c, instance.Namespace, storeSecret) Expect(len(errors)).Should(BeZero()) - c.Create(context.TODO(), ss) + Expect(c.Create(context.TODO(), ss)).To(Succeed()) defer WithWd("..")() - err := c.Create(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) s := &appsv1.StatefulSet{} Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, s) }, timeout, pollingInterval).Should(BeNil()) setStatefulSetReady(s) - err = c.Status().Update(context.TODO(), s) - Expect(err).NotTo(HaveOccurred()) - err = c.Delete(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Status().Update(context.TODO(), s)).To(Succeed()) + Expect(c.Delete(context.TODO(), instance)).To(Succeed()) }, Entry("when statefulset not owned by etcd, druid should adopt the statefulset", "foo20", WithoutOwner), Entry("when statefulset owned by etcd with owner reference set, druid should remove ownerref and add annotations", "foo21", WithOwnerReference), @@ -240,10 +237,8 @@ var _ = Describe("Druid", func() { c = mgr.GetClient() p = createPod(fmt.Sprintf("%s-0", instance.Name), "default", instance.Spec.Labels) ss = createStatefulset(instance.Name, instance.Namespace, instance.Spec.Labels) - err = c.Create(context.TODO(), p) - Expect(err).NotTo(HaveOccurred()) - err = c.Create(context.TODO(), ss) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(context.TODO(), p)).To(Succeed()) + Expect(c.Create(context.TODO(), ss)).To(Succeed()) p.Status.ContainerStatuses = []corev1.ContainerStatus{ { Name: "Container-0", @@ -264,17 +259,15 @@ var _ = Describe("Druid", func() { }) It("should restart pod", func() { defer WithWd("..")() - err = c.Create(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Create(context.TODO(), instance)).To(Succeed()) Eventually(func() error { return podDeleted(c, instance) }, timeout, pollingInterval).Should(BeNil()) }) AfterEach(func() { s := &appsv1.StatefulSet{} Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, s) }, timeout, pollingInterval).Should(BeNil()) setStatefulSetReady(s) - err = c.Status().Update(context.TODO(), s) - Expect(err).NotTo(HaveOccurred()) - c.Delete(context.TODO(), instance) + Expect(c.Status().Update(context.TODO(), s)).To(Succeed()) + Expect(c.Delete(context.TODO(), instance)).To(Succeed()) }) }) }) @@ -285,6 +278,21 @@ var _ = Describe("Druid", func() { var sts appsv1.StatefulSet instance := getEtcd(name, "default", false) c := mgr.GetClient() + Expect(c.Create(context.TODO(), instance)).To(Succeed()) + Eventually(func() error { + if err := c.Get(context.TODO(), client.ObjectKeyFromObject(instance), instance); err != nil { + return err + } + if instance.UID != "" { + instance.TypeMeta = metav1.TypeMeta{ + APIVersion: "druid.gardener.cloud/v1alpha1", + Kind: "etcd", + } + return nil + } + return fmt.Errorf("etcd object not yet created") + }, timeout, pollingInterval).Should(BeNil()) + switch setupStatefulSet { case WithoutOwner: ss = createStatefulset(name, "default", instance.Spec.Labels) @@ -301,17 +309,15 @@ var _ = Describe("Druid", func() { storeSecret := instance.Spec.Backup.Store.SecretRef.Name errors := createSecrets(c, instance.Namespace, storeSecret) Expect(len(errors)).Should(BeZero()) - c.Create(context.TODO(), ss) + Expect(c.Create(context.TODO(), ss)).To(Succeed()) defer WithWd("..")() - err := c.Create(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) Eventually(func() error { return statefulsetIsCorrectlyReconciled(c, instance, &sts) }, timeout, pollingInterval).Should(BeNil()) // This go-routine is to set that statefulset is ready manually as statefulset controller is absent for tests. go func() { for { select { case <-time.After(time.Second * 2): - setAndCheckStatefulSetReady(c, instance) + Expect(setAndCheckStatefulSetReady(c, instance)).To(Succeed()) case <-stopCh: return } @@ -319,8 +325,7 @@ var _ = Describe("Druid", func() { }() Eventually(func() error { return isEtcdReady(c, instance) }, timeout, pollingInterval).Should(BeNil()) close(stopCh) - err = c.Delete(context.TODO(), instance) - Expect(err).NotTo(HaveOccurred()) + Expect(c.Delete(context.TODO(), instance)).To(Succeed()) Eventually(func() error { return statefulSetRemoved(c, &sts) }, timeout, pollingInterval).Should(BeNil()) Eventually(func() error { return etcdRemoved(c, instance) }, timeout, pollingInterval).Should(BeNil()) }, @@ -346,7 +351,10 @@ var _ = Describe("Druid", func() { Name: instance.Namespace, }, } - c.Create(context.TODO(), &ns) + + _, err = controllerutil.CreateOrUpdate(context.TODO(), c, &ns, func() error { return nil }) + Expect(err).To(Not(HaveOccurred())) + if instance.Spec.Backup.Store != nil && instance.Spec.Backup.Store.SecretRef != nil { storeSecret := instance.Spec.Backup.Store.SecretRef.Name errors := createSecrets(c, instance.Namespace, storeSecret) @@ -408,7 +416,7 @@ func validateEtcdWithDefaults(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc * // Validate Metrics MetricsLevel Expect(instance.Spec.Etcd.Metrics).To(BeNil()) - Expect(config).To(HaveKeyWithValue(metricsKey, fmt.Sprintf("%s", druidv1alpha1.Basic))) + Expect(config).To(HaveKeyWithValue(metricsKey, string(druidv1alpha1.Basic))) // Validate DefragmentationSchedule *string Expect(instance.Spec.Etcd.DefragmentationSchedule).To(BeNil()) @@ -754,7 +762,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi // Validate Metrics MetricsLevel Expect(instance.Spec.Etcd.Metrics).NotTo(BeNil()) - Expect(config).To(HaveKeyWithValue(metricsKey, fmt.Sprintf("%s", *instance.Spec.Etcd.Metrics))) + Expect(config).To(HaveKeyWithValue(metricsKey, string(*instance.Spec.Etcd.Metrics))) // Validate DefragmentationSchedule *string Expect(instance.Spec.Etcd.DefragmentationSchedule).NotTo(BeNil()) @@ -795,7 +803,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi Expect(config).To(MatchKeys(IgnoreExtras, Keys{ "name": Equal(fmt.Sprintf("etcd-%s", instance.UID[:6])), "data-dir": Equal("/var/etcd/data/new.etcd"), - "metrics": Equal(fmt.Sprintf("%s", *instance.Spec.Etcd.Metrics)), + "metrics": Equal(string(*instance.Spec.Etcd.Metrics)), "snapshot-count": Equal(float64(75000)), "enable-v2": Equal(false), "quota-backend-bytes": Equal(float64(instance.Spec.Etcd.Quota.Value())), @@ -1031,7 +1039,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi fmt.Sprintf("--defragmentation-schedule=%s", *instance.Spec.Etcd.DefragmentationSchedule): Equal(fmt.Sprintf("--defragmentation-schedule=%s", *instance.Spec.Etcd.DefragmentationSchedule)), fmt.Sprintf("--schedule=%s", *instance.Spec.Backup.FullSnapshotSchedule): Equal(fmt.Sprintf("--schedule=%s", *instance.Spec.Backup.FullSnapshotSchedule)), fmt.Sprintf("%s=%s", "--garbage-collection-policy", *instance.Spec.Backup.GarbageCollectionPolicy): Equal(fmt.Sprintf("%s=%s", "--garbage-collection-policy", *instance.Spec.Backup.GarbageCollectionPolicy)), - fmt.Sprintf("%s=%s", "--storage-provider", store): Equal(fmt.Sprintf("%s=%s", "--storage-provider", fmt.Sprintf("%s", store))), + fmt.Sprintf("%s=%s", "--storage-provider", store): Equal(fmt.Sprintf("%s=%s", "--storage-provider", store)), fmt.Sprintf("%s=%s", "--store-prefix", instance.Spec.Backup.Store.Prefix): Equal(fmt.Sprintf("%s=%s", "--store-prefix", instance.Spec.Backup.Store.Prefix)), fmt.Sprintf("--delta-snapshot-memory-limit=%d", instance.Spec.Backup.DeltaSnapshotMemoryLimit.Value()): Equal(fmt.Sprintf("--delta-snapshot-memory-limit=%d", instance.Spec.Backup.DeltaSnapshotMemoryLimit.Value())), fmt.Sprintf("--garbage-collection-policy=%s", *instance.Spec.Backup.GarbageCollectionPolicy): Equal(fmt.Sprintf("--garbage-collection-policy=%s", *instance.Spec.Backup.GarbageCollectionPolicy)), @@ -1048,7 +1056,7 @@ func validateEtcd(s *appsv1.StatefulSet, cm *corev1.ConfigMap, svc *corev1.Servi ContainerPort: backupPort, }, }), - "Image": Equal(fmt.Sprintf("%s", *instance.Spec.Backup.Image)), + "Image": Equal(*instance.Spec.Backup.Image), "ImagePullPolicy": Equal(corev1.PullIfNotPresent), "VolumeMounts": MatchElements(volumeMountIterator, IgnoreExtras, Elements{ *instance.Spec.VolumeClaimTemplate: MatchFields(IgnoreExtras, Fields{ @@ -1542,7 +1550,7 @@ func statefulsetIsCorrectlyReconciled(c client.Client, instance *druidv1alpha1.E ctx, cancel := context.WithTimeout(context.TODO(), timeout) defer cancel() req := types.NamespacedName{ - Name: fmt.Sprintf("%s", instance.Name), + Name: instance.Name, Namespace: instance.Namespace, } @@ -1756,10 +1764,6 @@ func getEtcdWithTLS(name, namespace string) *druidv1alpha1.Etcd { return getEtcd(name, namespace, true) } -func getEtcdWithoutTLS(name, namespace string) *druidv1alpha1.Etcd { - return getEtcd(name, namespace, false) -} - func getEtcdWithDefault(name, namespace string) *druidv1alpha1.Etcd { instance := &druidv1alpha1.Etcd{ ObjectMeta: metav1.ObjectMeta{ diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 05150c62c..3f58aba2f 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "sync" "testing" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -50,8 +51,6 @@ var ( k8sClient client.Client testEnv *envtest.Environment mgr manager.Manager - recFn reconcile.Reconciler - requests chan reconcile.Request mgrStopped *sync.WaitGroup testLog = ctrl.Log.WithName("test") @@ -119,11 +118,14 @@ var _ = AfterSuite(func() { func startTestManager(ctx context.Context, mgr manager.Manager) *sync.WaitGroup { wg := &sync.WaitGroup{} + wg.Add(1) go func() { - wg.Add(1) Expect(mgr.Start(ctx)).NotTo(HaveOccurred()) wg.Done() }() + syncCtx, syncCancel := context.WithTimeout(ctx, 1*time.Minute) + defer syncCancel() + mgr.GetCache().WaitForCacheSync(syncCtx) return wg } diff --git a/main.go b/main.go index b56d2808a..bbfefb717 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,7 @@ import ( "github.com/gardener/etcd-druid/controllers" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" schemev1 "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" ctrl "sigs.k8s.io/controller-runtime" @@ -36,8 +37,8 @@ var ( ) func init() { - schemev1.AddToScheme(scheme) - druidv1alpha1.AddToScheme(scheme) + utilruntime.Must(schemev1.AddToScheme(scheme)) + utilruntime.Must(druidv1alpha1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/pkg/predicate/predicate.go b/pkg/predicate/predicate.go index 52ccee729..2093e90fb 100644 --- a/pkg/predicate/predicate.go +++ b/pkg/predicate/predicate.go @@ -16,7 +16,6 @@ package predicate import ( druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/event" @@ -26,11 +25,6 @@ import ( v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" ) -// Log is the logger for predicates. -var ( - logger = logrus.New() -) - type or struct { predicates []predicate.Predicate } diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.go b/vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.go similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.go rename to vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.go diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.pb.go b/vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.pb.go similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.pb.go rename to vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.pb.go diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.proto b/vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.proto similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/OpenAPIv2.proto rename to vendor/github.com/googleapis/gnostic/openapiv2/OpenAPIv2.proto diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/README.md b/vendor/github.com/googleapis/gnostic/openapiv2/README.md similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/README.md rename to vendor/github.com/googleapis/gnostic/openapiv2/README.md diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/document.go b/vendor/github.com/googleapis/gnostic/openapiv2/document.go similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/document.go rename to vendor/github.com/googleapis/gnostic/openapiv2/document.go diff --git a/vendor/github.com/googleapis/gnostic/OpenAPIv2/openapi-2.0.json b/vendor/github.com/googleapis/gnostic/openapiv2/openapi-2.0.json similarity index 100% rename from vendor/github.com/googleapis/gnostic/OpenAPIv2/openapi-2.0.json rename to vendor/github.com/googleapis/gnostic/openapiv2/openapi-2.0.json