From d4968702c25d9f8b5e1fbcf0603c2614791a867d Mon Sep 17 00:00:00 2001 From: Luna Xu <10015938+xuluna@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:26:34 -0600 Subject: [PATCH] Fix linting issue (#185) * reformat * fix errors * fix issues * fix issues * fix issues --- .golangci.yaml | 4 +++ cmd/podmon/main.go | 13 ++++++---- cmd/podmon/main_steps_test.go | 28 ++++++++++---------- internal/criapi/criapi.go | 6 ++--- internal/criapi/criapi_test.go | 3 ++- internal/criapi/mock.go | 4 +-- internal/csiapi/mock.go | 8 +++--- internal/k8sapi/interface.go | 4 +-- internal/k8sapi/mock.go | 26 +++++++++---------- internal/monitor/controller.go | 15 ++++++----- internal/monitor/driver.go | 17 +++++------- internal/monitor/integration_steps_test.go | 27 +++++++++++-------- internal/monitor/integration_test.go | 6 +++-- internal/monitor/monitor.go | 23 +++++++++-------- internal/monitor/monitor_steps_test.go | 25 +++++++++--------- internal/monitor/node.go | 7 ++--- internal/utils/linuxLoopBackDevice.go | 4 +-- internal/utils/mock.go | 6 ++--- test/podmontest/podmontest.go | 16 +++++++----- test/ssh/client.go | 30 +++++++++++++++++++++- test/ssh/client_test.go | 6 +++-- 21 files changed, 164 insertions(+), 114 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 56f53324..5e09bae4 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -4,6 +4,10 @@ run: tests: true skip-dirs-use-default: true modules-download-mode: readonly + skip-dirs: + - go/pkg # remove third party mod lib from scanning + - go/src # remove third party mod lib from scanning + - hostedtoolcache # remove the mod caches from scanning issues: max-issues-per-linter: 0 diff --git a/cmd/podmon/main.go b/cmd/podmon/main.go index 3a7838fa..4f7e7963 100644 --- a/cmd/podmon/main.go +++ b/cmd/podmon/main.go @@ -20,14 +20,15 @@ import ( "context" "flag" "fmt" - "podmon/internal/csiapi" - "podmon/internal/k8sapi" - "podmon/internal/monitor" "strconv" "strings" "sync" "time" + "podmon/internal/csiapi" + "podmon/internal/k8sapi" + "podmon/internal/monitor" + csiext "github.com/dell/dell-csi-extensions/podmon" "github.com/fsnotify/fsnotify" "github.com/kubernetes-csi/csi-lib-utils/leaderelection" @@ -89,8 +90,10 @@ var ArrayConnMonitorFc = monitor.PodMonitor.ArrayConnectivityMonitor var PodMonWait = podMonWait // GetCSIClient is reference to a function that returns a new CSIClient -var GetCSIClient = csiapi.NewCSIClient -var createArgsOnce sync.Once +var ( + GetCSIClient = csiapi.NewCSIClient + createArgsOnce sync.Once +) func main() { log.SetFormatter(&log.TextFormatter{ diff --git a/cmd/podmon/main_steps_test.go b/cmd/podmon/main_steps_test.go index c94e88e4..62bd7bda 100644 --- a/cmd/podmon/main_steps_test.go +++ b/cmd/podmon/main_steps_test.go @@ -20,13 +20,14 @@ import ( "context" "fmt" "os" - "podmon/internal/csiapi" - "podmon/internal/k8sapi" - "podmon/internal/monitor" "strings" "sync" "time" + "podmon/internal/csiapi" + "podmon/internal/k8sapi" + "podmon/internal/monitor" + "github.com/cucumber/godog" "github.com/dell/gofsutil" logtest "github.com/sirupsen/logrus/hooks/test" @@ -44,8 +45,10 @@ type mainFeature struct { failStartAPIMonitor bool } -var saveOriginalArgs sync.Once -var originalArgs []string +var ( + saveOriginalArgs sync.Once + originalArgs []string +) func (m *mainFeature) aPodmonInstance() error { if m.loghook == nil { @@ -84,21 +87,20 @@ func (le *mockLeaderElect) Run() error { return nil } -func (le *mockLeaderElect) WithNamespace(namespace string) { - +func (le *mockLeaderElect) WithNamespace(_ string) { } -func (m *mainFeature) mockGetCSIClient(csiSock string, clientOpts ...grpc.DialOption) (csiapi.CSIApi, error) { +func (m *mainFeature) mockGetCSIClient(_ string, _ ...grpc.DialOption) (csiapi.CSIApi, error) { return m.csiapiMock, nil } -func (m *mainFeature) mockStartPodMonitor(api k8sapi.K8sAPI, client kubernetes.Interface, labelKey, labelValue string, duration time.Duration) { +func (m *mainFeature) mockStartPodMonitor(_ k8sapi.K8sAPI, _ kubernetes.Interface, _, _ string, _ time.Duration) { } -func (m *mainFeature) mockStartNodeMonitor(api k8sapi.K8sAPI, client kubernetes.Interface, labelKey, labelValue string, duration time.Duration) { +func (m *mainFeature) mockStartNodeMonitor(_ k8sapi.K8sAPI, _ kubernetes.Interface, _, _ string, _ time.Duration) { } -func (m *mainFeature) mockStartAPIMonitor(api k8sapi.K8sAPI, first, retry, interval time.Duration, waiter func(interval time.Duration) bool) error { +func (m *mainFeature) mockStartAPIMonitor(_ k8sapi.K8sAPI, _, _, _ time.Duration, _ func(interval time.Duration) bool) error { if m.failStartAPIMonitor { return fmt.Errorf("induced StorageAPIMonitor failure") } @@ -109,7 +111,7 @@ func (m *mainFeature) mockPodMonWait() bool { return true } -func (m *mainFeature) mockLeaderElection(runFunc func(ctx context.Context)) leaderElection { +func (m *mainFeature) mockLeaderElection(_ func(ctx context.Context)) leaderElection { return m.leaderElect } @@ -142,7 +144,7 @@ func (m *mainFeature) theLastLogMessageContains(errormsg string) error { } func (m *mainFeature) csiExtensionsPresentIsFalse(expectedStr string) error { - var expected = strings.ToLower(expectedStr) == "true" + expected := strings.ToLower(expectedStr) == "true" return monitor.AssertExpectedAndActual(assert.Equal, expected, monitor.PodMonitor.CSIExtensionsPresent, fmt.Sprintf("Expected CSIExtensionsPresent flag to be %s, but was %v", expectedStr, monitor.PodMonitor.CSIExtensionsPresent)) diff --git a/internal/criapi/criapi.go b/internal/criapi/criapi.go index 96a6550a..9de47097 100644 --- a/internal/criapi/criapi.go +++ b/internal/criapi/criapi.go @@ -47,7 +47,7 @@ var CRIMaxConnectionRetry = 3 var CRINewClientTimeout = 90 * time.Second // NewCRIClient returns a new client connection to the ContainerRuntimeInterface or an error -func NewCRIClient(criSock string, clientOpts ...grpc.DialOption) (*Client, error) { +func NewCRIClient(criSock string, _ ...grpc.DialOption) (*Client, error) { var err error ctx, cancel := context.WithTimeout(context.Background(), CRINewClientTimeout) defer cancel() @@ -93,7 +93,7 @@ func (cri *Client) ListContainers(ctx context.Context, req *v1alpha2.ListContain return CRIClient.RuntimeServiceClient.ListContainers(ctx, req) } -var knownPaths [3]string = [3]string{"/var/run/dockershim.sock", "/run/containerd/containerd.sock", "/run/crio/crio.sock"} +var knownPaths = [3]string{"/var/run/dockershim.sock", "/run/containerd/containerd.sock", "/run/crio/crio.sock"} // ChooseCRIPath chooses an appropriate unix domain socket path to the CRI interface. // This is done according to the ordering described for the crictl command. @@ -110,7 +110,7 @@ func (cri *Client) ChooseCRIPath() (string, error) { // GetContainerInfo gets current status of all the containers on this server using CRI interface. // The result is a map of ID to a structure containing the ID, Name, and State. -func (cri *Client) GetContainerInfo(ctx context.Context) (map[string]*ContainerInfo, error) { +func (cri *Client) GetContainerInfo(_ context.Context) (map[string]*ContainerInfo, error) { result := make(map[string]*ContainerInfo) path, err := cri.ChooseCRIPath() diff --git a/internal/criapi/criapi_test.go b/internal/criapi/criapi_test.go index f09223fb..dd4020a2 100644 --- a/internal/criapi/criapi_test.go +++ b/internal/criapi/criapi_test.go @@ -22,8 +22,9 @@ package criapi import ( "context" "fmt" - "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "testing" + + "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" ) func TestListContainers(t *testing.T) { diff --git a/internal/criapi/mock.go b/internal/criapi/mock.go index 0d56edb0..a057594b 100644 --- a/internal/criapi/mock.go +++ b/internal/criapi/mock.go @@ -47,7 +47,7 @@ func (mock *MockClient) Close() error { } // ListContainers would list individual containers but is not implemented for the mock client. -func (mock *MockClient) ListContainers(ctx context.Context, req *v1alpha2.ListContainersRequest) (*v1alpha2.ListContainersResponse, error) { +func (mock *MockClient) ListContainers(_ context.Context, _ *v1alpha2.ListContainersRequest) (*v1alpha2.ListContainersResponse, error) { return nil, errors.New("unimplemented") } @@ -58,7 +58,7 @@ func (mock *MockClient) ChooseCRIPath() (string, error) { // GetContainerInfo gets current status of all the containers on this server using CRI interface. // The result is a map of ID to a structure containing the ID, Name, and State. -func (mock *MockClient) GetContainerInfo(ctx context.Context) (map[string]*ContainerInfo, error) { +func (mock *MockClient) GetContainerInfo(_ context.Context) (map[string]*ContainerInfo, error) { if mock.InducedErrors.GetContainerInfo { return mock.MockContainerInfos, errors.New("GetContainerInfo induced error") } diff --git a/internal/csiapi/mock.go b/internal/csiapi/mock.go index 81a660c4..257dabd2 100644 --- a/internal/csiapi/mock.go +++ b/internal/csiapi/mock.go @@ -57,7 +57,7 @@ func (mock *CSIMock) Close() error { } // ControllerUnpublishVolume is a mock implementation of csiapi.CSIApi.ControllerUnpublishVolume -func (mock *CSIMock) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) { +func (mock *CSIMock) ControllerUnpublishVolume(_ context.Context, _ *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) { rep := &csi.ControllerUnpublishVolumeResponse{} if mock.InducedErrors.ControllerUnpublishVolume { return rep, errors.New("ControllerUnpublishedVolume induced error") @@ -66,7 +66,7 @@ func (mock *CSIMock) ControllerUnpublishVolume(ctx context.Context, req *csi.Con } // NodeUnpublishVolume is a mock implementation of csiapi.CSIApi.NodeUnpublishVolume -func (mock *CSIMock) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { +func (mock *CSIMock) NodeUnpublishVolume(_ context.Context, _ *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) { rep := &csi.NodeUnpublishVolumeResponse{} if mock.InducedErrors.NodeUnpublishVolume { return rep, errors.New("NodeUnpublishedVolume induced error") @@ -78,7 +78,7 @@ func (mock *CSIMock) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpub } // NodeUnstageVolume is a mock implementation of csiapi.CSIApi.NodeUnstageVolume -func (mock *CSIMock) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { +func (mock *CSIMock) NodeUnstageVolume(_ context.Context, _ *csi.NodeUnstageVolumeRequest) (*csi.NodeUnstageVolumeResponse, error) { rep := &csi.NodeUnstageVolumeResponse{} if mock.InducedErrors.NodeUnstageVolume { return rep, errors.New("NodeUnstageedVolume induced error") @@ -90,7 +90,7 @@ func (mock *CSIMock) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstage } // ValidateVolumeHostConnectivity is a mock implementation of csiapi.CSIApi.ValidateVolumeHostConnectivity -func (mock *CSIMock) ValidateVolumeHostConnectivity(ctx context.Context, req *csiext.ValidateVolumeHostConnectivityRequest) (*csiext.ValidateVolumeHostConnectivityResponse, error) { +func (mock *CSIMock) ValidateVolumeHostConnectivity(_ context.Context, _ *csiext.ValidateVolumeHostConnectivityRequest) (*csiext.ValidateVolumeHostConnectivityResponse, error) { rep := &csiext.ValidateVolumeHostConnectivityResponse{} if mock.InducedErrors.ValidateVolumeHostConnectivity { return rep, errors.New("ValidateVolumeHostConnectivity induced error") diff --git a/internal/k8sapi/interface.go b/internal/k8sapi/interface.go index fbbb2c54..c635a9b2 100644 --- a/internal/k8sapi/interface.go +++ b/internal/k8sapi/interface.go @@ -100,8 +100,8 @@ type K8sAPI interface { // SetupNodeWatch setups up a node watch. SetupNodeWatch(ctx context.Context, listOptions metav1.ListOptions) (watch.Interface, error) - //TaintNode applies the specified 'taintKey' string and 'effect' to the node with 'nodeName' - //The 'remove' flag indicates if the taint should be removed from the node, if it exists. + // TaintNode applies the specified 'taintKey' string and 'effect' to the node with 'nodeName' + // The 'remove' flag indicates if the taint should be removed from the node, if it exists. TaintNode(ctx context.Context, nodeName, taintKey string, effect v1.TaintEffect, remove bool) error // CreateEvent creates an event on a runtime object. diff --git a/internal/k8sapi/mock.go b/internal/k8sapi/mock.go index d00c01e4..70f60879 100644 --- a/internal/k8sapi/mock.go +++ b/internal/k8sapi/mock.go @@ -115,7 +115,7 @@ func (mock *K8sMock) AddNode(node *v1.Node) { } // Connect connects to the Kubernetes system API -func (mock *K8sMock) Connect(kubeconfig *string) error { +func (mock *K8sMock) Connect(_ *string) error { if mock.InducedErrors.Connect { return errors.New("induced Connect error") } @@ -134,7 +134,7 @@ func (mock *K8sMock) GetContext(duration time.Duration) (context.Context, contex } // DeletePod deletes a pod of the given namespace and name, an optionally uses force deletion. -func (mock *K8sMock) DeletePod(ctx context.Context, namespace, name string, podUID types.UID, force bool) error { +func (mock *K8sMock) DeletePod(_ context.Context, namespace, name string, _ types.UID, _ bool) error { if mock.InducedErrors.DeletePod { return errors.New("induced DeletePod error") } @@ -144,7 +144,7 @@ func (mock *K8sMock) DeletePod(ctx context.Context, namespace, name string, podU } // GetPod retrieves a pod of the give namespace and name -func (mock *K8sMock) GetPod(ctx context.Context, namespace, name string) (*v1.Pod, error) { +func (mock *K8sMock) GetPod(_ context.Context, namespace, name string) (*v1.Pod, error) { var pod *v1.Pod if mock.InducedErrors.GetPod { return pod, errors.New("induced GetPod error") @@ -160,7 +160,7 @@ func (mock *K8sMock) GetPod(ctx context.Context, namespace, name string) (*v1.Po // GetCachedVolumeAttachment will try to load the volumeattachment select by the persistent volume name and node name. // If found it is returned from the cache. If not found, the cache is reloaded and the result returned from the reloaded data. -func (mock *K8sMock) GetCachedVolumeAttachment(ctx context.Context, pvName, nodeName string) (*storagev1.VolumeAttachment, error) { +func (mock *K8sMock) GetCachedVolumeAttachment(_ context.Context, pvName, nodeName string) (*storagev1.VolumeAttachment, error) { valist := &storagev1.VolumeAttachmentList{} if mock.InducedErrors.GetVolumeAttachments { return nil, errors.New("induced GetVolumeAttachments error") @@ -175,7 +175,7 @@ func (mock *K8sMock) GetCachedVolumeAttachment(ctx context.Context, pvName, node } // GetVolumeAttachments gets all the volume attachments in the K8S system -func (mock *K8sMock) GetVolumeAttachments(ctx context.Context) (*storagev1.VolumeAttachmentList, error) { +func (mock *K8sMock) GetVolumeAttachments(_ context.Context) (*storagev1.VolumeAttachmentList, error) { valist := &storagev1.VolumeAttachmentList{} if mock.InducedErrors.GetVolumeAttachments { return valist, errors.New("induced GetVolumeAttachments error") @@ -188,7 +188,7 @@ func (mock *K8sMock) GetVolumeAttachments(ctx context.Context) (*storagev1.Volum } // DeleteVolumeAttachment deletes a volume attachment by name. -func (mock *K8sMock) DeleteVolumeAttachment(ctx context.Context, va string) error { +func (mock *K8sMock) DeleteVolumeAttachment(_ context.Context, va string) error { if mock.InducedErrors.DeleteVolumeAttachment { return errors.New("induced DeleteVolumeAttachment error") } @@ -197,7 +197,7 @@ func (mock *K8sMock) DeleteVolumeAttachment(ctx context.Context, va string) erro } // GetPersistentVolumeClaimsInNamespace returns all the pvcs in a namespace. -func (mock *K8sMock) GetPersistentVolumeClaimsInNamespace(ctx context.Context, namespace string) (*v1.PersistentVolumeClaimList, error) { +func (mock *K8sMock) GetPersistentVolumeClaimsInNamespace(_ context.Context, _ string) (*v1.PersistentVolumeClaimList, error) { pvclist := &v1.PersistentVolumeClaimList{} if mock.InducedErrors.GetPersistentVolumeClaimsInNamespace { return pvclist, errors.New("induced GetPersistentVolumeClaimsInNamespace error") @@ -299,7 +299,7 @@ func (mock *K8sMock) GetPersistentVolumeClaimName(ctx context.Context, pvName st } // GetPersistentVolume retrieves a persistent volume given the pv name. -func (mock *K8sMock) GetPersistentVolume(ctx context.Context, pvName string) (*v1.PersistentVolume, error) { +func (mock *K8sMock) GetPersistentVolume(_ context.Context, pvName string) (*v1.PersistentVolume, error) { var pv *v1.PersistentVolume if mock.InducedErrors.GetPersistentVolume { return pv, errors.New("induced GetPersistentVolume error") @@ -309,7 +309,7 @@ func (mock *K8sMock) GetPersistentVolume(ctx context.Context, pvName string) (*v } // GetPersistentVolumeClaim returns the PVC of the given namespace/pvcName. -func (mock *K8sMock) GetPersistentVolumeClaim(ctx context.Context, namespace, pvcName string) (*v1.PersistentVolumeClaim, error) { +func (mock *K8sMock) GetPersistentVolumeClaim(_ context.Context, namespace, pvcName string) (*v1.PersistentVolumeClaim, error) { var pvc *v1.PersistentVolumeClaim if mock.InducedErrors.GetPersistentVolumeClaim { return pvc, errors.New("induced GetPersistentVolumeClaim error") @@ -320,7 +320,7 @@ func (mock *K8sMock) GetPersistentVolumeClaim(ctx context.Context, namespace, pv } // GetNode returns the node with the specified nodeName. -func (mock *K8sMock) GetNode(ctx context.Context, nodeName string) (*v1.Node, error) { +func (mock *K8sMock) GetNode(_ context.Context, nodeName string) (*v1.Node, error) { var node *v1.Node if mock.InducedErrors.GetNode { return node, errors.New("induced GetNode error") @@ -393,7 +393,7 @@ func (mock *K8sMock) getKey(namespace, name string) string { } // SetupPodWatch returns a mock watcher -func (mock *K8sMock) SetupPodWatch(ctx context.Context, namespace string, listOptions metav1.ListOptions) (watch.Interface, error) { +func (mock *K8sMock) SetupPodWatch(_ context.Context, _ string, _ metav1.ListOptions) (watch.Interface, error) { if mock.InducedErrors.Watch { return nil, errors.New("included Watch error") } @@ -401,7 +401,7 @@ func (mock *K8sMock) SetupPodWatch(ctx context.Context, namespace string, listOp } // SetupNodeWatch returns a mock watcher -func (mock *K8sMock) SetupNodeWatch(ctx context.Context, listOptions metav1.ListOptions) (watch.Interface, error) { +func (mock *K8sMock) SetupNodeWatch(_ context.Context, _ metav1.ListOptions) (watch.Interface, error) { if mock.InducedErrors.Watch { return nil, errors.New("included Watch error") } @@ -437,7 +437,7 @@ func (mock *K8sMock) TaintNode(ctx context.Context, nodeName, taintKey string, e } // CreateEvent creates an event for the specified object. -func (mock *K8sMock) CreateEvent(sourceComponent string, object runtime.Object, eventType, reason, messageFmt string, args ...interface{}) error { +func (mock *K8sMock) CreateEvent(_ string, _ runtime.Object, _, _, _ string, _ ...interface{}) error { if mock.InducedErrors.CreateEvent { return errors.New("induced CreateEvent error") } diff --git a/internal/monitor/controller.go b/internal/monitor/controller.go index e85a3c27..0f27de79 100644 --- a/internal/monitor/controller.go +++ b/internal/monitor/controller.go @@ -48,11 +48,13 @@ type ControllerPodInfo struct { // information controller keeps on hand about a PodAffinityLabels map[string]string // A list of pod affinity labels for the pod } -const notFound = "not found" -const hostNameTopologyKey = "kubernetes.io/hostname" -const arrayIDVolumeAttribute = "arrayID" -const storageSystemVolumeAttribute = "StorageSystem" -const defaultArray = "default" +const ( + notFound = "not found" + hostNameTopologyKey = "kubernetes.io/hostname" + arrayIDVolumeAttribute = "arrayID" + storageSystemVolumeAttribute = "StorageSystem" + defaultArray = "default" +) // controllerModePodHandler handles controller mode functionality when a pod event happens func (cm *PodMonitorType) controllerModePodHandler(pod *v1.Pod, eventType watch.EventType) error { @@ -408,7 +410,6 @@ func (cm *PodMonitorType) podToArrayIDs(ctx context.Context, pod *v1.Pod) ([]str // If connectivity is lost, will initiate cleanup of the pods. // This is a never ending function, intended to be called as Go routine. func (cm *PodMonitorType) ArrayConnectivityMonitor() { - // Loop through all the monitored Pods making sure they still have array access for { podKeysToClean := make([]string, 0) @@ -703,7 +704,7 @@ func (cm *PodMonitorType) controllerModeDriverPodHandler(pod *v1.Pod, eventType } else { hasTaint := nodeHasTaint(node, PodmonDriverPodTaintKey, v1.TaintEffectNoSchedule) log.Infof("Removing taint from node %s with %s", node.ObjectMeta.Name, PodmonDriverPodTaintKey) - //remove taint + // remove taint if hasTaint { err := taintNode(node.ObjectMeta.Name, PodmonDriverPodTaintKey, true) if err != nil { diff --git a/internal/monitor/driver.go b/internal/monitor/driver.go index c6937b9d..8489c337 100644 --- a/internal/monitor/driver.go +++ b/internal/monitor/driver.go @@ -20,9 +20,10 @@ import ( "crypto/sha256" "fmt" "os" - "podmon/internal/utils" "strings" + "podmon/internal/utils" + log "github.com/sirupsen/logrus" ) @@ -42,8 +43,7 @@ type drivertype interface { var Driver drivertype // VxflexDriver provides a Driver instance for the PowerFlex (VxFlex) architecture. -type VxflexDriver struct { -} +type VxflexDriver struct{} // GetDriverName returns the driver name string func (d *VxflexDriver) GetDriverName() string { @@ -116,8 +116,7 @@ func (d *VxflexDriver) FinalCleanup(rawBlock bool, volumeHandle, pvName, podUUID } // UnityDriver provides a Driver instance for the Unity architecture. -type UnityDriver struct { -} +type UnityDriver struct{} // GetDriverName returns the driver name string func (d *UnityDriver) GetDriverName() string { @@ -182,7 +181,7 @@ func (d *UnityDriver) NodeUnstageExcludedError(err error) bool { // FinalCleanup handles any driver specific final cleanup. func (d *UnityDriver) FinalCleanup(rawBlock bool, volumeHandle, pvName, podUUID string) error { - if rawBlock { //Do this cleanup on raw device block + if rawBlock { // Do this cleanup on raw device block loopBackDev, err := getLoopBackDevice(pvName) if err != nil || loopBackDev == "" { // nothing to clean @@ -212,8 +211,7 @@ var ( ) // PScaleDriver provides a Driver instance for the PowerScale architecture. -type PScaleDriver struct { -} +type PScaleDriver struct{} // GetDriverName returns the driver name string func (d *PScaleDriver) GetDriverName() string { @@ -267,8 +265,7 @@ func (d *PScaleDriver) FinalCleanup(rawBlock bool, volumeHandle, pvName, podUUID } // PStoreDriver provides a Driver instance for the Powerstore architecture. -type PStoreDriver struct { -} +type PStoreDriver struct{} // GetDriverName returns the driver name string func (d *PStoreDriver) GetDriverName() string { diff --git a/internal/monitor/integration_steps_test.go b/internal/monitor/integration_steps_test.go index 2edefbe8..0b9a7c97 100644 --- a/internal/monitor/integration_steps_test.go +++ b/internal/monitor/integration_steps_test.go @@ -23,13 +23,14 @@ import ( "os" "os/exec" "path/filepath" - "podmon/internal/k8sapi" - "podmon/test/ssh" "strconv" "strings" "sync" "time" + "podmon/internal/k8sapi" + "podmon/test/ssh" + "github.com/cucumber/godog" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -108,18 +109,24 @@ const ( var stopTestRequested bool // Workaround for non-inclusive word scan -var primary = []byte{'m', 'a', 's', 't', 'e', 'r'} -var primaryLabelKey = fmt.Sprintf("node-role.kubernetes.io/%s", string(primary)) -var controlPlane = "node-role.kubernetes.io/control-plane" +var ( + primary = []byte{'m', 'a', 's', 't', 'e', 'r'} + primaryLabelKey = fmt.Sprintf("node-role.kubernetes.io/%s", string(primary)) + controlPlane = "node-role.kubernetes.io/control-plane" +) // These are for tracking to which nodes the tests upload scripts. // With multiple scenarios, we want to do this only once. -var nodesWithScripts map[string]bool -var nodesWithScriptsInitOnce sync.Once +var ( + nodesWithScripts map[string]bool + nodesWithScriptsInitOnce sync.Once +) // Parameters for use with the background poller -var k8sPollInterval = 2 * time.Second -var pollTick *time.Ticker +var ( + k8sPollInterval = 2 * time.Second + pollTick *time.Ticker +) // sshOptions used in SSH cli commands to K8s nodes var sshOptions = fmt.Sprintf("-o 'ConnectTimeout=%d' -o 'UserKnownHostsFile /dev/null' -o 'StrictHostKeyChecking no'", sshTimeoutValue) @@ -1121,7 +1128,7 @@ func (i *integration) copyOverTestScriptsToNode(address string) error { // script file, we will use the 'scp' command from the Bastion node to copy over the files from // there to the Openshift node. func (i *integration) copyOverTestScriptsToOpenshift(address string) error { - //SSH and SCP are all done through the Bastion node + // SSH and SCP are all done through the Bastion node info := ssh.AccessInfo{ Hostname: i.bastionNode, Port: "22", diff --git a/internal/monitor/integration_test.go b/internal/monitor/integration_test.go index 02fd0e6d..e90ab58a 100644 --- a/internal/monitor/integration_test.go +++ b/internal/monitor/integration_test.go @@ -25,8 +25,10 @@ import ( log "github.com/sirupsen/logrus" ) -const enableIntTestVar = "RESILIENCY_INT_TEST" -const enableStopOnFailure = "RESILIENCY_INT_TEST_STOP_ON_FAILURE" +const ( + enableIntTestVar = "RESILIENCY_INT_TEST" + enableStopOnFailure = "RESILIENCY_INT_TEST_STOP_ON_FAILURE" +) var setupIsGood = false diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go index 8ab29120..55517fa2 100644 --- a/internal/monitor/monitor.go +++ b/internal/monitor/monitor.go @@ -19,12 +19,13 @@ package monitor import ( "context" "fmt" - "podmon/internal/csiapi" - "podmon/internal/k8sapi" "strings" "sync" "time" + "podmon/internal/csiapi" + "podmon/internal/k8sapi" + log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -50,27 +51,27 @@ var ( PodmonTaintKey = "" // PodmonDriverPodTaintKey is the key for this driver's node pod taint. PodmonDriverPodTaintKey = "" - //ShortTimeout used for initial try + // ShortTimeout used for initial try ShortTimeout = 10 * time.Second - //MediumTimeout is a wait-backoff after the ShortTimeout + // MediumTimeout is a wait-backoff after the ShortTimeout MediumTimeout = 30 * time.Second - //LongTimeout is a longer wait-backoff period + // LongTimeout is a longer wait-backoff period LongTimeout = 180 * time.Second - //PendingRetryTime time between retry of certain CSI calls + // PendingRetryTime time between retry of certain CSI calls PendingRetryTime = 30 * time.Second - //NodeAPIInterval time between NodeAPI checks + // NodeAPIInterval time between NodeAPI checks NodeAPIInterval = 30 * time.Second - //CSIMaxRetries max times to retry certain CSI calls + // CSIMaxRetries max times to retry certain CSI calls CSIMaxRetries = 3 - //MonitorRestartTimeDelay time to wait before restarting monitor + // MonitorRestartTimeDelay time to wait before restarting monitor MonitorRestartTimeDelay = 10 * time.Second - //LockSleepTimeDelay wait for lock retry + // LockSleepTimeDelay wait for lock retry LockSleepTimeDelay = 1 * time.Second // dynamicConfigUpdateMutex protects concurrently running threads that could be affected by dynamic configuration parameters. dynamicConfigUpdateMutex sync.Mutex // arrayConnectivityPollRate is the rate it polls to check array connectivity to nodes. arrayConnectivityPollRate = ShortTimeout - //IgnoreVolumelessPods when set will keep labeled pods with no volumes from being force deleted on node or connectivity failures. + // IgnoreVolumelessPods when set will keep labeled pods with no volumes from being force deleted on node or connectivity failures. IgnoreVolumelessPods bool ) diff --git a/internal/monitor/monitor_steps_test.go b/internal/monitor/monitor_steps_test.go index 284507ee..1d3197be 100644 --- a/internal/monitor/monitor_steps_test.go +++ b/internal/monitor/monitor_steps_test.go @@ -21,15 +21,16 @@ import ( "fmt" "os" "path/filepath" - "podmon/internal/criapi" - "podmon/internal/csiapi" - "podmon/internal/k8sapi" - "podmon/internal/utils" "strconv" "strings" "sync" "time" + "podmon/internal/criapi" + "podmon/internal/csiapi" + "podmon/internal/k8sapi" + "podmon/internal/utils" + "github.com/cucumber/godog" "github.com/dell/gofsutil" log "github.com/sirupsen/logrus" @@ -196,12 +197,12 @@ func (f *feature) iHaveAPodsForNodeWithVolumesDevicesCondition(nPods int, nodeNa mockCSIVolumePath := fmt.Sprintf(CSIVolumePathFormat, pod.UID) mockCSIDevicePath := fmt.Sprintf(CSIDevicePathFormat, pod.UID) - err = os.Mkdir(mockCSIVolumePath, 0700) + err = os.Mkdir(mockCSIVolumePath, 0o700) if err != nil { return err } - err = os.MkdirAll(mockCSIDevicePath, 0700) + err = os.MkdirAll(mockCSIDevicePath, 0o700) if err != nil { err = fmt.Errorf("Mkdir mockCSIDevicePath failed: %s", err) return err @@ -209,13 +210,13 @@ func (f *feature) iHaveAPodsForNodeWithVolumesDevicesCondition(nPods int, nodeNa mockPaths = append(mockPaths, mockCSIVolumePath) for _, pvName := range f.pvNames { - if err = os.Mkdir(filepath.Join(mockCSIVolumePath, pvName), 0700); err != nil { + if err = os.Mkdir(filepath.Join(mockCSIVolumePath, pvName), 0o700); err != nil { return err } } mockPaths = append(mockPaths, mockCSIDevicePath) for _, pvName := range f.pvNames { - if _, err = utils.Creat(filepath.Join(mockCSIDevicePath, pvName), 060700); err != nil { + if _, err = utils.Creat(filepath.Join(mockCSIDevicePath, pvName), 0o60700); err != nil { err = fmt.Errorf("Create mockCSIDevicePath failed: %s", err) return err } @@ -480,12 +481,12 @@ func (f *feature) iCallNodeModePodHandlerForNodeWithEvent(nodeName, eventType st mockCSIVolumePath := fmt.Sprintf(CSIVolumePathFormat, f.pod.UID) if !f.failCSIVolumePathDirRead { - if err = os.Mkdir(mockCSIVolumePath, 0700); err != nil { + if err = os.Mkdir(mockCSIVolumePath, 0o700); err != nil { return err } defer os.RemoveAll(mockCSIVolumePath) for _, pvName := range f.pvNames { - if err = os.Mkdir(filepath.Join(mockCSIVolumePath, pvName), 0700); err != nil { + if err = os.Mkdir(filepath.Join(mockCSIVolumePath, pvName), 0o700); err != nil { return err } } @@ -1098,7 +1099,7 @@ func (f *feature) aDriverPodForNodeWithCondition(node, condition string) error { func (f *feature) iCallControllerModeDriverPodHandlerWithEvent(event string) error { var eventType watch.EventType - PodmonDriverPodTaintKey = vxflexDriverPodTaint //assigned vxflex driver as default + PodmonDriverPodTaintKey = vxflexDriverPodTaint // assigned vxflex driver as default switch event { case "Added": eventType = watch.Added @@ -1118,7 +1119,7 @@ func (f *feature) iCallControllerModeDriverPodHandlerWithEvent(event string) err func (f *feature) theNodeIsTainted(nodename, boolean string) error { node, _ := f.k8sapiMock.GetNode(context.Background(), nodename) - //node.Spec.Taints + // node.Spec.Taints switch boolean { case "true": // check for taint diff --git a/internal/monitor/node.go b/internal/monitor/node.go index e86a03dd..e63237c8 100644 --- a/internal/monitor/node.go +++ b/internal/monitor/node.go @@ -21,11 +21,12 @@ import ( "errors" "fmt" "os" + "strings" + "time" + "podmon/internal/criapi" "podmon/internal/k8sapi" "podmon/internal/utils" - "strings" - "time" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/dell/gofsutil" @@ -141,7 +142,7 @@ func (pm *PodMonitorType) nodeModePodHandler(pod *v1.Pod, eventType watch.EventT nodeName := os.Getenv("KUBE_NODE_NAME") driverNamespace := os.Getenv("MY_POD_NAMESPACE") if driverNamespace == pod.ObjectMeta.Namespace { - //driver pod, no need to protect at node + // driver pod, no need to protect at node return nil } diff --git a/internal/utils/linuxLoopBackDevice.go b/internal/utils/linuxLoopBackDevice.go index 0213c7cf..7c21ef7e 100644 --- a/internal/utils/linuxLoopBackDevice.go +++ b/internal/utils/linuxLoopBackDevice.go @@ -27,9 +27,7 @@ import ( log "github.com/sirupsen/logrus" ) -var ( - execCommand = exec.Command -) +var execCommand = exec.Command // GetLoopBackDevice get the loopbackdevice for given pv func GetLoopBackDevice(pvname string) (string, error) { diff --git a/internal/utils/mock.go b/internal/utils/mock.go index 48365600..a49f45cb 100644 --- a/internal/utils/mock.go +++ b/internal/utils/mock.go @@ -39,7 +39,7 @@ func (mock *Mock) GetLoopBackDevice(pv string) (string, error) { } // DeleteLoopBackDevice deletes a loopbackdevice. -func (mock *Mock) DeleteLoopBackDevice(device string) ([]byte, error) { +func (mock *Mock) DeleteLoopBackDevice(_ string) ([]byte, error) { delSucc := []byte("loopbackdevice") if mock.InducedErrors.DeleteLoopBackDevice { return nil, errors.New("induced DeleteLoopBackDevice error") @@ -48,7 +48,7 @@ func (mock *Mock) DeleteLoopBackDevice(device string) ([]byte, error) { } // Unmount is a wrapper around syscall.Unmount -func (mock *Mock) Unmount(devName string, flags int) error { +func (mock *Mock) Unmount(_ string, _ int) error { if mock.InducedErrors.Unmount { return errors.New("induced Unmount error") } @@ -56,7 +56,7 @@ func (mock *Mock) Unmount(devName string, flags int) error { } // Creat is a wrapper around syscall.Creat -func (mock *Mock) Creat(filepath string, flags int) (int, error) { +func (mock *Mock) Creat(_ string, _ int) (int, error) { if mock.InducedErrors.Creat { return 1, errors.New("induced Creat error") } diff --git a/test/podmontest/podmontest.go b/test/podmontest/podmontest.go index 306a4d23..164211b0 100644 --- a/test/podmontest/podmontest.go +++ b/test/podmontest/podmontest.go @@ -32,9 +32,11 @@ const TAGSIZE = 16 // InitialPod is the prefix for the initial-pod tag line const InitialPod = "initial-pod" -var rootDir = "/" -var enableDoExit bool -var blockFiles map[string]*os.File +var ( + rootDir = "/" + enableDoExit bool + blockFiles map[string]*os.File +) func main() { var err error @@ -72,7 +74,7 @@ func readExistingEntries(rootDir string) bool { } for _, entry := range entries { if strings.HasPrefix(entry.Name(), "data") { - f, err := os.OpenFile(rootDir+"/"+entry.Name()+"/log", os.O_RDONLY, 0644) + f, err := os.OpenFile(rootDir+"/"+entry.Name()+"/log", os.O_RDONLY, 0o644) if err != nil { fmt.Printf("Couldn't open %s %s\n", entry.Name(), err.Error()) continue @@ -81,7 +83,7 @@ func readExistingEntries(rootDir string) bool { scanner := bufio.NewScanner(f) for scanner.Scan() { line := scanner.Text() - //fmt.Printf("line: %s\n", line) + // fmt.Printf("line: %s\n", line) if line == "" { key = "" computeTimeDelta = true @@ -144,7 +146,7 @@ func makeEntry(podTag, rootDir string, index int, initialPod bool) { doExit := false for _, entry := range entries { if strings.HasPrefix(entry.Name(), "data") { - f, err := os.OpenFile(rootDir+"/"+entry.Name()+"/log", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + f, err := os.OpenFile(rootDir+"/"+entry.Name()+"/log", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) if err != nil { fmt.Printf("Couldn't open %s %s\n", entry.Name(), err.Error()) doExit = true @@ -178,7 +180,7 @@ func makeEntry(podTag, rootDir string, index int, initialPod bool) { if strings.HasPrefix(entry.Name(), "blockdata") { var f *os.File if index == 0 { - f, err = os.OpenFile(rootDir+"/"+entry.Name(), os.O_WRONLY, 0644) + f, err = os.OpenFile(rootDir+"/"+entry.Name(), os.O_WRONLY, 0o644) if err != nil { fmt.Printf("Couldn't open %s %s\n", entry.Name(), err.Error()) } diff --git a/test/ssh/client.go b/test/ssh/client.go index 2e427e4e..fb959a8a 100644 --- a/test/ssh/client.go +++ b/test/ssh/client.go @@ -17,7 +17,10 @@ package ssh import ( + "encoding/base64" "fmt" + "log" + "net" "os" "strings" "time" @@ -104,12 +107,37 @@ func NewWrapper(accessInfo *AccessInfo) *Wrapper { ssh.Password(accessInfo.Password), }, // Non-production only - HostKeyCallback: ssh.InsecureIgnoreHostKey(), + // the original code is blocked by golint. this method currently set the key to empty for testing + // a warning message will be displayed with the currect key + HostKeyCallback: trustedHostKeyCallback(""), } wrapper := &Wrapper{SSHConfig: config} return wrapper } +// create human-readable SSH-key strings +func keyString(k ssh.PublicKey) string { + return k.Type() + " " + base64.StdEncoding.EncodeToString(k.Marshal()) +} + +func trustedHostKeyCallback(trustedKey string) ssh.HostKeyCallback { + if trustedKey == "" { + return func(_ string, _ net.Addr, k ssh.PublicKey) error { + log.Printf("WARNING: SSH-key verification is *NOT* in effect: to fix, add this trustedKey: %q", keyString(k)) + return nil + } + } + + return func(_ string, _ net.Addr, k ssh.PublicKey) error { + ks := keyString(k) + if trustedKey != ks { + return fmt.Errorf("SSH-key verification: expected %q but got %q", trustedKey, ks) + } + + return nil + } +} + // GetSession makes underlying call to crypto ssh library to create an SSH session func (w *Wrapper) GetSession(hostAndPort string) (SessionWrapper, error) { client, err := ssh.Dial("tcp", hostAndPort, w.SSHConfig) diff --git a/test/ssh/client_test.go b/test/ssh/client_test.go index d28fd80f..5493f8e7 100644 --- a/test/ssh/client_test.go +++ b/test/ssh/client_test.go @@ -41,7 +41,8 @@ func TestCommandExecution_Run(t *testing.T) { verifyThisOutput := func(t *testing.T, expectedOutput []string, actualOutput []string, expectedErrors []string, actualErrors []string, - expectedError error, actualError error) { + expectedError error, actualError error, + ) { if len(expectedOutput) > 0 { if len(expectedOutput) != len(actualOutput) { t.Fatalf("expected output '%s', but received '%s'", @@ -74,7 +75,8 @@ func TestCommandExecution_Run(t *testing.T) { verifyReturnedErrorSimilar := func(t *testing.T, expectedOutput []string, actualOutput []string, expectedErrors []string, actualErrors []string, - expectedError error, actualError error) { + expectedError error, actualError error, + ) { if expectedError != nil && !strings.Contains(actualError.Error(), expectedError.Error()) { t.Fatalf("expected error was '%s', but actual error was '%s'", expectedError.Error(), actualError.Error())