chore(ci): add golangci-lint and refactor according to the result

shardingsphere-operator/.golangci.yml

@@ -0,0 +1,85 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+run:
+  timeout: 10m
+linters:
+  disable-all: true
+  enable:
+    - ineffassign
+    - typecheck
+    - varcheck
+    - unused
+    - structcheck
+    - deadcode
+    - gosimple
+    - goimports
+    - errcheck
+    - staticcheck
+    - stylecheck
+    - gosec
+    - asciicheck
+    - bodyclose
+    - exportloopref
+    - rowserrcheck
+    - makezero
+    - durationcheck
+    - prealloc
+    - predeclared
+
+# Refers: https://gist.github.com/maratori/47a4d00457a92aa426dbd48a18776322
+linters-settings:
+  cyclop:
+    # The maximal code complexity to report.
+    # Default: 10
+    max-complexity: 30
+    # The maximal average package complexity.
+    # If it's higher than 0.0 (float) the check is enabled
+    # Default: 0.0
+    package-average: 10.0
+  errcheck:
+    # Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
+    # Such cases aren't reported by default.
+    # Default: false
+    check-type-assertions: true
+  exhaustive:
+    # Program elements to check for exhaustiveness.
+    # Default: [ switch ]
+    check:
+      - switch
+      - map
+  funlen:
+    # Checks the number of lines in a function.
+    # If lower than 0, disable the check.
+    # Default: 60
+    lines: 100
+    # Checks the number of statements in a function.
+    # If lower than 0, disable the check.
+    # Default: 40
+    statements: 50
+  gocognit:
+    # Minimal code complexity to report.
+    # Default: 30 (but we recommend 10-20)
+    min-complexity: 20
+issues:
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - errcheck
+        - gosec
+        - rowserrcheck
+        - makezero

shardingsphere-operator/Makefile

@@ -111,6 +111,7 @@ $(LOCALBIN):
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 ENVTEST ?= $(LOCALBIN)/setup-envtest
+CHECK_LINT?= $(LOCALBIN)/setup-golangci-lint
 
 ## Tool Versions
 KUSTOMIZE_VERSION ?= v4.5.7
@@ -131,3 +132,11 @@ $(CONTROLLER_GEN): $(LOCALBIN)
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)
 	GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+
+.PHONY: check
+check: check-lint 
+
+.PHONY: check-lint
+check-lint: $(CHECK_LINT) ## Download golangci-lint-setup locally if necessary.
+$(CHECK_LINT): $(LOCALBIN)
+	GOBIN=$(LOCALBIN) CGO_ENABLED=0 golangci-lint run -v

shardingsphere-operator/api/v1alpha1/compute_node_types.go

@@ -19,7 +19,6 @@ package v1alpha1
 
 import (
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -274,7 +273,7 @@ type ComputeNodeSpec struct {
 	// +optional
 	Env []corev1.EnvVar `json:"env,omitempty"`
 	// +optional
-	Resources v1.ResourceRequirements `json:"resources,omitempty"`
+	Resources corev1.ResourceRequirements `json:"resources,omitempty"`
 	// +optional
 	PortBindings []PortBinding `json:"portBindings,omitempty" yaml:"portBinding"`
 

shardingsphere-operator/api/v1alpha1/proxy_status.go

@@ -19,7 +19,6 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 type PhaseStatus string
@@ -67,7 +66,7 @@ type Conditions []Condition
 // | NotReady      | Failed     | ShardingSphere-Proxy failed to start correctly due to some problems|
 
 type Condition struct {
-	Type           ConditionType      `json:"type"`
-	Status         v1.ConditionStatus `json:"status"`
-	LastUpdateTime metav1.Time        `json:"lastUpdateTime,omitempty"`
+	Type           ConditionType          `json:"type"`
+	Status         metav1.ConditionStatus `json:"status"`
+	LastUpdateTime metav1.Time            `json:"lastUpdateTime,omitempty"`
 }

shardingsphere-operator/pkg/controllers/compute_node_controller.go

@@ -298,7 +298,7 @@ func newConditions(conditions []v1alpha1.ComputeNodeCondition, cond v1alpha1.Com
 	}
 
 	found := false
-	for idx, _ := range conditions {
+	for idx := range conditions {
 		if conditions[idx].Type == cond.Type {
 			conditions[idx].LastUpdateTime = cond.LastUpdateTime
 			conditions[idx].Status = cond.Status
@@ -321,7 +321,7 @@ func updateReadyConditions(conditions []v1alpha1.ComputeNodeCondition, cond v1al
 func updateNotReadyConditions(conditions []v1alpha1.ComputeNodeCondition, cond v1alpha1.ComputeNodeCondition) []v1alpha1.ComputeNodeCondition {
 	cur := newConditions(conditions, cond)
 
-	for idx, _ := range cur {
+	for idx := range cur {
 		if cur[idx].Type == v1alpha1.ComputeNodeConditionReady {
 			cur[idx].LastUpdateTime = metav1.Now()
 			cur[idx].Status = v1alpha1.ConditionStatusFalse

shardingsphere-operator/pkg/controllers/compute_node_controller_test.go

@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package controllers
+
+import (
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+)
+
+func Test_GetReadyProxyInstances(t *testing.T) {
+	// create sample PodList
+	podlist := v1.PodList{
+		Items: []v1.Pod{
+			{
+				Status: v1.PodStatus{
+					Phase: v1.PodRunning,
+					Conditions: []v1.PodCondition{
+						{
+							Type:   v1.PodReady,
+							Status: v1.ConditionTrue,
+						},
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						{
+							Name:  "shardingsphere-proxy",
+							Ready: true,
+						},
+					},
+				},
+			},
+			{
+				Status: v1.PodStatus{
+					Phase: v1.PodRunning,
+					Conditions: []v1.PodCondition{
+						{
+							Type:   v1.PodReady,
+							Status: v1.ConditionTrue,
+						},
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						{
+							Name:  "another-container",
+							Ready: true,
+						},
+					},
+				},
+			},
+			{
+				Status: v1.PodStatus{
+					Phase: v1.PodRunning,
+					Conditions: []v1.PodCondition{
+						{
+							Type:   v1.PodReady,
+							Status: v1.ConditionFalse,
+						},
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						{
+							Name:  "shardingsphere-proxy",
+							Ready: false,
+						},
+					},
+				},
+			},
+			{
+				Status: v1.PodStatus{
+					Phase: v1.PodPending,
+					Conditions: []v1.PodCondition{
+						{
+							Type:   v1.PodReady,
+							Status: v1.ConditionTrue,
+						},
+					},
+					ContainerStatuses: []v1.ContainerStatus{
+						{
+							Name:  "shardingsphere-proxy",
+							Ready: true,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	// expected result is 1 because only one pod has a ready shardingsphere-proxy container
+	expected := int32(1)
+
+	// call the function to get the actual result
+	actual := getReadyProxyInstances(podlist)
+
+	// compare the expected and actual results
+	if actual != expected {
+		t.Errorf("getReadyInstances returned %d, expected %d", actual, expected)
+	}
+}

shardingsphere-operator/pkg/controllers/proxy_controller.go

@@ -66,7 +66,7 @@ type ProxyReconciler struct {
 // move the current state of the cluster closer to the desired state.
 
 func (r *ProxyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	logger := r.Log.WithValues(computeNodeControllerName, req.NamespacedName)
+	logger := r.Log.WithValues(proxyControllerName, req.NamespacedName)
 
 	rt, err := r.getRuntimeShardingSphereProxy(ctx, req.NamespacedName)
 	if apierrors.IsNotFound(err) {

shardingsphere-operator/pkg/controllers/proxyconfig_controller.go

@@ -60,7 +60,7 @@ type ProxyConfigReconciler struct {
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.0/pkg/reconcile
 func (r *ProxyConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	logger := r.Log.WithValues(computeNodeControllerName, req.NamespacedName)
+	logger := r.Log.WithValues(proxyConfigControllerName, req.NamespacedName)
 
 	run := &shardingspherev1alpha1.ShardingSphereProxyServerConfig{}
 	err := r.Get(ctx, req.NamespacedName, run)

shardingsphere-operator/pkg/reconcile/computenode/configmap.go

@@ -61,7 +61,9 @@ func NewConfigMap(cn *v1alpha1.ComputeNode) *v1.ConfigMap {
 		servconf := cn.Spec.Bootstrap.ServerConfig.DeepCopy()
 		if cn.Spec.Bootstrap.ServerConfig.Mode.Type == v1alpha1.ModeTypeCluster {
 			if len(cluster) > 0 {
-				json.Unmarshal([]byte(cluster), &servconf.Mode.Repository)
+				if err := json.Unmarshal([]byte(cluster), &servconf.Mode.Repository); err != nil {
+					return &v1.ConfigMap{}
+				}
 			}
 		}
 		if y, err := yaml.Marshal(servconf); err == nil {

shardingsphere-operator/pkg/reconcile/computenode/configmap_test.go

@@ -28,8 +28,8 @@ import (
 
 var _ = Describe("ConfigMap", func() {
 	var (
-		expect *corev1.ConfigMap     = &corev1.ConfigMap{}
-		cn     *v1alpha1.ComputeNode = &v1alpha1.ComputeNode{
+		expect = &corev1.ConfigMap{}
+		cn     = &v1alpha1.ComputeNode{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test_name",
 				Namespace: "test_namespace",

shardingsphere-operator/pkg/reconcile/computenode/deployment.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/apache/shardingsphere-on-cloud/shardingsphere-operator/api/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
-	v1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -94,11 +93,6 @@ func NewShardingSphereProxyContainerBuilder() ShardingSphereProxyContainerBuilde
 	}
 }
 
-// Build returns a Container
-func (b *shardingSphereProxyContainerBuilder) Build() *corev1.Container {
-	return b.ContainerBuilder.Build()
-}
-
 // BootstrapContainerBuilder returns a Container for initialization
 // The container will handle initilialization in Pod's InitContainer
 type BootstrapContainerBuilder interface {
@@ -406,7 +400,7 @@ func (b *sharedVolumeAndMountBuilder) SetVolumeMountSize(size int) SharedVolumeA
 	if len(b.volumeMounts) != size {
 		vms := make([]*corev1.VolumeMount, size)
 		for vm := range b.volumeMounts {
-			vms = append(vms, b.volumeMounts[vm].DeepCopy())
+			vms[vm] = b.volumeMounts[vm].DeepCopy()
 		}
 		b.volumeMounts = vms
 	}
@@ -541,7 +535,7 @@ func (d *deploymentBuilder) Build() *appsv1.Deployment {
 }
 
 // NewDeployment creates a new Deployment
-func NewDeployment(cn *v1alpha1.ComputeNode) *v1.Deployment {
+func NewDeployment(cn *v1alpha1.ComputeNode) *appsv1.Deployment {
 	builder := NewDeploymentBuilder(cn.GetObjectMeta(), cn.GetObjectKind().GroupVersionKind())
 	builder.SetName(cn.Name).SetNamespace(cn.Namespace).SetLabelsAndSelectors(cn.Labels, cn.Spec.Selector).SetAnnotations(cn.Annotations).SetReplicas(&cn.Spec.Replicas)
 
@@ -681,11 +675,11 @@ func (d *deploymentBuilder) SetAgentBin(scb ContainerBuilder, cn *v1alpha1.Compu
 }
 
 // DefaultDeployment describes the default deployment
-func DefaultDeployment(meta metav1.Object, gvk schema.GroupVersionKind) *v1.Deployment {
+func DefaultDeployment(meta metav1.Object, gvk schema.GroupVersionKind) *appsv1.Deployment {
 	defaultMaxUnavailable := intstr.FromInt(0)
 	defaultMaxSurge := intstr.FromInt(3)
 
-	return &v1.Deployment{
+	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "shardingsphere-proxy",
 			Namespace: "default",
@@ -694,10 +688,10 @@ func DefaultDeployment(meta metav1.Object, gvk schema.GroupVersionKind) *v1.Depl
 				*metav1.NewControllerRef(meta, gvk),
 			},
 		},
-		Spec: v1.DeploymentSpec{
-			Strategy: v1.DeploymentStrategy{
-				Type: v1.RollingUpdateDeploymentStrategyType,
-				RollingUpdate: &v1.RollingUpdateDeployment{
+		Spec: appsv1.DeploymentSpec{
+			Strategy: appsv1.DeploymentStrategy{
+				Type: appsv1.RollingUpdateDeploymentStrategyType,
+				RollingUpdate: &appsv1.RollingUpdateDeployment{
 					MaxUnavailable: &defaultMaxUnavailable,
 					MaxSurge:       &defaultMaxSurge,
 				},
@@ -742,8 +736,8 @@ func DefaultDeployment(meta metav1.Object, gvk schema.GroupVersionKind) *v1.Depl
 }
 
 // UpdateDeployment updates the deployment
-func UpdateDeployment(cn *v1alpha1.ComputeNode, cur *v1.Deployment) *v1.Deployment {
-	exp := &v1.Deployment{}
+func UpdateDeployment(cn *v1alpha1.ComputeNode, cur *appsv1.Deployment) *appsv1.Deployment {
+	exp := &appsv1.Deployment{}
 	exp.ObjectMeta = cur.ObjectMeta
 	exp.ObjectMeta.ResourceVersion = ""
 	exp.Labels = cur.Labels