Skip to content

Commit

Permalink
chore(ci): add golangci-lint and refactor according to the result (#294)
Browse files Browse the repository at this point in the history
* chore: add golangci-lint check config and update Makefile for check

* fix: fix according to golangci-lint

* fix: rename Api to API

* fix: fix sharedVolumeAndMountBuild SetVolumeMountSize non-zero slice

* fix: remove overlapped Build method from shardingSphereProxyContainerBuilder

* chore: add license header
  • Loading branch information
mlycore authored Apr 5, 2023
1 parent 91ccfa0 commit c02a11e
Show file tree
Hide file tree
Showing 18 changed files with 265 additions and 68 deletions.
85 changes: 85 additions & 0 deletions shardingsphere-operator/.golangci.yml
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions shardingsphere-operator/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
3 changes: 1 addition & 2 deletions shardingsphere-operator/api/v1alpha1/compute_node_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

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

Expand Down
7 changes: 3 additions & 4 deletions shardingsphere-operator/api/v1alpha1/proxy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"`
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 10 additions & 16 deletions shardingsphere-operator/pkg/reconcile/computenode/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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",
Expand All @@ -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,
},
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit c02a11e

Please sign in to comment.