From 51325fa7347d09be902179da97d9a9c6e44b9ec4 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 24 Apr 2024 16:28:55 +0200 Subject: [PATCH] Ensure golangci-lint runs on all files * removed unused func params * ignore revive dot-imports rule on ginkgo and gomega as dot import there is the recommended practice * added missing error handling * various small style fixes found by the linter --- .pre-commit-config.yaml | 2 +- .../client/openstackclient_controller.go | 4 +- .../core/openstackcontrolplane_controller.go | 2 +- .../core/openstackversion_controller.go | 5 +- pkg/openstack/ca.go | 17 +++++++ pkg/openstack/galera.go | 4 +- pkg/openstack/memcached.go | 4 +- pkg/openstack/nova.go | 4 +- pkg/openstack/ovn.go | 2 +- pkg/openstack/rabbitmq.go | 4 +- pkg/openstack/version.go | 2 +- pkg/openstackclient/funcs.go | 1 - tests/functional/base_test.go | 2 +- .../openstackclient_webhook_test.go | 4 +- .../openstackoperator_controller_test.go | 50 ++++++++----------- .../openstackversion_controller_test.go | 8 +-- tests/functional/suite_test.go | 4 +- 17 files changed, 65 insertions(+), 54 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 70ea5ae71..62b5c3198 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -58,5 +58,5 @@ repos: - repo: https://github.com/golangci/golangci-lint rev: v1.55.2 hooks: - - id: golangci-lint + - id: golangci-lint-full args: ["-v"] diff --git a/controllers/client/openstackclient_controller.go b/controllers/client/openstackclient_controller.go index 724cb5b4c..6b3a96f99 100644 --- a/controllers/client/openstackclient_controller.go +++ b/controllers/client/openstackclient_controller.go @@ -314,7 +314,7 @@ func (r *OpenStackClientReconciler) Reconcile(ctx context.Context, req ctrl.Requ op, err := controllerutil.CreateOrPatch(ctx, r.Client, osclient, func() error { isPodUpdate := !osclient.ObjectMeta.CreationTimestamp.IsZero() if !isPodUpdate { - osclient.Spec = openstackclient.ClientPodSpec(ctx, instance, helper, clientLabels, configVarsHash) + osclient.Spec = openstackclient.ClientPodSpec(ctx, instance, helper, configVarsHash) } else { hashupdate := false @@ -544,7 +544,7 @@ func (r *OpenStackClientReconciler) findObjectsForSrc(ctx context.Context, src c 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{} } diff --git a/controllers/core/openstackcontrolplane_controller.go b/controllers/core/openstackcontrolplane_controller.go index 441f86d54..ec4ba0f17 100644 --- a/controllers/core/openstackcontrolplane_controller.go +++ b/controllers/core/openstackcontrolplane_controller.go @@ -198,7 +198,7 @@ func (r *OpenStackControlPlaneReconciler) Reconcile(ctx context.Context, req ctr return ctrlResult, nil } - if instance.Status.DeployedVersion == nil || version.Spec.TargetVersion == *instance.Status.DeployedVersion { + if instance.Status.DeployedVersion == nil || version.Spec.TargetVersion == *instance.Status.DeployedVersion { //revive:disable:indent-error-flow // green field deployment or no minor update in progress ctrlResult, err := r.reconcileNormal(ctx, instance, version, helper) if err != nil { diff --git a/controllers/core/openstackversion_controller.go b/controllers/core/openstackversion_controller.go index c040ad867..ee859d2c9 100644 --- a/controllers/core/openstackversion_controller.go +++ b/controllers/core/openstackversion_controller.go @@ -191,7 +191,7 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req Log.Info("Target version not found in defaults", "targetVersion", instance.Spec.TargetVersion) return ctrl.Result{}, nil } - instance.Status.ContainerImages = openstack.GetContainerImages(ctx, val, *instance) + instance.Status.ContainerImages = openstack.GetContainerImages(val, *instance) instance.Status.Conditions.MarkTrue( corev1beta1.OpenStackVersionInitialized, @@ -209,9 +209,8 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req if k8s_errors.IsNotFound(err) { Log.Info("No controlplane found.") return ctrl.Result{}, nil - } else { - return ctrl.Result{}, err } + return ctrl.Result{}, err } // greenfield deployment //FIXME check dataplane here too diff --git a/pkg/openstack/ca.go b/pkg/openstack/ca.go index 33fa8dba3..ed0cfe447 100644 --- a/pkg/openstack/ca.go +++ b/pkg/openstack/ca.go @@ -136,6 +136,11 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h bundle, caOnlyBundle, ) + if err != nil { + return ctrl.Result{}, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } } // create CA for internal podLevel termination @@ -178,6 +183,12 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h bundle, caOnlyBundle, ) + if err != nil { + return ctrl.Result{}, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + } // create CA for ovn @@ -219,6 +230,12 @@ func ReconcileCAs(ctx context.Context, instance *corev1.OpenStackControlPlane, h bundle, caOnlyBundle, ) + if err != nil { + return ctrl.Result{}, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + } instance.Status.Conditions.MarkTrue(corev1.OpenStackControlPlaneCAReadyCondition, corev1.OpenStackControlPlaneCAReadyMessage) diff --git a/pkg/openstack/galera.go b/pkg/openstack/galera.go index ed19be934..e8f0c3135 100644 --- a/pkg/openstack/galera.go +++ b/pkg/openstack/galera.go @@ -38,8 +38,8 @@ func ReconcileGaleras( return ctrl.Result{}, nil } - var failures []string = []string{} - var inprogress []string = []string{} + var failures = []string{} + var inprogress = []string{} for name, spec := range instance.Spec.Galera.Templates { hostname := fmt.Sprintf("%s.%s.svc", name, instance.Namespace) diff --git a/pkg/openstack/memcached.go b/pkg/openstack/memcached.go index 5e0742cd2..cf1f8d62c 100644 --- a/pkg/openstack/memcached.go +++ b/pkg/openstack/memcached.go @@ -35,8 +35,8 @@ func ReconcileMemcacheds( version *corev1beta1.OpenStackVersion, helper *helper.Helper, ) (ctrl.Result, error) { - var failures []string = []string{} - var inprogress []string = []string{} + var failures = []string{} + var inprogress = []string{} // We first remove memcacheds no longer owned memcacheds := &memcachedv1.MemcachedList{} diff --git a/pkg/openstack/nova.go b/pkg/openstack/nova.go index 1efff7441..8989d8698 100644 --- a/pkg/openstack/nova.go +++ b/pkg/openstack/nova.go @@ -352,9 +352,9 @@ func cellMetadataLabelMap(name string, cell string) map[string]string { } func metadataEnabled(metadata novav1.NovaMetadataTemplate) bool { - return metadata.Enabled != nil && *metadata.Enabled == true + return metadata.Enabled != nil && *metadata.Enabled } func noVNCProxyEnabled(vncproxy novav1.NovaNoVNCProxyTemplate) bool { - return vncproxy.Enabled != nil && *vncproxy.Enabled == true + return vncproxy.Enabled != nil && *vncproxy.Enabled } diff --git a/pkg/openstack/ovn.go b/pkg/openstack/ovn.go index f8e8ba2ea..a19341152 100644 --- a/pkg/openstack/ovn.go +++ b/pkg/openstack/ovn.go @@ -270,7 +270,7 @@ func ReconcileOVNNorthd(ctx context.Context, instance *corev1beta1.OpenStackCont Log.Info(fmt.Sprintf("OVNNorthd %s - %s", OVNNorthd.Name, op)) } - if OVNNorthd.Status.ObservedGeneration == OVNNorthd.Generation && OVNNorthd.IsReady() { + if OVNNorthd.Status.ObservedGeneration == OVNNorthd.Generation && OVNNorthd.IsReady() { //revive:disable:indent-error-flow instance.Status.ContainerImages.OvnNorthdImage = version.Status.ContainerImages.OvnNorthdImage return true, nil } else { diff --git a/pkg/openstack/rabbitmq.go b/pkg/openstack/rabbitmq.go index 90a6b54d8..831c9212c 100644 --- a/pkg/openstack/rabbitmq.go +++ b/pkg/openstack/rabbitmq.go @@ -41,8 +41,8 @@ func ReconcileRabbitMQs( version *corev1beta1.OpenStackVersion, helper *helper.Helper, ) (ctrl.Result, error) { - var failures []string = []string{} - var inprogress []string = []string{} + var failures = []string{} + var inprogress = []string{} var ctrlResult ctrl.Result var err error var status mqStatus diff --git a/pkg/openstack/version.go b/pkg/openstack/version.go index 7f1ff4c66..764c59443 100644 --- a/pkg/openstack/version.go +++ b/pkg/openstack/version.go @@ -70,7 +70,7 @@ func getImg(val1 *string, val2 *string) *string { } // GetContainerImages - initializes OpenStackVersion CR with default container images -func GetContainerImages(ctx context.Context, defaults *corev1beta1.ContainerDefaults, instance corev1beta1.OpenStackVersion) corev1beta1.ContainerImages { +func GetContainerImages(defaults *corev1beta1.ContainerDefaults, instance corev1beta1.OpenStackVersion) corev1beta1.ContainerImages { containerImages := corev1beta1.ContainerImages{ CinderVolumeImages: instance.Spec.CustomContainerImages.CinderVolumeImages, diff --git a/pkg/openstackclient/funcs.go b/pkg/openstackclient/funcs.go index fb84ebc24..64bbfc17c 100644 --- a/pkg/openstackclient/funcs.go +++ b/pkg/openstackclient/funcs.go @@ -32,7 +32,6 @@ func ClientPodSpec( ctx context.Context, instance *clientv1.OpenStackClient, helper *helper.Helper, - labels map[string]string, configHash string, ) corev1.PodSpec { envVars := map[string]env.Setter{} diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index 6eb060015..1ae532a8c 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -19,7 +19,7 @@ package functional_test import ( "encoding/base64" - . "github.com/onsi/gomega" + . "github.com/onsi/gomega" //revive:disable:dot-imports k8s_corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" diff --git a/tests/functional/openstackclient_webhook_test.go b/tests/functional/openstackclient_webhook_test.go index 1fa3eb870..88a8ce40f 100644 --- a/tests/functional/openstackclient_webhook_test.go +++ b/tests/functional/openstackclient_webhook_test.go @@ -19,8 +19,8 @@ package functional_test import ( "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.io/apimachinery/pkg/types" openstackclientv1 "github.com/openstack-k8s-operators/openstack-operator/apis/client/v1beta1" diff --git a/tests/functional/openstackoperator_controller_test.go b/tests/functional/openstackoperator_controller_test.go index 2452fc887..d69ccc924 100644 --- a/tests/functional/openstackoperator_controller_test.go +++ b/tests/functional/openstackoperator_controller_test.go @@ -20,8 +20,12 @@ 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 + + //revive:disable-next-line:dot-imports + . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" + k8s_corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -30,7 +34,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1" - . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" "github.com/openstack-k8s-operators/lib-common/modules/common/tls" manilav1 "github.com/openstack-k8s-operators/manila-operator/api/v1beta1" clientv1 "github.com/openstack-k8s-operators/openstack-operator/apis/client/v1beta1" @@ -268,13 +271,11 @@ var _ = Describe("OpenStackOperator controller", func() { Expect(OSCtlplane.Spec.Manila.Enabled).Should(BeTrue()) // manila exists + manila := &manilav1.Manila{} Eventually(func(g Gomega) { - manila := &manilav1.Manila{} - Eventually(func(g Gomega) { - g.Expect(k8sClient.Get(ctx, names.ManilaName, manila)).Should(Succeed()) - g.Expect(manila).ShouldNot(BeNil()) - }, timeout, interval).Should(Succeed()) - }) + g.Expect(k8sClient.Get(ctx, names.ManilaName, manila)).Should(Succeed()) + g.Expect(manila).ShouldNot(BeNil()) + }, timeout, interval).Should(Succeed()) // FIXME add helpers to manila-operator to simulate ready state Eventually(func(g Gomega) { @@ -305,13 +306,11 @@ var _ = Describe("OpenStackOperator controller", func() { Expect(OSCtlplane.Spec.Manila.Enabled).Should(BeTrue()) // manila exists + manila := &manilav1.Manila{} Eventually(func(g Gomega) { - manila := &manilav1.Manila{} - Eventually(func(g Gomega) { - g.Expect(k8sClient.Get(ctx, names.ManilaName, manila)).Should(Succeed()) - g.Expect(manila).ShouldNot(BeNil()) - }, timeout, interval).Should(Succeed()) - }) + g.Expect(k8sClient.Get(ctx, names.ManilaName, manila)).Should(Succeed()) + g.Expect(manila).ShouldNot(BeNil()) + }, timeout, interval).Should(Succeed()) // FIXME add helpers to manila-operator to simulate ready state Eventually(func(g Gomega) { @@ -377,13 +376,11 @@ var _ = Describe("OpenStackOperator controller", func() { Expect(OSCtlplane.Spec.Cinder.Enabled).Should(BeTrue()) // cinder exists + cinder := &cinderv1.Cinder{} Eventually(func(g Gomega) { - cinder := &cinderv1.Cinder{} - Eventually(func(g Gomega) { - g.Expect(k8sClient.Get(ctx, names.CinderName, cinder)).Should(Succeed()) - g.Expect(cinder).ShouldNot(BeNil()) - }, timeout, interval).Should(Succeed()) - }) + g.Expect(k8sClient.Get(ctx, names.CinderName, cinder)).Should(Succeed()) + g.Expect(cinder).ShouldNot(BeNil()) + }, timeout, interval).Should(Succeed()) // FIXME add helpers to cinder-operator to simulate ready state Eventually(func(g Gomega) { @@ -414,14 +411,11 @@ var _ = Describe("OpenStackOperator controller", func() { OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName) Expect(OSCtlplane.Spec.Cinder.Enabled).Should(BeTrue()) - // cinder exists + cinder := &cinderv1.Cinder{} Eventually(func(g Gomega) { - cinder := &cinderv1.Cinder{} - Eventually(func(g Gomega) { - g.Expect(k8sClient.Get(ctx, names.CinderName, cinder)).Should(Succeed()) - g.Expect(cinder).ShouldNot(BeNil()) - }, timeout, interval).Should(Succeed()) - }) + g.Expect(k8sClient.Get(ctx, names.CinderName, cinder)).Should(Succeed()) + g.Expect(cinder).ShouldNot(BeNil()) + }, timeout, interval).Should(Succeed()) // FIXME add helpers to cinder-operator to simulate ready state Eventually(func(g Gomega) { diff --git a/tests/functional/openstackversion_controller_test.go b/tests/functional/openstackversion_controller_test.go index 95e2af472..c0a8e6a7d 100644 --- a/tests/functional/openstackversion_controller_test.go +++ b/tests/functional/openstackversion_controller_test.go @@ -19,12 +19,14 @@ package functional_test import ( "os" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - k8s_corev1 "k8s.io/api/core/v1" + . "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" + corev1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" + k8s_corev1 "k8s.io/api/core/v1" ) var _ = Describe("OpenStackOperator controller", func() { diff --git a/tests/functional/suite_test.go b/tests/functional/suite_test.go index caafa9f98..dae7497e5 100644 --- a/tests/functional/suite_test.go +++ b/tests/functional/suite_test.go @@ -11,8 +11,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 "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/types"