Skip to content

Commit

Permalink
[YUNIKORN-2184] k8shim: fix goconst linter issues (apache#736)
Browse files Browse the repository at this point in the history 
Closes: apache#736

Signed-off-by: PoAn Yang <[email protected]>
  • Loading branch information
@targetoee @FrankYang0529
targetoee authored and FrankYang0529 committed Nov 25, 2023
1 parent c5a82ac commit 779450c
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 31 deletions.
3 changes: 2 additions & 1 deletion pkg/admission/webhook_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/cache/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
28 changes: 9 additions & 19 deletions pkg/cache/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -443,22 +439,20 @@ 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'")
}
// 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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1001,27 +994,25 @@ 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'")
}
// 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)
Expand Down Expand Up @@ -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"
Expand Down
13 changes: 4 additions & 9 deletions pkg/cache/context_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ import (
)

const (
Host1 = "HOST1"
Host1 = "HOST1"
appID1 = "app00001"
appID2 = "app00002"
appID3 = "app00003"
)

var (
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
6 changes: 5 additions & 1 deletion pkg/cache/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 779450c

Please sign in to comment.