Skip to content

Commit

Permalink
Ensure golangci-lint runs on all files
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gibizer committed Apr 15, 2024
1 parent 69252e9 commit 097bfb0
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 81 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 6 additions & 53 deletions controllers/placementapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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{}
Expand Down Expand Up @@ -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{}
}
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions pkg/placement/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -36,8 +33,6 @@ import (

// Deployment func
func Deployment(
ctx context.Context,
helper *helper.Helper,
instance *placementv1.PlacementAPI,
configHash string,
labels map[string]string,
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 9 additions & 7 deletions tests/functional/placementapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)

Expand All @@ -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", "")
})
Expand Down
24 changes: 12 additions & 12 deletions tests/functional/placementapi_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
}
Expand All @@ -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,
))
Expand All @@ -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",
))
Expand All @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 097bfb0

Please sign in to comment.