Enable golangci-lint (#330)

Add golangci-lint to the CI pipeline to help maintain code cleanliness.
* Fixup various issues discovered.

Signed-off-by: SuperQ <superq@gmail.com>

Author: SuperQ

.github/workflows/ci.yml

@@ -38,6 +38,11 @@ jobs:
       - name: Code Check
         run: |
           make code-check
+      - name: Lint
+        uses: golangci/golangci-lint-action@1481404843c368bc19ca9406f87d6e0fc97bdcfd # v7.0.0
+        with:
+          args: --verbose
+          version: v2.0.2
   unit-test:
     name: Unit Test
     runs-on: ubuntu-latest

.golangci.yml

@@ -0,0 +1,18 @@
+version: "2"
+formatters:
+  enable:
+    - goimports
+  settings:
+    goimports:
+      local-prefixes:
+        - github.com/zilliztech/milvus-operator
+linters:
+  exclusions:
+    rules:
+      - linters:
+          - errcheck
+        # Taken from the default exclusions in v1.
+        text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+      - linters:
+          - errcheck
+        path: _test.go

apis/milvus.io/v1alpha1/milvus_webhook.go

@@ -19,13 +19,10 @@ package v1alpha1
 import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/conversion"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	v1beta1 "github.com/zilliztech/milvus-operator/apis/milvus.io/v1beta1"
 )
 
-var milvuslog = logf.Log.WithName("milvus-v1alpha1")
-
 func (r *Milvus) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).

apis/milvus.io/v1beta1/milvus_types.go

@@ -114,7 +114,7 @@ func (ms MilvusSpec) GetServiceComponent() *ServiceComponent {
 // GetMilvusVersionByImage returns the version of Milvus by ms.Com.ComponentSpec.Image
 func (ms MilvusSpec) GetMilvusVersionByImage() (semver.Version, error) {
 	// parse format: registry/namespace/image:tag
-	splited := strings.Split(ms.Com.ComponentSpec.Image, ":")
+	splited := strings.Split(ms.Com.Image, ":")
 	if len(splited) != 2 {
 		return semver.Version{}, errors.Errorf("unknown version of image[%s]", splited[0])
 	}

apis/milvus.io/v1beta1/milvus_types_test.go

@@ -206,15 +206,15 @@ func TestGetMilvusVersionByGlobalImage(t *testing.T) {
 	_, err = m.Spec.GetMilvusVersionByImage()
 	assert.NoError(t, err)
 
-	m.Spec.Com.ComponentSpec.Image = "milvusdb/milvus:v2.3.1-beta1"
+	m.Spec.Com.Image = "milvusdb/milvus:v2.3.1-beta1"
 	ver, err := m.Spec.GetMilvusVersionByImage()
 	assert.NoError(t, err)
 	assert.Equal(t, uint64(2), ver.Major)
 	assert.Equal(t, uint64(3), ver.Minor)
 	assert.Equal(t, uint64(1), ver.Patch)
 	assert.Equal(t, "beta1", ver.Pre[0].VersionStr)
 
-	m.Spec.Com.ComponentSpec.Image = "harbor.milvus.io/milvus/milvus:latest"
+	m.Spec.Com.Image = "harbor.milvus.io/milvus/milvus:latest"
 	_, err = m.Spec.GetMilvusVersionByImage()
 	assert.Error(t, err)
 }

apis/milvus.io/v1beta1/milvus_webhook.go

@@ -27,7 +27,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -36,9 +35,6 @@ import (
 	"github.com/zilliztech/milvus-operator/pkg/util"
 )
 
-// log is for logging in this package.
-var milvuslog = logf.Log.WithName("milvus-resource")
-
 func (r *Milvus) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).

apis/milvus.io/v1beta1/persistence_types.go

@@ -46,6 +46,6 @@ type PersistentVolumeClaim struct {
 
 func (p *PersistentVolumeClaim) GetSpec() *corev1.PersistentVolumeClaimSpec {
 	ret := new(corev1.PersistentVolumeClaimSpec)
-	p.Spec.AsObject(ret)
+	p.Spec.AsObject(ret) //nolint:errcheck
 	return ret
 }

main.go

@@ -48,8 +48,8 @@ func main() {
 	var enablePprof bool
 	var probeAddr string
 	var workDir string
-	var k8sQps int = 100
-	var k8sBurst int = 100
+	var k8sQps = 100
+	var k8sBurst = 100
 	var enableWebhook bool
 	showVersion := flag.Bool("version", false, "Show version")
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")

pkg/config/config.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"io/ioutil"
 	"os"
 
 	corev1 "k8s.io/api/core/v1"
@@ -75,12 +74,12 @@ func NewConfig(workDir string) (*Config, error) {
 
 	templateDir := workDir + TemplateRelativeDir
 
-	tmpls, err := ioutil.ReadDir(templateDir)
+	tmpls, err := os.ReadDir(templateDir)
 	if err != nil {
 		return nil, err
 	}
 	for _, tmpl := range tmpls {
-		data, err := ioutil.ReadFile(templateDir + "/" + tmpl.Name())
+		data, err := os.ReadFile(templateDir + "/" + tmpl.Name())
 		if err != nil {
 			return nil, err
 		}

pkg/controllers/components_test.go

@@ -301,7 +301,7 @@ func TestMilvusComponent_GetReplicas(t *testing.T) {
 	spec.Com.QueryNode = &v1beta1.MilvusQueryNode{}
 	com := QueryNode
 	replica := int32(1)
-	spec.Com.QueryNode.Component.Replicas = &replica
+	spec.Com.QueryNode.Replicas = &replica
 	assert.Equal(t, &replica, com.GetReplicas(spec))
 }
 
@@ -442,7 +442,7 @@ func TestMilvusComponent_GetComponentPort(t *testing.T) {
 
 func TestMilvusComponent_GetComponentSpec(t *testing.T) {
 	spec := newSpecCluster()
-	spec.Com.QueryNode.Component.ComponentSpec.Image = "a"
+	spec.Com.QueryNode.Image = "a"
 	com := QueryNode
 	assert.Equal(t, "a", com.GetComponentSpec(spec).Image)
 }

pkg/controllers/dependencies.go

@@ -67,7 +67,7 @@ func (l LocalHelmReconciler) NewHelmCfg(namespace string) *action.Configuration
 	}
 
 	// cfg.Init will never return err, only panic if bad driver
-	cfg.Init(
+	_ = cfg.Init(
 		getRESTClientGetterFromClient(l.helmSettings, namespace, l.mgr),
 		namespace,
 		os.Getenv("HELM_DRIVER"),
@@ -77,20 +77,6 @@ func (l LocalHelmReconciler) NewHelmCfg(namespace string) *action.Configuration
 	return cfg
 }
 
-func getRESTClientGetterWithNamespace(env *cli.EnvSettings, namespace string) genericclioptions.RESTClientGetter {
-	return &genericclioptions.ConfigFlags{
-		Namespace:        &namespace,
-		Context:          &env.KubeContext,
-		BearerToken:      &env.KubeToken,
-		APIServer:        &env.KubeAPIServer,
-		CAFile:           &env.KubeCaFile,
-		KubeConfig:       &env.KubeConfig,
-		Impersonate:      &env.KubeAsUser,
-		ImpersonateGroup: &env.KubeAsGroups,
-		Insecure:         &configFlagInsecure,
-	}
-}
-
 func getRESTClientGetterFromClient(env *cli.EnvSettings, namespace string, mgr manager.Manager) genericclioptions.RESTClientGetter {
 	return &clientRESTClientGetter{
 		namespace:  namespace,

pkg/controllers/deploy_ctrl.go

@@ -228,8 +228,8 @@ func (c *DeployControllerBizImpl) HandleCreate(ctx context.Context, mc v1beta1.M
 	if err == nil {
 		return nil
 	}
-	switch {
-	case err == ErrNotFound:
+	switch err {
+	case ErrNotFound:
 		err := c.util.MarkMilvusComponentGroupId(ctx, mc, c.component, 0)
 		if err != nil {
 			return errors.Wrapf(err, "mark milvus %s group id to %d", c.component.Name, 0)
@@ -238,7 +238,7 @@ func (c *DeployControllerBizImpl) HandleCreate(ctx context.Context, mc v1beta1.M
 		if err != nil {
 			return errors.Wrapf(err, "create %s deployment 0", c.component.Name)
 		}
-	case err == ErrNoLastDeployment:
+	case ErrNoLastDeployment:
 		return c.util.CreateDeploy(ctx, mc, nil, 1)
 	default:
 		return errors.Wrapf(err, "get %s deploys", c.component.Name)

pkg/controllers/deploy_ctrl_util.go

@@ -261,7 +261,7 @@ func (c *DeployControllerBizUtilImpl) LastRolloutFinished(ctx context.Context, m
 		return false, nil
 	}
 	// make sure all old pods are down
-	pods, err := c.K8sUtil.ListDeployPods(ctx, lastDeployment, c.component)
+	pods, err := c.ListDeployPods(ctx, lastDeployment, c.component)
 	if err != nil {
 		return false, err
 	}
@@ -497,7 +497,7 @@ func (c *DeployControllerBizUtilImpl) doScaleAction(ctx context.Context, action
 		return nil
 	}
 	action.deploy.Spec.Replicas = int32Ptr(getDeployReplicas(action.deploy) + action.replicaChange)
-	return c.K8sUtil.UpdateAndRequeue(ctx, action.deploy)
+	return c.UpdateAndRequeue(ctx, action.deploy)
 }
 
 func (c *DeployControllerBizUtilImpl) markDeployAsCurrent(ctx context.Context, mc v1beta1.Milvus, currentDeployment *appsv1.Deployment) error {
@@ -510,20 +510,20 @@ func (c *DeployControllerBizUtilImpl) markDeployAsCurrent(ctx context.Context, m
 }
 
 func (c *DeployControllerBizUtilImpl) checkDeploymentsStable(ctx context.Context, currentDeployment, lastDeployment *appsv1.Deployment) error {
-	lastDeployPods, err := c.K8sUtil.ListDeployPods(ctx, lastDeployment, c.component)
+	lastDeployPods, err := c.ListDeployPods(ctx, lastDeployment, c.component)
 	if err != nil {
 		return errors.Wrap(err, "list last deploy pods")
 	}
-	isStable, reason := c.K8sUtil.DeploymentIsStable(lastDeployment, lastDeployPods)
+	isStable, reason := c.DeploymentIsStable(lastDeployment, lastDeployPods)
 	if !isStable {
 		return errors.Wrapf(ErrRequeue, "last deploy is not stable[%s]", reason)
 	}
 
-	currentDeployPods, err := c.K8sUtil.ListDeployPods(ctx, currentDeployment, c.component)
+	currentDeployPods, err := c.ListDeployPods(ctx, currentDeployment, c.component)
 	if err != nil {
 		return errors.Wrap(err, "list current deploy pods")
 	}
-	isStable, reason = c.K8sUtil.DeploymentIsStable(currentDeployment, currentDeployPods)
+	isStable, reason = c.DeploymentIsStable(currentDeployment, currentDeployPods)
 	if !isStable {
 		return errors.Wrapf(ErrRequeue, "current deploy is not stable[%s]", reason)
 	}

pkg/controllers/deployment_updater.go

@@ -409,7 +409,7 @@ func newMilvusDeploymentUpdater(m v1beta1.Milvus, scheme *runtime.Scheme, compon
 }
 
 func (m milvusDeploymentUpdater) GetPersistenceConfig() *v1beta1.Persistence {
-	return m.Milvus.Spec.GetPersistenceConfig()
+	return m.Spec.GetPersistenceConfig()
 }
 
 func (m milvusDeploymentUpdater) GetIntanceName() string {
@@ -455,7 +455,7 @@ func (m milvusDeploymentUpdater) GetInitContainers() []corev1.Container {
 }
 
 func (m milvusDeploymentUpdater) GetDeploymentStrategy() appsv1.DeploymentStrategy {
-	if m.Milvus.Spec.Com.ImageUpdateMode == v1beta1.ImageUpdateModeForce {
+	if m.Spec.Com.ImageUpdateMode == v1beta1.ImageUpdateModeForce {
 		all := intstr.FromString("100%")
 		return appsv1.DeploymentStrategy{
 			Type: appsv1.RollingUpdateDeploymentStrategyType,
@@ -465,7 +465,7 @@ func (m milvusDeploymentUpdater) GetDeploymentStrategy() appsv1.DeploymentStrate
 			},
 		}
 	}
-	return m.component.GetDeploymentStrategy(m.Milvus.Spec.Conf.Data)
+	return m.component.GetDeploymentStrategy(m.Spec.Conf.Data)
 }
 
 func (m milvusDeploymentUpdater) GetConfCheckSum() string {
@@ -480,7 +480,7 @@ func (m milvusDeploymentUpdater) GetMergedComponentSpec() ComponentSpec {
 }
 
 func (m milvusDeploymentUpdater) GetArgs() []string {
-	var ret = []string{}
+	var ret []string
 	if len(m.GetMergedComponentSpec().Commands) > 0 {
 		ret = append([]string{RunScriptPath}, m.GetMergedComponentSpec().Commands...)
 	} else {
@@ -501,7 +501,7 @@ func (m milvusDeploymentUpdater) GetMilvus() *v1beta1.Milvus {
 }
 
 func (m milvusDeploymentUpdater) RollingUpdateImageDependencyReady() bool {
-	if m.Milvus.Status.ObservedGeneration < m.Milvus.Generation {
+	if m.Status.ObservedGeneration < m.Generation {
 		return false
 	}
 	deps := m.component.GetDependencies(m.Spec)
@@ -514,5 +514,5 @@ func (m milvusDeploymentUpdater) RollingUpdateImageDependencyReady() bool {
 }
 
 func (m milvusDeploymentUpdater) HasHookConfig() bool {
-	return len(m.Milvus.Spec.HookConf.Data) > 0
+	return len(m.Spec.HookConf.Data) > 0
 }

pkg/controllers/deployments.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -11,7 +12,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/pkg/errors"
 	pkgerr "github.com/pkg/errors"
 
 	"github.com/zilliztech/milvus-operator/apis/milvus.io/v1beta1"
@@ -40,7 +40,7 @@ const (
 
 var (
 	DefaultConfigMapMode = corev1.ConfigMapVolumeSourceDefaultMode
-	ErrRequeue           = pkgerr.New("requeue")
+	ErrRequeue           = errors.New("requeue")
 )
 
 func GetStorageSecretRefEnv(secretRef string) []corev1.EnvVar {
@@ -79,7 +79,7 @@ func (r *MilvusReconciler) updateDeployment(
 	updater := newMilvusDeploymentUpdater(mc, r.Scheme, component)
 	hasTerminatingPod, err := CheckComponentHasTerminatingPod(ctx, r.Client, mc, component)
 	if err != nil {
-		return errors.Wrap(err, "check component has terminating pod")
+		return pkgerr.Wrap(err, "check component has terminating pod")
 	}
 	if hasTerminatingPod {
 		return updateDeploymentWithoutPodTemplate(deployment, updater)
@@ -148,9 +148,9 @@ func (r *MilvusReconciler) handleOldInstanceChangingMode(ctx context.Context, mc
 
 		mc.Annotations[v1beta1.PodServiceLabelAddedAnnotation] = v1beta1.TrueStr
 		if err := r.Update(ctx, &mc); err != nil {
-			return errors.Wrap(err, "update milvus annotation")
+			return pkgerr.Wrap(err, "update milvus annotation")
 		}
-		return errors.Wrap(ErrRequeue, "requeue after updated milvus annotation")
+		return pkgerr.Wrap(ErrRequeue, "requeue after updated milvus annotation")
 	}
 	return nil
 }

pkg/controllers/ingress_test.go

@@ -97,7 +97,7 @@ func TestMilvusClusterReconciler_ReconcileIngress(t *testing.T) {
 			}).Return(nil)
 		mockClient.EXPECT().Update(gomock.Any(), &rendered).Return(mockErr)
 		err := r.ReconcileIngress(ctx, mc)
-		assert.Equal(t, finalizers, rendered.ObjectMeta.Finalizers)
+		assert.Equal(t, finalizers, rendered.Finalizers)
 		assert.Error(t, err)
 	})
 }