From 097bfb0f7f539c846cfc7a6ab12f3f95f0f9c6fd Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 15 Apr 2024 15:17:33 +0200 Subject: [PATCH] Ensure golangci-lint runs on all files This needed the following adjustments: * removed unused functions and function parameters * rename variables to follow style * ignore revive dot-imports rule as ginkgo and gomega --- .pre-commit-config.yaml | 2 +- controllers/placementapi_controller.go | 59 ++----------------- pkg/placement/deployment.go | 5 -- tests/functional/base_test.go | 2 +- .../placementapi_controller_test.go | 16 ++--- tests/functional/placementapi_webhook_test.go | 24 ++++---- tests/functional/suite_test.go | 4 +- 7 files changed, 31 insertions(+), 81 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 90dcba4b..e143809c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -28,7 +28,7 @@ repos: - repo: https://github.com/golangci/golangci-lint rev: v1.55.2 hooks: - - id: golangci-lint + - id: golangci-lint-full args: ["--verbose"] - repo: https://github.com/pre-commit/pre-commit-hooks diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index a929c8d3..5890be8c 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -21,7 +21,6 @@ import ( "fmt" "time" - apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -146,52 +145,6 @@ func (r *PlacementAPIReconciler) GetLogger(ctx context.Context) logr.Logger { return log.FromContext(ctx).WithName("Controllers").WithName("PlacementAPI") } -func (r *PlacementAPIReconciler) GetSecretMapperFor(crs client.ObjectList, ctx context.Context) func(client.Object) []reconcile.Request { - Log := r.GetLogger(ctx) - mapper := func(secret client.Object) []reconcile.Request { - var namespace string = secret.GetNamespace() - var secretName string = secret.GetName() - result := []reconcile.Request{} - - listOpts := []client.ListOption{ - client.InNamespace(namespace), - } - if err := r.Client.List(ctx, crs, listOpts...); err != nil { - Log.Error(err, "Unable to retrieve the list of CRs") - panic(err) - } - - err := apimeta.EachListItem(crs, func(o runtime.Object) error { - // NOTE(gibi): intentionally let the failed cast panic to catch - // this implementation error as soon as possible. - cr := o.(GetSecret) - if cr.GetSecret() == secretName { - name := client.ObjectKey{ - Namespace: namespace, - Name: cr.GetName(), - } - Log.Info( - "Requesting reconcile due to secret change", - "Secret", secretName, "CR", name.Name, - ) - result = append(result, reconcile.Request{NamespacedName: name}) - } - return nil - }) - if err != nil { - Log.Error(err, "Unable to iterate the list of CRs") - panic(err) - } - - if len(result) > 0 { - return result - } - return nil - } - - return mapper -} - // PlacementAPIReconciler reconciles a PlacementAPI object type PlacementAPIReconciler struct { client.Client @@ -261,7 +214,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // when a condition's state doesn't change. savedConditions := instance.Status.Conditions.DeepCopy() // initialize status fields - if err = r.initStatus(ctx, h, instance); err != nil { + if err = r.initStatus(instance); err != nil { return ctrl.Result{}, err } @@ -735,9 +688,9 @@ func (r *PlacementAPIReconciler) ensureKeystoneEndpoint( } func (r *PlacementAPIReconciler) initStatus( - ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, + instance *placementv1.PlacementAPI, ) error { - if err := r.initConditions(ctx, h, instance); err != nil { + if err := r.initConditions(instance); err != nil { return err } @@ -754,7 +707,7 @@ func (r *PlacementAPIReconciler) initStatus( } func (r *PlacementAPIReconciler) initConditions( - ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, + instance *placementv1.PlacementAPI, ) error { if instance.Status.Conditions == nil { instance.Status.Conditions = condition.Conditions{} @@ -930,7 +883,7 @@ func (r *PlacementAPIReconciler) findObjectsForSrc(ctx context.Context, src clie FieldSelector: fields.OneTermEqualSelector(field, src.GetName()), Namespace: src.GetNamespace(), } - err := r.List(context.TODO(), crList, listOps) + err := r.List(ctx, crList, listOps) if err != nil { return []reconcile.Request{} } @@ -1126,7 +1079,7 @@ func (r *PlacementAPIReconciler) ensureDeployment( serviceLabels := getServiceLabels(instance) // Define a new Deployment object - deplDef, err := placement.Deployment(ctx, h, instance, inputHash, serviceLabels, serviceAnnotations) + deplDef, err := placement.Deployment(instance, inputHash, serviceLabels, serviceAnnotations) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.DeploymentReadyCondition, diff --git a/pkg/placement/deployment.go b/pkg/placement/deployment.go index fd0b3e11..da7e60e8 100644 --- a/pkg/placement/deployment.go +++ b/pkg/placement/deployment.go @@ -16,12 +16,9 @@ limitations under the License. package placement import ( - "context" - common "github.com/openstack-k8s-operators/lib-common/modules/common" affinity "github.com/openstack-k8s-operators/lib-common/modules/common/affinity" env "github.com/openstack-k8s-operators/lib-common/modules/common/env" - "github.com/openstack-k8s-operators/lib-common/modules/common/helper" "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" @@ -36,8 +33,6 @@ import ( // Deployment func func Deployment( - ctx context.Context, - helper *helper.Helper, instance *placementv1.PlacementAPI, configHash string, labels map[string]string, diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index 73528538..8e718adc 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -17,7 +17,7 @@ limitations under the License. package functional_test import ( - . "github.com/onsi/gomega" + . "github.com/onsi/gomega" //revive:disable:dot-imports corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index e963ccd5..8bea9c47 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -20,8 +20,10 @@ import ( "fmt" "os" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports + . "github.com/onsi/gomega" //revive:disable:dot-imports + + //revive:disable-next-line:dot-imports . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" "github.com/openstack-k8s-operators/placement-operator/pkg/placement" @@ -485,7 +487,7 @@ var _ = Describe("PlacementAPI controller", func() { Expect(int(*deployment.Spec.Replicas)).To(Equal(1)) Expect(deployment.Spec.Selector.MatchLabels).To(Equal(map[string]string{"service": "placement", "owner": names.PlacementAPIName.Name})) Expect(deployment.Spec.Template.Spec.ServiceAccountName).To(Equal(names.ServiceAccountName.Name)) - Expect(len(deployment.Spec.Template.Spec.Containers)).To(Equal(2)) + Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(2)) th.SimulateDeploymentReplicaReady(names.DeploymentName) @@ -858,7 +860,7 @@ var _ = Describe("PlacementAPI controller", func() { // that exercise standard account create / update patterns that should be // common to all controllers that ensure MariaDBAccount CRs. - mariadb_suite := &mariadb_test.MariaDBTestHarness{ + mariadbSuite := &mariadb_test.MariaDBTestHarness{ PopulateHarness: func(harness *mariadb_test.MariaDBTestHarness) { harness.Setup( "Placement", @@ -916,9 +918,9 @@ var _ = Describe("PlacementAPI controller", func() { }, } - mariadb_suite.RunBasicSuite() + mariadbSuite.RunBasicSuite() - mariadb_suite.RunURLAssertSuite(func(accountName types.NamespacedName, username string, password string) { + mariadbSuite.RunURLAssertSuite(func(accountName types.NamespacedName, username string, password string) { Eventually(func(g Gomega) { cm := th.GetSecret(names.ConfigMapName) @@ -931,7 +933,7 @@ var _ = Describe("PlacementAPI controller", func() { }) - mariadb_suite.RunConfigHashSuite(func() string { + mariadbSuite.RunConfigHashSuite(func() string { deployment := th.GetDeployment(names.DeploymentName) return GetEnvVarValue(deployment.Spec.Template.Spec.Containers[0].Env, "CONFIG_HASH", "") }) diff --git a/tests/functional/placementapi_webhook_test.go b/tests/functional/placementapi_webhook_test.go index 5851be7d..333d0882 100644 --- a/tests/functional/placementapi_webhook_test.go +++ b/tests/functional/placementapi_webhook_test.go @@ -20,8 +20,8 @@ import ( "errors" "os" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports + . "github.com/onsi/gomega" //revive:disable:dot-imports k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -32,11 +32,11 @@ import ( var _ = Describe("PlacementAPI Webhook", func() { - var placementApiName types.NamespacedName + var placementAPIName types.NamespacedName BeforeEach(func() { - placementApiName = types.NamespacedName{ + placementAPIName = types.NamespacedName{ Name: "placement", Namespace: namespace, } @@ -47,11 +47,11 @@ var _ = Describe("PlacementAPI Webhook", func() { When("A PlacementAPI instance is created without container images", func() { BeforeEach(func() { - DeferCleanup(th.DeleteInstance, CreatePlacementAPI(placementApiName, GetDefaultPlacementAPISpec())) + DeferCleanup(th.DeleteInstance, CreatePlacementAPI(placementAPIName, GetDefaultPlacementAPISpec())) }) It("should have the defaults initialized by webhook", func() { - PlacementAPI := GetPlacementAPI(placementApiName) + PlacementAPI := GetPlacementAPI(placementAPIName) Expect(PlacementAPI.Spec.ContainerImage).Should(Equal( placementv1.PlacementAPIContainerImage, )) @@ -60,13 +60,13 @@ var _ = Describe("PlacementAPI Webhook", func() { When("A PlacementAPI instance is created with container images", func() { BeforeEach(func() { - placementApiSpec := GetDefaultPlacementAPISpec() - placementApiSpec["containerImage"] = "api-container-image" - DeferCleanup(th.DeleteInstance, CreatePlacementAPI(placementApiName, placementApiSpec)) + placementAPISpec := GetDefaultPlacementAPISpec() + placementAPISpec["containerImage"] = "api-container-image" + DeferCleanup(th.DeleteInstance, CreatePlacementAPI(placementAPIName, placementAPISpec)) }) It("should use the given values", func() { - PlacementAPI := GetPlacementAPI(placementApiName) + PlacementAPI := GetPlacementAPI(placementAPIName) Expect(PlacementAPI.Spec.ContainerImage).Should(Equal( "api-container-image", )) @@ -83,8 +83,8 @@ var _ = Describe("PlacementAPI Webhook", func() { "apiVersion": "placement.openstack.org/v1beta1", "kind": "PlacementAPI", "metadata": map[string]interface{}{ - "name": placementApiName.Name, - "namespace": placementApiName.Namespace, + "name": placementAPIName.Name, + "namespace": placementAPIName.Namespace, }, "spec": spec, } diff --git a/tests/functional/suite_test.go b/tests/functional/suite_test.go index 86f84497..f00bd50b 100644 --- a/tests/functional/suite_test.go +++ b/tests/functional/suite_test.go @@ -27,8 +27,8 @@ import ( "github.com/go-logr/logr" "github.com/google/uuid" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports + . "github.com/onsi/gomega" //revive:disable:dot-imports "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme"