Skip to content

Commit

Permalink
Merge pull request #2204 from AndrewSirenko/lint
Browse files Browse the repository at this point in the history 
Enable recommended golang-ci linters
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Nov 4, 2024
2 parents abf285a + c0949b3 commit 7f8e841
Show file tree
Hide file tree
Showing 70 changed files with 855 additions and 1,122 deletions.
414 changes: 51 additions & 363 deletions .golangci.yml
View file

Large diffs are not rendered by default.

24 changes: 16 additions & 8 deletions cmd/hooks/prestop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package hooks

import (
"context"
"errors"
"fmt"
"os"

Expand Down Expand Up @@ -46,7 +47,7 @@ const clusterAutoscalerTaint = "ToBeDeletedByClusterAutoscaler"
const v1KarpenterTaint = "karpenter.sh/disrupted"
const v1beta1KarpenterTaint = "karpenter.sh/disruption"

// drainTaints includes taints used by K8s or autoscalers that signify node draining or pod eviction
// drainTaints includes taints used by K8s or autoscalers that signify node draining or pod eviction.
var drainTaints = map[string]struct{}{
v1.TaintNodeUnschedulable: {}, // Kubernetes common eviction taint (kubectl drain)
clusterAutoscalerTaint: {},
Expand All @@ -59,18 +60,19 @@ func PreStop(clientset kubernetes.Interface) error {

nodeName := os.Getenv("CSI_NODE_NAME")
if nodeName == "" {
return fmt.Errorf("PreStop: CSI_NODE_NAME missing")
return errors.New("PreStop: CSI_NODE_NAME missing")
}

node, err := fetchNode(clientset, nodeName)
if k8serrors.IsNotFound(err) {
switch {
case k8serrors.IsNotFound(err):
klog.InfoS("PreStop: node does not exist - assuming this is a termination event, checking for remaining VolumeAttachments", "node", nodeName)
} else if err != nil {
case err != nil:
return err
} else if !isNodeBeingDrained(node) {
case !isNodeBeingDrained(node):
klog.InfoS("PreStop: node is not being drained, skipping VolumeAttachments check", "node", nodeName)
return nil
} else {
default:
klog.InfoS("PreStop: node is being drained, checking for remaining VolumeAttachments", "node", nodeName)
}

Expand Down Expand Up @@ -104,7 +106,10 @@ func waitForVolumeAttachments(clientset kubernetes.Interface, nodeName string) e
_, err := informer.AddEventHandler(cache.ResourceEventHandlerFuncs{
DeleteFunc: func(obj interface{}) {
klog.V(5).InfoS("DeleteFunc: VolumeAttachment deleted", "node", nodeName)
va := obj.(*storagev1.VolumeAttachment)
va, ok := obj.(*storagev1.VolumeAttachment)
if !ok {
klog.Error("DeleteFunc: error asserting object as type VolumeAttachment", "obj", va)
}
if va.Spec.NodeName == nodeName {
if err := checkVolumeAttachments(clientset, nodeName, allAttachmentsDeleted); err != nil {
klog.ErrorS(err, "DeleteFunc: error checking VolumeAttachments")
Expand All @@ -113,7 +118,10 @@ func waitForVolumeAttachments(clientset kubernetes.Interface, nodeName string) e
},
UpdateFunc: func(oldObj, newObj interface{}) {
klog.V(5).InfoS("UpdateFunc: VolumeAttachment updated", "node", nodeName)
va := newObj.(*storagev1.VolumeAttachment)
va, ok := newObj.(*storagev1.VolumeAttachment)
if !ok {
klog.Error("UpdateFunc: error asserting object as type VolumeAttachment", "obj", va)
}
if va.Spec.NodeName == nodeName {
if err := checkVolumeAttachments(clientset, nodeName, allAttachmentsDeleted); err != nil {
klog.ErrorS(err, "UpdateFunc: error checking VolumeAttachments")
Expand Down
14 changes: 4 additions & 10 deletions cmd/hooks/prestop_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package hooks

import (
"fmt"
"errors"
"testing"

"github.com/golang/mock/gomock"
Expand All @@ -37,19 +37,19 @@ func TestPreStopHook(t *testing.T) {
{
name: "TestPreStopHook: CSI_NODE_NAME not set",
nodeName: "",
expErr: fmt.Errorf("PreStop: CSI_NODE_NAME missing"),
expErr: errors.New("PreStop: CSI_NODE_NAME missing"),
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockStorageV1 *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {
return nil
},
},
{
name: "TestPreStopHook: failed to retrieve node information",
nodeName: "test-node",
expErr: fmt.Errorf("fetchNode: failed to retrieve node information: non-existent node"),
expErr: errors.New("fetchNode: failed to retrieve node information: non-existent node"),
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockStorageV1 *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {
mockClient.EXPECT().CoreV1().Return(mockCoreV1).Times(1)
mockCoreV1.EXPECT().Nodes().Return(mockNode).Times(1)
mockNode.EXPECT().Get(gomock.Any(), gomock.Eq(nodeName), gomock.Any()).Return(nil, fmt.Errorf("non-existent node")).Times(1)
mockNode.EXPECT().Get(gomock.Any(), gomock.Eq(nodeName), gomock.Any()).Return(nil, errors.New("non-existent node")).Times(1)

return nil
},
Expand Down Expand Up @@ -77,7 +77,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down Expand Up @@ -109,7 +108,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down Expand Up @@ -149,7 +147,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down Expand Up @@ -206,7 +203,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-karpenter-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down Expand Up @@ -238,7 +234,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-karpenter-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down Expand Up @@ -278,7 +273,6 @@ func TestPreStopHook(t *testing.T) {
nodeName: "test-karpenter-node",
expErr: nil,
mockFunc: func(nodeName string, mockClient *driver.MockKubernetesClient, mockCoreV1 *driver.MockCoreV1Interface, mockNode *driver.MockNodeInterface, mockVolumeAttachments *driver.MockVolumeAttachmentInterface, mockStorageV1Interface *driver.MockStorageV1Interface) error {

fakeNode := &v1.Node{
Spec: v1.NodeSpec{
Taints: []v1.Taint{
Expand Down
5 changes: 3 additions & 2 deletions cmd/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func main() {
klog.ErrorS(err, "failed to get version")
klog.FlushAndExit(klog.ExitFlushTimeout, 1)
}
//nolint:forbidigo // Print version info without klog/timestamp
fmt.Println(versionInfo)
os.Exit(0)
}
Expand All @@ -133,9 +134,9 @@ func main() {
}()
}

if options.HttpEndpoint != "" {
if options.HTTPEndpoint != "" {
r := metrics.InitializeRecorder()
r.InitializeMetricsHandler(options.HttpEndpoint, "/metrics", options.MetricsCertFile, options.MetricsKeyFile)
r.InitializeMetricsHandler(options.HTTPEndpoint, "/metrics", options.MetricsCertFile, options.MetricsKeyFile)
}

cfg := metadata.MetadataServiceConfig{
Expand Down
13 changes: 7 additions & 6 deletions pkg/batcher/batcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package batcher

import (
"errors"
"fmt"
"sync"
"testing"
Expand All @@ -39,10 +40,11 @@ func mockExecutionWithError(inputs []string) (map[string]string, error) {
for _, input := range inputs {
results[input] = input
}
return results, fmt.Errorf("mock execution error")
return results, errors.New("mock execution error")
}

func TestBatcher(t *testing.T) {
t.Parallel()
type testCase struct {
name string
mockFunc func(inputs []string) (map[string]string, error)
Expand Down Expand Up @@ -127,7 +129,6 @@ func TestBatcher(t *testing.T) {
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

Expand All @@ -136,7 +137,7 @@ func TestBatcher(t *testing.T) {

var wg sync.WaitGroup

for i := 0; i < len(tc.tasks); i++ {
for i := range tc.tasks {
wg.Add(1)
go func(taskNum int) {
defer wg.Done()
Expand All @@ -148,7 +149,7 @@ func TestBatcher(t *testing.T) {

wg.Wait()

for i := 0; i < len(tc.tasks); i++ {
for i := range tc.tasks {
select {
case r := <-resultChans[i]:
task := fmt.Sprintf("task%d", i)
Expand All @@ -175,7 +176,7 @@ func TestBatcherConcurrentTaskAdditions(t *testing.T) {
b := New(numTasks, 1*time.Second, mockExecution)
resultChans := make([]chan BatchResult[string], numTasks)

for i := 0; i < numTasks; i++ {
for i := range numTasks {
wg.Add(1)
go func(taskNum int) {
defer wg.Done()
Expand All @@ -187,7 +188,7 @@ func TestBatcherConcurrentTaskAdditions(t *testing.T) {

wg.Wait()

for i := 0; i < numTasks; i++ {
for i := range numTasks {
r := <-resultChans[i]
task := fmt.Sprintf("task%d", i)
if r.Err != nil {
Expand Down
Loading

0 comments on commit 7f8e841

Please sign in to comment.