[YUNIKORN-3024] Update to go 1.23 (#950)

kaichiachen · craigcondit · commit 560ef3d83978 · 2025-03-05T19:52:13.000-06:00
Closes: #950 Signed-off-by: Craig Condit <ccondit@apache.org> (cherry picked from commit 2964df3)
diff --git a/.go_version b/.go_version
@@ -1 +1 @@
-1.22
+1.23
diff --git a/.golangci.yml b/.golangci.yml
@@ -18,7 +18,6 @@
 # options for analysis running
 run:
   issues-exit-code: 1
-  skip-dirs-use-default: true
   modules-download-mode: readonly
   timeout: 10m
 
diff --git a/Makefile b/Makefile
@@ -162,7 +162,7 @@ endif
 export PATH := $(BASE_DIR)/$(SHELLCHECK_PATH):$(PATH)
 
 # golangci-lint
-GOLANGCI_LINT_VERSION=1.57.2
+GOLANGCI_LINT_VERSION=1.63.4
 GOLANGCI_LINT_PATH=$(TOOLS_DIR)/golangci-lint-v$(GOLANGCI_LINT_VERSION)
 GOLANGCI_LINT_BIN=$(GOLANGCI_LINT_PATH)/golangci-lint
 GOLANGCI_LINT_ARCHIVE=golangci-lint-$(GOLANGCI_LINT_VERSION)-$(OS)-$(EXEC_ARCH).tar.gz
diff --git a/pkg/admission/admission_controller.go b/pkg/admission/admission_controller.go
@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -607,7 +608,7 @@ func (c *AdmissionController) validateConfigMap(namespace string, cm *v1.ConfigM
 		return nil
 	}
 	if !responseData.Allowed {
-		err = fmt.Errorf(responseData.Reason)
+		err = errors.New(responseData.Reason)
 		log.Log(log.Admission).Error("Configmap validation failed, aborting", zap.Error(err))
 		return err
 	}
diff --git a/pkg/admission/admission_controller_test.go b/pkg/admission/admission_controller_test.go
@@ -568,7 +568,9 @@ func TestMutate(t *testing.T) {
 	assert.Check(t, resp.Allowed, "response not allowed for unknown object type")
 	assert.Check(t, len(resp.Patch) > 0, "empty patch for deployment")
 	annotations := annotationsFromDeployment(t, resp.Patch)
-	assert.Equal(t, annotations[common.UserInfoAnnotation].(string), "{\"user\":\"testExtUser\"}")
+	annotationValue, ok := annotations[common.UserInfoAnnotation].(string)
+	assert.Assert(t, ok, "UserInfoAnnotation value is not a string")
+	assert.Equal(t, annotationValue, "{\"user\":\"testExtUser\"}")
 
 	// deployment - annotation is not set, bypassAuth is enabled
 	ac = prepareController(t, "", "", "^kube-system$,^bypass$", "", "^nolabel$", true, true)
@@ -838,7 +840,9 @@ func schedulerName(t *testing.T, patch []byte) string {
 	ops := parsePatch(t, patch)
 	for _, op := range ops {
 		if op.Path == "/spec/schedulerName" {
-			return op.Value.(string)
+			val, ok := op.Value.(string)
+			assert.Assert(t, ok, "scheduler name value is not a string")
+			return val
 		}
 	}
 	return ""
@@ -848,7 +852,9 @@ func labels(t *testing.T, patch []byte) map[string]interface{} {
 	ops := parsePatch(t, patch)
 	for _, op := range ops {
 		if op.Path == "/metadata/labels" {
-			return op.Value.(map[string]interface{})
+			val, ok := op.Value.(map[string]interface{})
+			assert.Assert(t, ok, "labels value is not a map")
+			return val
 		}
 	}
 	return make(map[string]interface{})
@@ -858,7 +864,9 @@ func annotationsFromDeployment(t *testing.T, patch []byte) map[string]interface{
 	ops := parsePatch(t, patch)
 	for _, op := range ops {
 		if op.Path == "/spec/template/metadata/annotations" {
-			return op.Value.(map[string]interface{})
+			val, ok := op.Value.(map[string]interface{})
+			assert.Assert(t, ok, "annotations value is not a map")
+			return val
 		}
 	}
 	return make(map[string]interface{})
diff --git a/pkg/cache/application_state.go b/pkg/cache/application_state.go
@@ -470,7 +470,8 @@ func newAppState() *fsm.FSM { //nolint:funlen
 			RejectApplication.String(): func(_ context.Context, event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
 				eventArgs := make([]string, 1)
-				if err := events.GetEventArgsAsStrings(eventArgs, event.Args[1].([]interface{})); err != nil {
+				generic := event.Args[1].([]interface{}) //nolint:errcheck
+				if err := events.GetEventArgsAsStrings(eventArgs, generic); err != nil {
 					log.Log(log.ShimFSM).Error("fail to parse event arg", zap.Error(err))
 					return
 				}
@@ -484,7 +485,8 @@ func newAppState() *fsm.FSM { //nolint:funlen
 			FailApplication.String(): func(_ context.Context, event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
 				eventArgs := make([]string, 1)
-				if err := events.GetEventArgsAsStrings(eventArgs, event.Args[1].([]interface{})); err != nil {
+				generic := event.Args[1].([]interface{}) //nolint:errcheck
+				if err := events.GetEventArgsAsStrings(eventArgs, generic); err != nil {
 					log.Log(log.ShimFSM).Error("fail to parse event arg", zap.Error(err))
 					return
 				}
@@ -498,7 +500,8 @@ func newAppState() *fsm.FSM { //nolint:funlen
 			ReleaseAppAllocation.String(): func(_ context.Context, event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
 				eventArgs := make([]string, 2)
-				if err := events.GetEventArgsAsStrings(eventArgs, event.Args[1].([]interface{})); err != nil {
+				generic := event.Args[1].([]interface{}) //nolint:errcheck
+				if err := events.GetEventArgsAsStrings(eventArgs, generic); err != nil {
 					log.Log(log.ShimFSM).Error("fail to parse event arg", zap.Error(err))
 					return
 				}
diff --git a/pkg/cache/placeholder_manager.go b/pkg/cache/placeholder_manager.go
@@ -40,7 +40,7 @@ type PlaceholderManager struct {
 	// and keep retrying deleting them in order to avoid wasting resources.
 	orphanPods  map[string]*v1.Pod
 	stopChan    chan struct{}
-	running     atomic.Value
+	running     atomic.Bool
 	cleanupTime time.Duration
 	// a simple mutex will do we do not have separate read and write paths
 	locking.RWMutex
@@ -54,11 +54,8 @@ var (
 func NewPlaceholderManager(clients *client.Clients) *PlaceholderManager {
 	mu.Lock()
 	defer mu.Unlock()
-	var r atomic.Value
-	r.Store(false)
 	placeholderMgr = &PlaceholderManager{
 		clients:     clients,
-		running:     r,
 		orphanPods:  make(map[string]*v1.Pod),
 		stopChan:    make(chan struct{}),
 		cleanupTime: 5 * time.Second,
@@ -173,7 +170,7 @@ func (mgr *PlaceholderManager) Stop() {
 }
 
 func (mgr *PlaceholderManager) isRunning() bool {
-	return mgr.running.Load().(bool)
+	return mgr.running.Load()
 }
 
 func (mgr *PlaceholderManager) setRunning(flag bool) {
diff --git a/pkg/cache/scheduler_callback.go b/pkg/cache/scheduler_callback.go
@@ -191,7 +191,7 @@ func (callback *AsyncRMCallback) PreemptionPredicates(args *si.PreemptionPredica
 	}
 	return &si.PreemptionPredicatesResponse{
 		Success: ok,
-		Index:   int32(index),
+		Index:   int32(index), //nolint:gosec
 	}
 }
 
diff --git a/pkg/cache/task_state.go b/pkg/cache/task_state.go
@@ -402,7 +402,8 @@ func callbacks(states *TStates) fsm.Callbacks {
 			task := event.Args[0].(*Task) //nolint:errcheck
 			eventArgs := make([]string, 1)
 			reason := ""
-			if err := events.GetEventArgsAsStrings(eventArgs, event.Args[1].([]interface{})); err != nil {
+			generic := event.Args[1].([]interface{}) //nolint:errcheck
+			if err := events.GetEventArgsAsStrings(eventArgs, generic); err != nil {
 				log.Log(log.ShimFSM).Error("failed to parse event arg", zap.Error(err))
 				reason = err.Error()
 			} else {
@@ -421,7 +422,8 @@ func callbacks(states *TStates) fsm.Callbacks {
 		beforeHook(TaskAllocated): func(_ context.Context, event *fsm.Event) {
 			task := event.Args[0].(*Task) //nolint:errcheck
 			eventArgs := make([]string, 2)
-			if err := events.GetEventArgsAsStrings(eventArgs, event.Args[1].([]interface{})); err != nil {
+			generic := event.Args[1].([]interface{}) //nolint:errcheck
+			if err := events.GetEventArgsAsStrings(eventArgs, generic); err != nil {
 				log.Log(log.ShimFSM).Error("failed to parse event arg", zap.Error(err))
 				return
 			}
diff --git a/pkg/client/apifactory_mock.go b/pkg/client/apifactory_mock.go
@@ -397,12 +397,18 @@ func (m *MockedAPIProvider) UpdatePriorityClass(oldObj *schedv1.PriorityClass, n
 	}
 }
 
-func (m *MockedAPIProvider) GetPodBindStats() BindStats {
-	return m.clients.KubeClient.(*KubeClientMock).GetBindStats()
+func (m *MockedAPIProvider) GetPodBindStats() *BindStats {
+	kubeClient, ok := m.clients.KubeClient.(*KubeClientMock)
+	if !ok {
+		log.Log(log.Test).Error("failed to get KubeClientMock")
+		return nil
+	}
+	bindStats := kubeClient.GetBindStats()
+	return &bindStats
 }
 
 func (m *MockedAPIProvider) GetBoundPods(clear bool) []BoundPod {
-	return m.clients.KubeClient.(*KubeClientMock).GetBoundPods(clear)
+	return m.clients.KubeClient.(*KubeClientMock).GetBoundPods(clear) // nolint: errcheck
 }
 
 // MockedPersistentVolumeInformer implements PersistentVolumeInformer interface
diff --git a/pkg/client/apifactory_test.go b/pkg/client/apifactory_test.go
@@ -43,3 +43,59 @@ func TestInformerTypes(t *testing.T) {
 	assert.Equal(t, "ReplicaSet", ReplicaSetInformerHandlers.String())
 	assert.Equal(t, "StatefulSet", StatefulSetInformerHandlers.String())
 }
+
+func TestMockedAPIProvider_GetPodBindStats(t *testing.T) {
+	mock := &MockedAPIProvider{
+		clients: &Clients{
+			KubeClient: &KubeClientMock{
+				bindStats: BindStats{
+					Success: 10,
+					Errors:  2,
+				},
+			},
+		},
+	}
+
+	got := mock.GetPodBindStats()
+	assert.DeepEqual(t, &BindStats{
+		Success: 10,
+		Errors:  2,
+	}, got)
+
+	nilMock := &MockedAPIProvider{
+		clients: &Clients{
+			KubeClient: nil,
+		},
+	}
+
+	got = nilMock.GetPodBindStats()
+	assert.DeepEqual(t, (*BindStats)(nil), got)
+}
+
+func TestMockedAPIProvider_GetBoundPods(t *testing.T) {
+	mock := &MockedAPIProvider{
+		clients: &Clients{
+			KubeClient: &KubeClientMock{
+				boundPods: []BoundPod{
+					{Pod: "pod1", Host: "h1"},
+					{Pod: "pod2", Host: "h2"},
+				},
+			},
+		},
+	}
+	got := mock.GetBoundPods(true)
+	assert.DeepEqual(t, []BoundPod{
+		{Pod: "pod1", Host: "h1"},
+		{Pod: "pod2", Host: "h2"},
+	}, got)
+
+	emptyMock := &MockedAPIProvider{
+		clients: &Clients{
+			KubeClient: &KubeClientMock{
+				boundPods: []BoundPod{},
+			},
+		},
+	}
+	got = emptyMock.GetBoundPods(true)
+	assert.DeepEqual(t, []BoundPod{}, got)
+}
diff --git a/pkg/conf/schedulerconf.go b/pkg/conf/schedulerconf.go
@@ -248,7 +248,7 @@ func checkNonReloadableBool(name string, old *bool, new *bool) {
 
 func GetSchedulerConf() *SchedulerConf {
 	once.Do(createConfigs)
-	return confHolder.Load().(*SchedulerConf)
+	return confHolder.Load().(*SchedulerConf) //nolint:errcheck
 }
 
 func SetSchedulerConf(conf *SchedulerConf) {
diff --git a/pkg/dispatcher/dispatch_test.go b/pkg/dispatcher/dispatch_test.go
@@ -283,7 +283,9 @@ func TestExceedAsyncDispatchLimit(t *testing.T) {
 		Stop()
 		// check error
 		if err := recover(); err != nil {
-			assert.Assert(t, strings.Contains(err.(error).Error(), "dispatcher exceeds async-dispatch limit"))
+			errStr, ok := err.(error)
+			assert.Assert(t, ok, "Expected error type from panic, got %T", err)
+			assert.Assert(t, strings.Contains(errStr.Error(), "dispatcher exceeds async-dispatch limit"))
 		} else {
 			t.Error("Panic should be caught here")
 		}
diff --git a/pkg/dispatcher/dispatcher.go b/pkg/dispatcher/dispatcher.go
@@ -55,7 +55,7 @@ type Dispatcher struct {
 	eventChan chan events.SchedulingEvent
 	stopChan  chan struct{}
 	handlers  map[EventType]map[string]func(interface{})
-	running   atomic.Value
+	running   atomic.Bool
 	lock      locking.RWMutex
 	stopped   sync.WaitGroup
 }
@@ -66,12 +66,11 @@ func initDispatcher() {
 		eventChan: make(chan events.SchedulingEvent, eventChannelCapacity),
 		handlers:  make(map[EventType]map[string]func(interface{})),
 		stopChan:  make(chan struct{}),
-		running:   atomic.Value{},
 		lock:      locking.RWMutex{},
 	}
 	dispatcher.setRunning(false)
 	DispatchTimeout = conf.GetSchedulerConf().DispatchTimeout
-	AsyncDispatchLimit = max(10000, int32(eventChannelCapacity/10))
+	AsyncDispatchLimit = max(10000, int32(eventChannelCapacity/10)) //nolint:gosec
 
 	log.Log(log.ShimDispatcher).Info("Init dispatcher",
 		zap.Int("EventChannelCapacity", eventChannelCapacity),
@@ -146,7 +145,7 @@ func Dispatch(event events.SchedulingEvent) {
 }
 
 func (p *Dispatcher) isRunning() bool {
-	return p.running.Load().(bool)
+	return p.running.Load()
 }
 
 func (p *Dispatcher) setRunning(flag bool) {
diff --git a/pkg/log/logger.go b/pkg/log/logger.go
@@ -292,14 +292,16 @@ func parseLevel(level string) *zapcore.Level {
 	}
 
 	// parse numeric
-	levelNum, err := strconv.ParseInt(level, 10, 31)
+	levelNum, err := strconv.ParseInt(level, 10, 8)
 	if err == nil {
-		zapLevel = zapcore.Level(levelNum)
-		if zapLevel < zapcore.DebugLevel {
+		// Validate levelNum is within valid zapcore.Level range before conversion
+		switch {
+		case levelNum < int64(zapcore.DebugLevel):
 			zapLevel = zapcore.DebugLevel
-		}
-		if zapLevel >= zapcore.InvalidLevel {
+		case levelNum >= int64(zapcore.InvalidLevel):
 			zapLevel = zapcore.InvalidLevel - 1
+		default:
+			zapLevel = zapcore.Level(levelNum)
 		}
 		return &zapLevel
 	}
diff --git a/pkg/log/logger_test.go b/pkg/log/logger_test.go
@@ -164,6 +164,9 @@ func TestParseLevel(t *testing.T) {
 	assert.Equal(t, zapcore.PanicLevel, *parseLevel("PAnIC"))
 	assert.Equal(t, zapcore.FatalLevel, *parseLevel("faTal"))
 	assert.Assert(t, parseLevel("x") == nil, "parse error")
+
+	assert.Assert(t, parseLevel("-129") == nil, "Values outside int8 range (-128 to 127)")
+	assert.Assert(t, parseLevel("128") == nil, "Values outside int8 range (-128 to 127)")
 }
 
 func TestParentLogger(t *testing.T) {
diff --git a/pkg/plugin/predicates/predicate_manager_test.go b/pkg/plugin/predicates/predicate_manager_test.go
@@ -1099,7 +1099,7 @@ func makeResources(milliCPU, memory, pods, extendedA, storage, hugePageA int64)
 func newPodWithPort(hostPorts ...int) *v1.Pod {
 	var networkPorts []v1.ContainerPort
 	for _, port := range hostPorts {
-		networkPorts = append(networkPorts, v1.ContainerPort{HostPort: int32(port)})
+		networkPorts = append(networkPorts, v1.ContainerPort{HostPort: int32(port)}) // nolint: gosec
 	}
 	return &v1.Pod{
 		Spec: v1.PodSpec{
diff --git a/pkg/shim/scheduler_mock_test.go b/pkg/shim/scheduler_mock_test.go
@@ -323,7 +323,7 @@ func (fc *MockScheduler) getApplicationFromCore(appID, partition string) *object
 	return fc.coreContext.Scheduler.GetClusterContext().GetApplication(appID, partition)
 }
 
-func (fc *MockScheduler) GetPodBindStats() client.BindStats {
+func (fc *MockScheduler) GetPodBindStats() *client.BindStats {
 	return fc.apiProvider.GetPodBindStats()
 }
 
diff --git a/pkg/shim/scheduler_perf_test.go b/pkg/shim/scheduler_perf_test.go
@@ -136,8 +136,10 @@ func BenchmarkSchedulingThroughPut(b *testing.B) {
 	assert.NilError(b, err, "scheduling did not finish in time")
 
 	stat := cluster.GetPodBindStats()
-	diff := stat.Last.Sub(stat.First)
-	fmt.Printf("Overall throughput: %.0f allocations/s\n", float64(totalPods)/diff.Seconds())
+	if stat != nil {
+		diff := stat.Last.Sub(stat.First)
+		fmt.Printf("Overall throughput: %.0f allocations/s\n", float64(totalPods)/diff.Seconds())
+	}
 	fmt.Println("Container allocation throughput based on metrics")
 	for _, d := range collector.getData() {
 		if d != 0 {
diff --git a/test/e2e/framework/helpers/common/utils.go b/test/e2e/framework/helpers/common/utils.go
@@ -236,7 +236,7 @@ func RunShellCmdForeground(cmdStr string) (string, error) {
 	}
 
 	if errStr := errStream.String(); len(errStr) > 0 {
-		return stdOutStream.String(), fmt.Errorf(errStr)
+		return stdOutStream.String(), errors.New(errStr)
 	}
 
 	return stdOutStream.String(), nil
diff --git a/test/e2e/framework/helpers/k8s/gang_job.go b/test/e2e/framework/helpers/k8s/gang_job.go
diff --git a/test/e2e/framework/helpers/k8s/k8s_utils.go b/test/e2e/framework/helpers/k8s/k8s_utils.go
diff --git a/test/e2e/framework/helpers/yunikorn/rest_api_utils.go b/test/e2e/framework/helpers/yunikorn/rest_api_utils.go
diff --git a/test/e2e/recovery_and_restart/recovery_and_restart_test.go b/test/e2e/recovery_and_restart/recovery_and_restart_test.go

Original file line number	Diff line number	Diff line change
`@@ -191,7 +191,7 @@ func (callback AsyncRMCallback) PreemptionPredicates(args si.PreemptionPredica`
`191`	`191`	`}`
`192`	`192`	`return &si.PreemptionPredicatesResponse{`
`193`	`193`	`Success: ok,`
`194`		`- Index: int32(index),`
	`194`	`+ Index: int32(index), //nolint:gosec`
`195`	`195`	`}`
`196`	`196`	`}`
`197`	`197`
Original file line number	Diff line number	Diff line change
`@@ -397,12 +397,18 @@ func (m MockedAPIProvider) UpdatePriorityClass(oldObj schedv1.PriorityClass, n`
`397`	`397`	`}`
`398`	`398`	`}`
`399`	`399`
`400`		`-func (m *MockedAPIProvider) GetPodBindStats() BindStats {`
`401`		`- return m.clients.KubeClient.(*KubeClientMock).GetBindStats()`
	`400`	`+func (m MockedAPIProvider) GetPodBindStats() BindStats {`
	`401`	`+ kubeClient, ok := m.clients.KubeClient.(*KubeClientMock)`
	`402`	`+ if !ok {`
	`403`	`+ log.Log(log.Test).Error("failed to get KubeClientMock")`
	`404`	`+ return nil`
	`405`	`+ }`
	`406`	`+ bindStats := kubeClient.GetBindStats()`
	`407`	`+ return &bindStats`
`402`	`408`	`}`
`403`	`409`
`404`	`410`	`func (m *MockedAPIProvider) GetBoundPods(clear bool) []BoundPod {`
`405`		`- return m.clients.KubeClient.(*KubeClientMock).GetBoundPods(clear)`
	`411`	`+ return m.clients.KubeClient.(*KubeClientMock).GetBoundPods(clear) // nolint: errcheck`
`406`	`412`	`}`
`407`	`413`
`408`	`414`	`// MockedPersistentVolumeInformer implements PersistentVolumeInformer interface`