Skip to content

Remove the 'all-priority-classes' flag from armadactl commands #5135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JamesMurkin wants to merge 2 commits into
base: master
Choose a base branch
from
+468 −511
Open

Remove the 'all-priority-classes' flag from armadactl commands #5135

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c24ec6
Remove the 'all-priority-classes' flag from armadactl commands
JamesMurkin Aug 27, 2026
cef6269
More robust tests
JamesMurkin Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 3 additions & 60 deletions cmd/armadactl/cmd/cancel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,6 @@ func cancelExecutorCmd() *cobra.Command {
Long: `Cancels jobs on executor with provided executor name, priority classes, queues, and pools.`,
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return fmt.Errorf("error reading all-priority-classes flag: %s", err)
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return fmt.Errorf("error marking priority-class flag as required: %s", err)
}
}
return initParams(cmd, a.Params)
},
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -92,10 +83,6 @@ func cancelExecutorCmd() *cobra.Command {
if err != nil {
return fmt.Errorf("error reading priority-class selection: %s", err)
}
allPriorityClasses, _ := cmd.Flags().GetBool("all-priority-classes")
if allPriorityClasses {
priorityClasses = nil
}

queues, err := cmd.Flags().GetStringSlice("queues")
if err != nil {
Expand All @@ -121,13 +108,7 @@ func cancelExecutorCmd() *cobra.Command {
"priority-classes",
"p",
[]string{},
"Cancel jobs on executor matching the specified priority classes. Provided priority classes should be comma separated, as in the following example: armada-default,armada-preemptible.",
)
cmd.Flags().BoolP(
"all-priority-classes",
"a",
false,
"Cancel jobs on executor for all priority classes.",
"Cancel jobs on executor matching the specified priority classes, comma separated (e.g. armada-default,armada-preemptible). If no priority classes are provided, jobs across all priority classes will be cancelled.",
)
cmd.Flags().StringSlice(
"pools",
Expand All @@ -145,15 +126,6 @@ func cancelNodeCmd() *cobra.Command {
Long: `Cancels jobs on node for executor with provided executor name, priority classes and queues.`,
Args: cobra.ExactArgs(1),
PreRunE: func(cmd *cobra.Command, args []string) error {
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return fmt.Errorf("error reading all-priority-classes flag: %s", err)
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return fmt.Errorf("error marking priority-class flag as required: %s", err)
}
}
if err := cmd.MarkFlagRequired("executor"); err != nil {
return fmt.Errorf("error marking executor flag as required: %s", err)
}
Expand All @@ -166,10 +138,6 @@ func cancelNodeCmd() *cobra.Command {
if err != nil {
return fmt.Errorf("error reading priority-class selection: %s", err)
}
allPriorityClasses, _ := cmd.Flags().GetBool("all-priority-classes")
if allPriorityClasses {
priorityClasses = nil
}

queues, err := cmd.Flags().GetStringSlice("queues")
if err != nil {
Expand All @@ -195,13 +163,7 @@ func cancelNodeCmd() *cobra.Command {
"priority-classes",
"p",
[]string{},
"Cancel jobs on node for specified executor matching the specified priority classes. Provided priority classes should be comma separated, as in the following example: armada-default,armada-preemptible.",
)
cmd.Flags().BoolP(
"all-priority-classes",
"a",
false,
"Preempt jobs on executor for all priority classes.",
"Cancel jobs on node for specified executor matching the specified priority classes, comma separated (e.g. armada-default,armada-preemptible). If no priority classes are provided, jobs across all priority classes will be cancelled.",
)
cmd.Flags().StringP(
"executor",
Expand All @@ -223,15 +185,6 @@ func cancelQueueCmd() *cobra.Command {
if err := cmd.MarkFlagRequired("job-states"); err != nil {
return err
}
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return fmt.Errorf("error reading all-priority-classes flag: %s", err)
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return fmt.Errorf("error marking priority-class flag as required: %s", err)
}
}
return initParams(cmd, a.Params)
},
RunE: func(cmd *cobra.Command, queues []string) error {
Expand Down Expand Up @@ -273,10 +226,6 @@ func cancelQueueCmd() *cobra.Command {
if err != nil {
return fmt.Errorf("error reading priority-classes flag: %s", err)
}
allPriorityClasses, _ := cmd.Flags().GetBool("all-priority-classes")
if allPriorityClasses {
priorityClasses = nil
}

pools, err := cmd.Flags().GetStringSlice("pools")
if err != nil {
Expand Down Expand Up @@ -313,13 +262,7 @@ func cancelQueueCmd() *cobra.Command {
"priority-classes",
"p",
[]string{},
"Jobs matching the provided priority classes will be cancelled.",
)
cmd.Flags().BoolP(
"all-priority-classes",
"a",
false,
"Preempt jobs on executor for all priority classes.",
"Jobs matching the provided priority classes will be cancelled. If no priority classes are provided, jobs across all priority classes will be cancelled.",
)
cmd.Flags().StringSlice(
"pools",
Expand Down
162 changes: 45 additions & 117 deletions cmd/armadactl/cmd/cancel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,158 +120,86 @@ func TestCancelQueue(t *testing.T) {
}
}

func TestCancelExecutorAllPriorityClasses(t *testing.T) {
func TestCancelExecutor(t *testing.T) {
tests := map[string]struct {
flags []flag
expectError bool
flags []flag
wantPriorityClasses []string
}{
"with all-priority-classes flag set": {
flags: []flag{{"all-priority-classes", "true"}},
expectError: false,
// Omitting priority-classes means all priority classes, which the
// executor API represents as an empty slice.
"without priority-classes": {
flags: nil,
wantPriorityClasses: []string{},
},
"without all-priority-classes and without priority-classes": {
flags: nil,
expectError: true,
"with a single priority class": {
flags: []flag{{"priority-classes", "armada-default"}},
wantPriorityClasses: []string{"armada-default"},
},
"without all-priority-classes but with priority-classes": {
flags: []flag{{"priority-classes", "armada-default"}},
expectError: false,
"with multiple priority classes": {
flags: []flag{{"priority-classes", "armada-default,armada-preemptible"}},
wantPriorityClasses: []string{"armada-default", "armada-preemptible"},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
cmd := cancelExecutorCmd()
cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return err
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return err
}
}
return nil
}

var gotPriorityClasses []string
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return nil
var err error
gotPriorityClasses, err = cmd.Flags().GetStringSlice("priority-classes")
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
return err
}

cmd.SetArgs([]string{"test-executor"})
for _, f := range tc.flags {
require.NoError(t, cmd.Flags().Set(f.name, f.value))
}
err := cmd.Execute()
if tc.expectError {
require.Error(t, err)
} else {
require.NoError(t, err)
}

require.NoError(t, cmd.Execute())
require.Equal(t, tc.wantPriorityClasses, gotPriorityClasses)
})
}
}

func TestCancelNodeAllPriorityClasses(t *testing.T) {
func TestCancelNode(t *testing.T) {
tests := map[string]struct {
flags []flag
expectError bool
flags []flag
wantPriorityClasses []string
}{
"with all-priority-classes flag set": {
flags: []flag{{"all-priority-classes", "true"}, {"executor", "test-exec"}},
expectError: false,
// Omitting priority-classes means all priority classes, which the
// node API represents as an empty slice.
"without priority-classes": {
flags: []flag{{"executor", "test-executor"}},
wantPriorityClasses: []string{},
},
"without all-priority-classes and without priority-classes": {
flags: []flag{{"executor", "test-exec"}},
expectError: true,
"with a single priority class": {
flags: []flag{{"executor", "test-executor"}, {"priority-classes", "armada-default"}},
wantPriorityClasses: []string{"armada-default"},
},
"without all-priority-classes but with priority-classes": {
flags: []flag{{"priority-classes", "armada-default"}, {"executor", "test-exec"}},
expectError: false,
"with multiple priority classes": {
flags: []flag{{"executor", "test-executor"}, {"priority-classes", "armada-default,armada-preemptible"}},
wantPriorityClasses: []string{"armada-default", "armada-preemptible"},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
cmd := cancelNodeCmd()
cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return err
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return err
}
}
if err := cmd.MarkFlagRequired("executor"); err != nil {
return err
}
return nil
}

var gotPriorityClasses []string
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return nil
var err error
gotPriorityClasses, err = cmd.Flags().GetStringSlice("priority-classes")
return err
}

cmd.SetArgs([]string{"test-node"})
for _, f := range tc.flags {
require.NoError(t, cmd.Flags().Set(f.name, f.value))
}
err := cmd.Execute()
if tc.expectError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}

func TestCancelQueueAllPriorityClasses(t *testing.T) {
tests := map[string]struct {
flags []flag
expectError bool
}{
"with all-priority-classes flag set": {
flags: []flag{{"all-priority-classes", "true"}, {"job-states", "queued"}},
expectError: false,
},
"without all-priority-classes and without priority-classes": {
flags: []flag{{"job-states", "queued"}},
expectError: true,
},
"without all-priority-classes but with priority-classes": {
flags: []flag{{"priority-classes", "armada-default"}, {"job-states", "queued"}},
expectError: false,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
cmd := cancelQueueCmd()
cmd.PreRunE = func(cmd *cobra.Command, args []string) error {
if err := cmd.MarkFlagRequired("job-states"); err != nil {
return err
}
all, err := cmd.Flags().GetBool("all-priority-classes")
if err != nil {
return err
}
if !all {
if err := cmd.MarkFlagRequired("priority-classes"); err != nil {
return err
}
}
return nil
}
cmd.RunE = func(cmd *cobra.Command, args []string) error {
return nil
}
cmd.SetArgs([]string{"test-queue"})
for _, f := range tc.flags {
require.NoError(t, cmd.Flags().Set(f.name, f.value))
}
err := cmd.Execute()
if tc.expectError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
require.NoError(t, cmd.Execute())
require.Equal(t, tc.wantPriorityClasses, gotPriorityClasses)
})
}
}
Loading
Loading