Skip to content

Commit

Permalink
Add golint -> Need to continue
Browse files Browse the repository at this point in the history
  • Loading branch information
Farnaz Babaeian committed Jan 30, 2025
1 parent 08e0687 commit 54079d2
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 28 deletions.
50 changes: 50 additions & 0 deletions .github/workflows/pr-check.yaml
Original file line number Diff line number Diff line change
@@ -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
47 changes: 47 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -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
8 changes: 5 additions & 3 deletions cloudstack/cloudstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cloudstack/cloudstack_instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 12 additions & 10 deletions cloudstack/cloudstack_instances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Check failure on line 11 in cloudstack/cloudstack_instances_test.go

View workflow job for this annotation

GitHub Actions / build

no required module provides package go.uber.org/mock/gomock; to add it:
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",
Expand Down Expand Up @@ -101,7 +103,7 @@ func TestInstanceExists(t *testing.T) {
client: cs,
}

nodeName := "testDummyVM"
nodeName := testVM

tests := []struct {
name string
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -171,7 +173,7 @@ func TestInstanceShutdown(t *testing.T) {
client: cs,
}

nodeName := "testDummyVM"
nodeName := testVM

tests := []struct {
name string
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -248,7 +250,7 @@ func TestInstanceMetadata(t *testing.T) {
client: cs,
}

nodeName := "testDummyVM"
nodeName := testVM

tests := []struct {
name string
Expand Down Expand Up @@ -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)
}
Expand Down
17 changes: 11 additions & 6 deletions cloudstack/cloudstack_loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{}
Expand Down
16 changes: 8 additions & 8 deletions cloudstack/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
}
Expand All @@ -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 ""
}
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cloudstack/service_patcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}

Expand Down

0 comments on commit 54079d2

Please sign in to comment.