You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@yunikorn.apache.org by pa...@apache.org on 2023/11/25 01:52:48 UTC

(yunikorn-k8shim) branch master updated: [YUNIKORN-2184] k8shim: fix goconst linter issues (#736)

This is an automated email from the ASF dual-hosted git repository.

payang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git


The following commit(s) were added to refs/heads/master by this push:
     new 779450c1 [YUNIKORN-2184] k8shim: fix goconst linter issues (#736)
779450c1 is described below

commit 779450c1fa75e603fe8dade73281bfbf7ebe5902
Author: targetoee <ta...@gmail.com>
AuthorDate: Sat Nov 25 09:52:01 2023 +0800

    [YUNIKORN-2184] k8shim: fix goconst linter issues (#736)
    
    Closes: #736
    
    Signed-off-by: PoAn Yang <pa...@apache.org>
---
 pkg/admission/webhook_manager.go |  3 ++-
 pkg/cache/application.go         |  4 +++-
 pkg/cache/application_test.go    | 28 +++++++++-------------------
 pkg/cache/context_test.go        | 13 ++++---------
 pkg/cache/utils_test.go          |  6 +++++-
 5 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/pkg/admission/webhook_manager.go b/pkg/admission/webhook_manager.go
index 19e57efc..5a419fc3 100644
--- a/pkg/admission/webhook_manager.go
+++ b/pkg/admission/webhook_manager.go
@@ -50,6 +50,7 @@ const (
 	caCert2Path       = "cacert2.pem"
 	caPrivateKey1Path = "cakey1.pem"
 	caPrivateKey2Path = "cakey2.pem"
+	webhookLabel      = "yunikorn"
 )
 
 // WebhookManager is used to handle all registration requirements for the webhook, including certificates
@@ -382,7 +383,7 @@ func (wm *webhookManagerImpl) checkValidatingWebhook(webhook *v1.ValidatingWebho
 	path := "/validate-conf"
 
 	value, ok := webhook.ObjectMeta.GetLabels()["app"]
-	if !ok || value != "yunikorn" {
+	if !ok || value != webhookLabel {
 		return errors.New("webhook: missing label app=yunikorn")
 	}
 
diff --git a/pkg/cache/application.go b/pkg/cache/application.go
index 57ee897a..7e318533 100644
--- a/pkg/cache/application.go
+++ b/pkg/cache/application.go
@@ -61,6 +61,8 @@ type Application struct {
 	originatingTask            *Task // Original Pod which creates the requests
 }
 
+const transitionErr = "no transition"
+
 func (app *Application) String() string {
 	return fmt.Sprintf("applicationID: %s, queue: %s, partition: %s,"+
 		" totalNumOfTasks: %d, currentState: %s",
@@ -101,7 +103,7 @@ func (app *Application) handle(ev events.ApplicationEvent) error {
 	defer app.lock.Unlock()
 	err := app.sm.Event(context.Background(), ev.GetEvent(), app, ev.GetArgs())
 	// handle the same state transition not nil error (limit of fsm).
-	if err != nil && err.Error() != "no transition" {
+	if err != nil && err.Error() != transitionErr {
 		return err
 	}
 	return nil
diff --git a/pkg/cache/application_test.go b/pkg/cache/application_test.go
index 8b2f6d4b..092d5d9e 100644
--- a/pkg/cache/application_test.go
+++ b/pkg/cache/application_test.go
@@ -163,7 +163,6 @@ func TestFailApplication(t *testing.T) {
 			Containers: containers,
 		},
 	}
-	appID := "app-test-001"
 	app := NewApplication(appID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	task1 := NewTask("task01", app, context, pod)
 	task2 := NewTask("task02", app, context, pod)
@@ -189,8 +188,7 @@ func TestFailApplication(t *testing.T) {
 	assert.Equal(t, rt.time, int64(6))
 	// reset time to 0
 	rt.time = 0
-	appID2 := "app-test-002"
-	app2 := NewApplication(appID2, "root.abc", "testuser", testGroups, map[string]string{}, ms)
+	app2 := NewApplication(app2ID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	app2.SetState(ApplicationStates().New)
 	err = app2.handle(NewFailApplicationEvent(app2.applicationID, errMess))
 	if err == nil {
@@ -285,7 +283,6 @@ func TestSetUnallocatedPodsToFailedWhenFailApplication(t *testing.T) {
 		},
 	})
 	assert.NilError(t, err)
-	appID := "app-test-001"
 	app := NewApplication(appID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	task1 := NewTask("task01", app, context, pod1)
 	task2 := NewTaskPlaceholder("task02", app, context, pod2)
@@ -380,7 +377,6 @@ func TestSetUnallocatedPodsToFailedWhenRejectApplication(t *testing.T) {
 		},
 	})
 	assert.NilError(t, err)
-	appID := "app-test-001"
 	app := NewApplication(appID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	task1 := NewTask("task01", app, context, pod1)
 	task2 := NewTask("task02", app, context, pod2)
@@ -443,14 +439,12 @@ func TestReleaseAppAllocation(t *testing.T) {
 			Containers: containers,
 		},
 	}
-	appID := "app-test-001"
-	UUID := "testUUID001"
 	app := NewApplication(appID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	task := NewTask("task01", app, context, pod)
 	app.addTask(task)
-	task.allocationUUID = UUID
+	task.allocationUUID = taskUUID
 	// app must be running states
-	err := app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	err := app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, taskUUID))
 	if err == nil {
 		// this should give an error
 		t.Error("expecting error got 'nil'")
@@ -458,7 +452,7 @@ func TestReleaseAppAllocation(t *testing.T) {
 	// set app states to running, let event can be trigger
 	app.SetState(ApplicationStates().Running)
 	assertAppState(t, app, ApplicationStates().Running, 3*time.Second)
-	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, taskUUID))
 	assert.NilError(t, err)
 	// after handle release event the states of app must be running
 	assertAppState(t, app, ApplicationStates().Running, 3*time.Second)
@@ -530,7 +524,6 @@ func assertAppState(t *testing.T, app *Application, expectedState string, durati
 
 func TestGetNonTerminatedTaskAlias(t *testing.T) {
 	context := initContextForTest()
-	appID := "app00001"
 	app := NewApplication(appID, "root.a", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	context.addApplication(app)
 	// app doesn't have any task
@@ -1001,14 +994,12 @@ func TestReleaseAppAllocationInFailingState(t *testing.T) {
 			Containers: containers,
 		},
 	}
-	appID := "app-test-001"
-	UUID := "testUUID001"
 	app := NewApplication(appID, "root.abc", "testuser", testGroups, map[string]string{}, ms)
 	task := NewTask("task01", app, context, pod)
 	app.addTask(task)
-	task.allocationUUID = UUID
+	task.allocationUUID = taskUUID
 	// app must be running states
-	err := app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	err := app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, taskUUID))
 	if err == nil {
 		// this should give an error
 		t.Error("expecting error got 'nil'")
@@ -1016,12 +1007,12 @@ func TestReleaseAppAllocationInFailingState(t *testing.T) {
 	// set app states to running, let event can be trigger
 	app.SetState(ApplicationStates().Running)
 	assertAppState(t, app, ApplicationStates().Running, 3*time.Second)
-	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, taskUUID))
 	assert.NilError(t, err)
 	// after handle release event the states of app must be running
 	assertAppState(t, app, ApplicationStates().Running, 3*time.Second)
 	app.SetState(ApplicationStates().Failing)
-	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, UUID))
+	err = app.handle(NewReleaseAppAllocationEvent(appID, si.TerminationType_TIMEOUT, taskUUID))
 	assert.NilError(t, err)
 	// after handle release event the states of app must be failing
 	assertAppState(t, app, ApplicationStates().Failing, 3*time.Second)
@@ -1061,8 +1052,7 @@ func TestResumingStateTransitions(t *testing.T) {
 	// Add tasks
 	app.addTask(task1)
 	app.addTask(task2)
-	UUID := "testUUID001"
-	task1.allocationUUID = UUID
+	task1.allocationUUID = taskUUID
 	context.addApplication(app)
 
 	// Set app state to "reserving"
diff --git a/pkg/cache/context_test.go b/pkg/cache/context_test.go
index 1769c950..9ebb59c3 100644
--- a/pkg/cache/context_test.go
+++ b/pkg/cache/context_test.go
@@ -48,7 +48,10 @@ import (
 )
 
 const (
-	Host1 = "HOST1"
+	Host1  = "HOST1"
+	appID1 = "app00001"
+	appID2 = "app00002"
+	appID3 = "app00003"
 )
 
 var (
@@ -241,9 +244,6 @@ func TestGetApplication(t *testing.T) {
 func TestRemoveApplication(t *testing.T) {
 	// add 3 applications
 	context := initContextForTest()
-	appID1 := "app00001"
-	appID2 := "app00002"
-	appID3 := "app00003"
 	app1 := NewApplication(appID1, "root.a", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	app2 := NewApplication(appID2, "root.b", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	app3 := NewApplication(appID3, "root.c", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
@@ -312,8 +312,6 @@ func TestRemoveApplication(t *testing.T) {
 
 func TestRemoveApplicationInternal(t *testing.T) {
 	context := initContextForTest()
-	appID1 := "app00001"
-	appID2 := "app00002"
 	app1 := NewApplication(appID1, "root.a", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	app2 := NewApplication(appID2, "root.b", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	context.applications[appID1] = app1
@@ -1009,9 +1007,6 @@ func TestRemoveTask(t *testing.T) {
 func TestGetTask(t *testing.T) {
 	// add 3 applications
 	context := initContextForTest()
-	appID1 := "app00001"
-	appID2 := "app00002"
-	appID3 := "app00003"
 	app1 := NewApplication(appID1, "root.a", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	app2 := NewApplication(appID2, "root.b", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
 	app3 := NewApplication(appID3, "root.c", "testuser", testGroups, map[string]string{}, newMockSchedulerAPI())
diff --git a/pkg/cache/utils_test.go b/pkg/cache/utils_test.go
index 67e2188d..cf6d34436 100644
--- a/pkg/cache/utils_test.go
+++ b/pkg/cache/utils_test.go
@@ -29,7 +29,11 @@ import (
 	"github.com/apache/yunikorn-k8shim/pkg/common/constants"
 )
 
-const appID = "app01"
+const (
+	appID    = "app01"
+	app2ID   = "app02"
+	taskUUID = "UUID01"
+)
 
 //nolint:funlen
 func TestGetTaskGroupFromAnnotation(t *testing.T) {


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org