From 54079d23d306e772f9294e7f27e6e0500b728444 Mon Sep 17 00:00:00 2001 From: Farnaz Babaeian Date: Thu, 30 Jan 2025 13:11:40 +0100 Subject: [PATCH] Add golint -> Need to continue --- .github/workflows/pr-check.yaml | 50 +++++++++++++++++++++++++ .golangci.yml | 47 +++++++++++++++++++++++ cloudstack/cloudstack.go | 8 ++-- cloudstack/cloudstack_instances.go | 2 +- cloudstack/cloudstack_instances_test.go | 22 ++++++----- cloudstack/cloudstack_loadbalancer.go | 17 ++++++--- cloudstack/protocol.go | 16 ++++---- cloudstack/service_patcher.go | 1 + 8 files changed, 135 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/pr-check.yaml create mode 100644 .golangci.yml diff --git a/.github/workflows/pr-check.yaml b/.github/workflows/pr-check.yaml new file mode 100644 index 00000000..a446d660 --- /dev/null +++ b/.github/workflows/pr-check.yaml @@ -0,0 +1,50 @@ +name: PR Check + +on: + pull_request: {} + +jobs: + lint: + name: Lint + runs-on: ubuntu-24.04 + steps: + - name: Setup up Go 1.23 + uses: actions/setup-go@v5 + with: + go-version: "1.23" + - name: Check out code + uses: actions/checkout@v4 + - name: golangci-lint + uses: golangci/golangci-lint-action@v6 + with: + version: v1.63.4 + args: --timeout=5m + + build: + name: Test & Build + runs-on: ubuntu-24.04 + steps: + - name: Setup up Go 1.23 + uses: actions/setup-go@v5 + with: + go-version: "1.23" + + - name: Check out code + uses: actions/checkout@v4 + + - name: Cache + uses: actions/cache@v4 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: Run unit tests + run: make test + + - name: Run sanity tests + run: make test-sanity + + - name: Build + run: make diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..3ee0edd7 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,47 @@ +issues: + exclude-use-default: true + max-issues-per-linter: 50 + max-same-issues: 0 # disable + exclude-rules: + - path: _test.go + linters: + - funlen + - nestif + +linters-settings: + gci: + sections: + - standard + - default + - prefix(github.com/leaseweb/cloudstack-csi-driver) + goimports: + local-prefixes: github.com/leaseweb/cloudstack-csi-driver + + misspell: + locale: US + +linters: + enable-all: true + disable: + - execinquery + - cyclop + - depguard + - err113 + - exhaustruct + - funlen + - gochecknoglobals + - gomnd + - inamedparam + - ireturn + - lll + - mnd + - paralleltest + - tagliatelle + - testpackage + - varnamelen + - wrapcheck + - wsl + - execinquery + - gomnd + # Todo remove nestedif + - nestif diff --git a/cloudstack/cloudstack.go b/cloudstack/cloudstack.go index 29fc614c..1b186b8c 100644 --- a/cloudstack/cloudstack.go +++ b/cloudstack/cloudstack.go @@ -49,9 +49,11 @@ type CSConfig struct { } } -var _ cloudprovider.Interface = (*CSCloud)(nil) -var _ cloudprovider.InstancesV2 = (*CSCloud)(nil) -var _ cloudprovider.LoadBalancer = (*CSCloud)(nil) +var ( + _ cloudprovider.Interface = (*CSCloud)(nil) + _ cloudprovider.InstancesV2 = (*CSCloud)(nil) + _ cloudprovider.LoadBalancer = (*CSCloud)(nil) +) // CSCloud is an implementation of Interface for CloudStack. type CSCloud struct { diff --git a/cloudstack/cloudstack_instances.go b/cloudstack/cloudstack_instances.go index 602d389e..cd4bc463 100644 --- a/cloudstack/cloudstack_instances.go +++ b/cloudstack/cloudstack_instances.go @@ -53,7 +53,7 @@ func (cs *CSCloud) nodeAddresses(instance *cloudstack.VirtualMachine) ([]corev1. func (cs *CSCloud) InstanceExists(ctx context.Context, node *corev1.Node) (bool, error) { _, err := cs.getInstance(ctx, node) - if errors.Is(err, cloudprovider.InstanceNotFound) { //nolint: errorlint + if errors.Is(err, cloudprovider.InstanceNotFound) { klog.V(4).Infof("Instance not found for node: %s", node.Name) return false, nil diff --git a/cloudstack/cloudstack_instances_test.go b/cloudstack/cloudstack_instances_test.go index 0328b83c..4af4c011 100644 --- a/cloudstack/cloudstack_instances_test.go +++ b/cloudstack/cloudstack_instances_test.go @@ -6,19 +6,21 @@ import ( "testing" "github.com/apache/cloudstack-go/v2/cloudstack" - "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cloudprovider "k8s.io/cloud-provider" ) +const testVM = "testDummyVM" + func makeInstance(instanceID, privateIP, publicIP string, stateName string) *cloudstack.VirtualMachine { instance := cloudstack.VirtualMachine{ Id: instanceID, - Name: "testDummyVM", - Displayname: "testDummyVM", + Name: testVM, + Displayname: testVM, State: stateName, Zoneid: "1d8d87d4-1425-459c-8d81-c6f57dca2bd2", Zonename: "shouldwork", @@ -101,7 +103,7 @@ func TestInstanceExists(t *testing.T) { client: cs, } - nodeName := "testDummyVM" + nodeName := testVM tests := []struct { name string @@ -139,8 +141,8 @@ func TestInstanceExists(t *testing.T) { t.Run(test.name, func(t *testing.T) { // t.Logf("test node: %v", test.node) if test.node.Spec.ProviderID == "" { - if test.node.Name == "testDummyVM" { - ms.EXPECT().GetVirtualMachineByName("testDummyVM", gomock.Any()).Return(test.mockedCSOutput, 1, nil) + if test.node.Name == testVM { + ms.EXPECT().GetVirtualMachineByName(testVM, gomock.Any()).Return(test.mockedCSOutput, 1, nil) } else { ms.EXPECT().GetVirtualMachineByName("nonExistingVM", gomock.Any()).Return(test.mockedCSOutput, 0, errors.New("No match found for ...")) //nolint: revive } @@ -171,7 +173,7 @@ func TestInstanceShutdown(t *testing.T) { client: cs, } - nodeName := "testDummyVM" + nodeName := testVM tests := []struct { name string @@ -220,7 +222,7 @@ func TestInstanceShutdown(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { if test.node.Spec.ProviderID == "" { - ms.EXPECT().GetVirtualMachineByName("testDummyVM", gomock.Any()).Return(test.mockedCSOutput, 1, nil) + ms.EXPECT().GetVirtualMachineByName(testVM, gomock.Any()).Return(test.mockedCSOutput, 1, nil) } else { ms.EXPECT().GetVirtualMachineByID("915653c4-298b-4d74-bdee-4ced282114f1", gomock.Any()).Return(test.mockedCSOutput, 1, nil) } @@ -248,7 +250,7 @@ func TestInstanceMetadata(t *testing.T) { client: cs, } - nodeName := "testDummyVM" + nodeName := testVM tests := []struct { name string @@ -369,7 +371,7 @@ func TestInstanceMetadata(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { if test.node.Spec.ProviderID == "" { - ms.EXPECT().GetVirtualMachineByName("testDummyVM", gomock.Any()).Return(test.mockedCSOutput, 1, nil) + ms.EXPECT().GetVirtualMachineByName(testVM, gomock.Any()).Return(test.mockedCSOutput, 1, nil) } else { ms.EXPECT().GetVirtualMachineByID("915653c4-298b-4d74-bdee-4ced282114f1", gomock.Any()).Return(test.mockedCSOutput, 1, nil) } diff --git a/cloudstack/cloudstack_loadbalancer.go b/cloudstack/cloudstack_loadbalancer.go index 69c21528..7f3eb752 100644 --- a/cloudstack/cloudstack_loadbalancer.go +++ b/cloudstack/cloudstack_loadbalancer.go @@ -53,8 +53,11 @@ const ( ServiceAnnotationLoadBalancerAddress = "service.beta.kubernetes.io/cloudstack-load-balancer-address" // Used to construct the load balancer name. - servicePrefix = "K8s_svc_" - lbNameFormat = "%s%s_%s_%s" + servicePrefix = "K8s_svc_" + lbNameFormat = "%s%s_%s_%s" + ProtocolTCP = "tcp" + ProtocolUDP = "udp" + ProtocolTCPProxy = "tcp-proxy" ) type loadBalancer struct { @@ -96,6 +99,8 @@ func (cs *CSCloud) GetLoadBalancer(ctx context.Context, clusterName string, serv } // EnsureLoadBalancer creates a new load balancer, or updates the existing one. Returns the status of the balancer. +// +//nolint:gocognit func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, service *corev1.Service, nodes []*corev1.Node) (status *corev1.LoadBalancerStatus, err error) { klog.V(4).InfoS("EnsureLoadBalancer", "cluster", clusterName, "service", klog.KObj(service)) serviceName := fmt.Sprintf("%s/%s", service.Namespace, service.Name) @@ -146,7 +151,7 @@ func (cs *CSCloud) EnsureLoadBalancer(ctx context.Context, clusterName string, s defer func(lb *loadBalancer) { if err != nil { if err := lb.releaseLoadBalancerIP(); err != nil { - klog.Errorf(err.Error()) + klog.Errorf("error occurred: %s", err.Error()) } } }(lb) @@ -742,9 +747,9 @@ func ruleToString(rule *cloudstack.FirewallRule) string { ls.WriteString("nil") } else { switch rule.Protocol { - case "tcp": + case ProtocolTCP: fallthrough - case "udp": + case ProtocolUDP: fmt.Fprintf(ls, "{[%s] -> %s:[%d-%d] (%s)}", rule.Cidrlist, rule.Ipaddress, rule.Startport, rule.Endport, rule.Protocol) case "icmp": fmt.Fprintf(ls, "{[%s] -> %s [%d,%d] (%s)}", rule.Cidrlist, rule.Ipaddress, rule.Icmptype, rule.Icmpcode, rule.Protocol) @@ -981,7 +986,7 @@ func getBoolFromServiceAnnotation(service *corev1.Service, annotationKey string, return defaultSetting } -// setServiceAnnotation is used to create/set or update an annotation on the Service object +// setServiceAnnotation is used to create/set or update an annotation on the Service object. func setServiceAnnotation(service *corev1.Service, key, value string) { if service.ObjectMeta.Annotations == nil { service.ObjectMeta.Annotations = map[string]string{} diff --git a/cloudstack/protocol.go b/cloudstack/protocol.go index bf168b4f..2541114e 100644 --- a/cloudstack/protocol.go +++ b/cloudstack/protocol.go @@ -45,11 +45,11 @@ func (p LoadBalancerProtocol) String() string { func (p LoadBalancerProtocol) CSProtocol() string { switch p { case LoadBalancerProtocolTCP: - return "tcp" + return ProtocolTCP case LoadBalancerProtocolUDP: - return "udp" + return ProtocolUDP case LoadBalancerProtocolTCPProxy: - return "tcp-proxy" + return ProtocolTCPProxy default: return "" } @@ -62,9 +62,9 @@ func (p LoadBalancerProtocol) IPProtocol() string { case LoadBalancerProtocolTCP: fallthrough case LoadBalancerProtocolTCPProxy: - return "tcp" + return ProtocolTCP case LoadBalancerProtocolUDP: - return "udp" + return ProtocolUDP default: return "" } @@ -101,11 +101,11 @@ func ProtocolFromServicePort(port corev1.ServicePort, service *corev1.Service) L // CloudStack load balancer protocol name. func ProtocolFromLoadBalancer(protocol string) LoadBalancerProtocol { switch protocol { - case "tcp": + case ProtocolTCP: return LoadBalancerProtocolTCP - case "udp": + case ProtocolUDP: return LoadBalancerProtocolUDP - case "tcp-proxy": + case ProtocolTCPProxy: return LoadBalancerProtocolTCPProxy default: return LoadBalancerProtocolInvalid diff --git a/cloudstack/service_patcher.go b/cloudstack/service_patcher.go index e5404dc1..94a54750 100644 --- a/cloudstack/service_patcher.go +++ b/cloudstack/service_patcher.go @@ -55,6 +55,7 @@ func (sp *servicePatcher) Patch(ctx context.Context, err error) error { return err } perr := patchService(ctx, sp.kclient, sp.base, sp.updated) + return utilerrors.NewAggregate([]error{err, perr}) }