kubernetes-sigs · trasc · Jul 4, 2024
diff --git a/cmd/experimental/kjobctl/go.mod b/cmd/experimental/kjobctl/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.19.0
 	github.com/onsi/gomega v1.33.1
 	github.com/spf13/cobra v1.8.1
+	github.com/spf13/pflag v1.0.5
 	k8s.io/api v0.30.3
 	k8s.io/apimachinery v0.30.3
 	k8s.io/cli-runtime v0.30.3
@@ -62,7 +63,6 @@ require (
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
-	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/xlab/treeprint v1.2.0 // indirect
 	go.starlark.net v0.0.0-20230525235612-a134d8f9ddca // indirect
 	golang.org/x/net v0.25.0 // indirect

diff --git a/cmd/experimental/kjobctl/pkg/builder/builder.go b/cmd/experimental/kjobctl/pkg/builder/builder.go
@@ -21,13 +21,13 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"slices"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	k8s "k8s.io/client-go/kubernetes"
+	"k8s.io/utils/set"
 
 	"sigs.k8s.io/kueue/cmd/experimental/kjobctl/apis/v1alpha1"
 	"sigs.k8s.io/kueue/cmd/experimental/kjobctl/client-go/clientset/versioned"
@@ -41,11 +41,8 @@ var (
 	noApplicationProfileModeSpecifiedErr   = errors.New("no application profile mode specified")
 	invalidApplicationProfileModeErr       = errors.New("invalid application profile mode")
 	applicationProfileModeNotConfiguredErr = errors.New("application profile mode not configured")
-	noCommandSpecifiedErr                  = errors.New("no command specified")
-	noParallelismSpecifiedErr              = errors.New("no parallelism specified")
-	noCompletionsSpecifiedErr              = errors.New("no completions specified")
-	noRequestsSpecifiedErr                 = errors.New("no requests specified")
-	noLocalQueueSpecifiedErr               = errors.New("no local queue specified")
+	missingFlagsErr                        = errors.New("required flags missing")
+	extraFlagsErr                          = errors.New("extra flags provided")
 )
 
 type builder interface {
@@ -71,7 +68,8 @@ type Builder struct {
 	mode          *v1alpha1.SupportedMode
 	volumeBundles []v1alpha1.VolumeBundle
 
-	buildTime time.Time
+	buildTime     time.Time
+	providedFlags []v1alpha1.Flag
 }
 
 func NewBuilder(clientGetter util.ClientGetter, buildTime time.Time) *Builder {
@@ -118,6 +116,11 @@ func (b *Builder) WithLocalQueue(localQueue string) *Builder {
 	return b
 }
 
+func (b *Builder) WithProvidedFlags(flags []v1alpha1.Flag) *Builder {
+	b.providedFlags = flags
+	return b
+}
+
 func (b *Builder) validateGeneral() error {
 	if b.namespace == "" {
 		return noNamespaceSpecifiedErr
@@ -173,26 +176,19 @@ func (b *Builder) complete(ctx context.Context) error {
 }
 
 func (b *Builder) validateFlags() error {
-	if slices.Contains(b.mode.RequiredFlags, v1alpha1.CmdFlag) && len(b.command) == 0 {
-		return noCommandSpecifiedErr
-	}
+	requiredSet := set.New(b.mode.RequiredFlags...)
+	providedSet := set.New(b.providedFlags...)
 
-	if slices.Contains(b.mode.RequiredFlags, v1alpha1.ParallelismFlag) && b.parallelism == nil {
-		return noParallelismSpecifiedErr
-	}
-
-	if slices.Contains(b.mode.RequiredFlags, v1alpha1.CompletionsFlag) && b.completions == nil {
-		return noCompletionsSpecifiedErr
-	}
+	missing := requiredSet.Difference(providedSet)
+	extra := providedSet.Difference(requiredSet)
 
-	if slices.Contains(b.mode.RequiredFlags, v1alpha1.RequestFlag) && b.requests == nil {
-		return noRequestsSpecifiedErr
+	if missing.Len() > 0 {
+		return fmt.Errorf("%w: %v", missingFlagsErr, missing.SortedList())
 	}
 
-	if slices.Contains(b.mode.RequiredFlags, v1alpha1.LocalQueueFlag) && b.localQueue == "" {
-		return noLocalQueueSpecifiedErr
+	if extra.Len() > 0 {
+		return fmt.Errorf("%w: %s", extraFlagsErr, extra.SortedList())
 	}
-
 	return nil
 }
 

diff --git a/cmd/experimental/kjobctl/pkg/builder/builder_test.go b/cmd/experimental/kjobctl/pkg/builder/builder_test.go
@@ -40,12 +40,13 @@ func TestBuilder(t *testing.T) {
 	testStartTime := time.Now()
 
 	testCases := map[string]struct {
-		namespace   string
-		profile     string
-		mode        v1alpha1.ApplicationProfileMode
-		kjobctlObjs []runtime.Object
-		wantObj     runtime.Object
-		wantErr     error
+		namespace     string
+		profile       string
+		mode          v1alpha1.ApplicationProfileMode
+		providedFlags []v1alpha1.Flag
+		kjobctlObjs   []runtime.Object
+		wantObj       runtime.Object
+		wantErr       error
 	}{
 		"shouldn't build job because no namespace specified": {
 			wantErr: noNamespaceSpecifiedErr,
@@ -92,75 +93,34 @@ func TestBuilder(t *testing.T) {
 			},
 			wantErr: invalidApplicationProfileModeErr,
 		},
-		"shouldn't build job because command not specified with required flags": {
+		"shouldn't build job because required flags are missing": {
 			namespace: metav1.NamespaceDefault,
 			profile:   "profile",
 			mode:      v1alpha1.JobMode,
 			kjobctlObjs: []runtime.Object{
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
 					WithSupportedMode(v1alpha1.SupportedMode{
 						Name:          v1alpha1.JobMode,
-						RequiredFlags: []v1alpha1.Flag{v1alpha1.CmdFlag},
-					}).
-					Obj(),
-			},
-			wantErr: noCommandSpecifiedErr,
-		},
-		"shouldn't build job because parallelism not specified with required flags": {
-			namespace: metav1.NamespaceDefault,
-			profile:   "profile",
-			mode:      v1alpha1.JobMode,
-			kjobctlObjs: []runtime.Object{
-				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(v1alpha1.SupportedMode{
-						Name:          v1alpha1.JobMode,
-						RequiredFlags: []v1alpha1.Flag{v1alpha1.ParallelismFlag},
-					}).
-					Obj(),
-			},
-			wantErr: noParallelismSpecifiedErr,
-		},
-		"shouldn't build job because completions not specified with required flags": {
-			namespace: metav1.NamespaceDefault,
-			profile:   "profile",
-			mode:      v1alpha1.JobMode,
-			kjobctlObjs: []runtime.Object{
-				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(v1alpha1.SupportedMode{
-						Name:          v1alpha1.JobMode,
-						RequiredFlags: []v1alpha1.Flag{v1alpha1.CompletionsFlag},
+						RequiredFlags: []v1alpha1.Flag{v1alpha1.CmdFlag, v1alpha1.ParallelismFlag},
 					}).
 					Obj(),
 			},
-			wantErr: noCompletionsSpecifiedErr,
+			wantErr: missingFlagsErr,
 		},
-		"shouldn't build job because request not specified with required flags": {
-			namespace: metav1.NamespaceDefault,
-			profile:   "profile",
-			mode:      v1alpha1.JobMode,
+		"shouldn't build job because extra flags are provided": {
+			namespace:     metav1.NamespaceDefault,
+			profile:       "profile",
+			mode:          v1alpha1.JobMode,
+			providedFlags: []v1alpha1.Flag{v1alpha1.ParallelismFlag, v1alpha1.CmdFlag},
 			kjobctlObjs: []runtime.Object{
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
 					WithSupportedMode(v1alpha1.SupportedMode{
 						Name:          v1alpha1.JobMode,
-						RequiredFlags: []v1alpha1.Flag{v1alpha1.RequestFlag},
-					}).
-					Obj(),
-			},
-			wantErr: noRequestsSpecifiedErr,
-		},
-		"shouldn't build job because local queue not specified with required flags": {
-			namespace: metav1.NamespaceDefault,
-			profile:   "profile",
-			mode:      v1alpha1.JobMode,
-			kjobctlObjs: []runtime.Object{
-				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(v1alpha1.SupportedMode{
-						Name:          v1alpha1.JobMode,
-						RequiredFlags: []v1alpha1.Flag{v1alpha1.LocalQueueFlag},
+						RequiredFlags: []v1alpha1.Flag{v1alpha1.CmdFlag},
 					}).
 					Obj(),
 			},
-			wantErr: noLocalQueueSpecifiedErr,
+			wantErr: extraFlagsErr,
 		},
 		"should build job": {
 			namespace: metav1.NamespaceDefault,
@@ -191,6 +151,7 @@ func TestBuilder(t *testing.T) {
 				WithNamespace(tc.namespace).
 				WithProfileName(tc.profile).
 				WithModeName(tc.mode).
+				WithProvidedFlags(tc.providedFlags).
 				Do(ctx)
 
 			var opts []cmp.Option

diff --git a/cmd/experimental/kjobctl/pkg/cmd/create/create.go b/cmd/experimental/kjobctl/pkg/cmd/create/create.go
@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	corev1 "k8s.io/api/core/v1"
 	apiresource "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -39,6 +40,7 @@ import (
 	"k8s.io/kubectl/pkg/cmd/exec"
 	"k8s.io/utils/clock"
 	"k8s.io/utils/ptr"
+	"k8s.io/utils/set"
 
 	"sigs.k8s.io/kueue/cmd/experimental/kjobctl/apis/v1alpha1"
 	"sigs.k8s.io/kueue/cmd/experimental/kjobctl/pkg/builder"
@@ -71,6 +73,8 @@ const (
 
 var (
 	podRunningTimeoutDefault = 1 * time.Minute
+
+	profileFlags = set.New(commandFlagName, parallelismFlagName, completionsFlagName, requestFlagName, localQueueFlagName)
 )
 
 type CreateOptions struct {
@@ -103,6 +107,8 @@ type CreateOptions struct {
 	PrintObj printers.ResourcePrinterFunc
 
 	genericiooptions.IOStreams
+
+	providedFlags []v1alpha1.Flag
 }
 
 func NewCreateOptions(streams genericiooptions.IOStreams) *CreateOptions {
@@ -210,6 +216,12 @@ func (o *CreateOptions) Complete(clientGetter util.ClientGetter, cmd *cobra.Comm
 
 	var err error
 
+	cmd.Flags().Visit(func(f *pflag.Flag) {
+		if profileFlags.Has(f.Name) {
+			o.providedFlags = append(o.providedFlags, v1alpha1.Flag(f.Name))
+		}
+	})
+
 	o.Namespace, _, err = clientGetter.ToRawKubeConfigLoader().Namespace()
 	if err != nil {
 		return err
@@ -282,6 +294,7 @@ func (o *CreateOptions) Run(ctx context.Context, clientGetter util.ClientGetter,
 		WithCompletions(o.Completions).
 		WithRequests(o.Requests).
 		WithLocalQueue(o.LocalQueue).
+		WithProvidedFlags(o.providedFlags).
 		Do(ctx)
 	if err != nil {
 		return err

diff --git a/cmd/experimental/kjobctl/pkg/cmd/create/create_test.go b/cmd/experimental/kjobctl/pkg/cmd/create/create_test.go
@@ -159,7 +159,11 @@ func TestCreateCmd(t *testing.T) {
 			kjobctlObjs: []runtime.Object{
 				wrappers.MakeJobTemplate("job-template", metav1.NamespaceDefault).Obj(),
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+					WithSupportedMode(
+						*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+							RequiredFlags(v1alpha1.LocalQueueFlag).
+							Obj(),
+					).
 					Obj(),
 			},
 			gvk: schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"},
@@ -184,7 +188,11 @@ func TestCreateCmd(t *testing.T) {
 					Completions(1).
 					Obj(),
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+					WithSupportedMode(
+						*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+							RequiredFlags(v1alpha1.ParallelismFlag).
+							Obj(),
+					).
 					Obj(),
 			},
 			gvk: schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"},
@@ -210,7 +218,11 @@ func TestCreateCmd(t *testing.T) {
 					Completions(1).
 					Obj(),
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+					WithSupportedMode(
+						*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+							RequiredFlags(v1alpha1.CompletionsFlag).
+							Obj(),
+					).
 					Obj(),
 			},
 			gvk: schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"},
@@ -238,7 +250,11 @@ func TestCreateCmd(t *testing.T) {
 					WithContainer(*wrappers.MakeContainer("c2", "sleep").Obj()).
 					Obj(),
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+					WithSupportedMode(
+						*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+							RequiredFlags(v1alpha1.CmdFlag).
+							Obj(),
+					).
 					Obj(),
 			},
 			gvk: schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"},
@@ -276,7 +292,11 @@ func TestCreateCmd(t *testing.T) {
 					WithContainer(*wrappers.MakeContainer("c2", "sleep").Obj()).
 					Obj(),
 				wrappers.MakeApplicationProfile("profile", metav1.NamespaceDefault).
-					WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+					WithSupportedMode(
+						*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+							RequiredFlags(v1alpha1.RequestFlag).
+							Obj(),
+					).
 					Obj(),
 			},
 			gvk: schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"},

diff --git a/cmd/experimental/kjobctl/pkg/cmd/util/dry_run.go b/cmd/experimental/kjobctl/pkg/cmd/util/dry_run.go
@@ -46,10 +46,14 @@ const (
 	DryRunServer
 )
 
+const (
+	DryRunFlagName = "dry-run"
+)
+
 // AddDryRunFlag adds dry-run flag to a command.
 func AddDryRunFlag(cmd *cobra.Command) {
 	cmd.PersistentFlags().String(
-		"dry-run",
+		DryRunFlagName,
 		"none",
 		`Must be "none", "server", or "client". If client strategy, only print the object that would be sent, without sending it. If server strategy, submit server-side request without persisting the resource.`,
 	)

diff --git a/cmd/experimental/kjobctl/pkg/testing/wrappers/supported_mode_wrappers.go b/cmd/experimental/kjobctl/pkg/testing/wrappers/supported_mode_wrappers.go
@@ -37,3 +37,8 @@ func MakeSupportedMode(name v1alpha1.ApplicationProfileMode, template v1alpha1.T
 func (m *SupportedModeWrapper) Obj() *v1alpha1.SupportedMode {
 	return &m.SupportedMode
 }
+
+func (m *SupportedModeWrapper) RequiredFlags(flags ...v1alpha1.Flag) *SupportedModeWrapper {
+	m.SupportedMode.RequiredFlags = flags
+	return m
+}
diff --git a/cmd/experimental/kjobctl/test/integration/kjobctl/create_test.go b/cmd/experimental/kjobctl/test/integration/kjobctl/create_test.go
@@ -66,7 +66,11 @@ var _ = ginkgo.Describe("Kjobctl Create", ginkgo.Ordered, ginkgo.ContinueOnFailu
 			gomega.Expect(k8sClient.Create(ctx, jobTemplate)).To(gomega.Succeed())
 
 			profile = wrappers.MakeApplicationProfile("profile", ns.Name).
-				WithSupportedMode(*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").Obj()).
+				WithSupportedMode(
+					*wrappers.MakeSupportedMode(v1alpha1.JobMode, "job-template").
+						RequiredFlags(v1alpha1.CmdFlag, v1alpha1.ParallelismFlag, v1alpha1.CompletionsFlag, v1alpha1.RequestFlag, v1alpha1.LocalQueueFlag).
+						Obj(),
+				).
 				Obj()
 			gomega.Expect(k8sClient.Create(ctx, profile)).To(gomega.Succeed())
 		})
@@ -152,7 +156,17 @@ var _ = ginkgo.Describe("Kjobctl Create", ginkgo.Ordered, ginkgo.ContinueOnFailu
 				kjobctlCmd := cmd.NewKjobctlCmd(cmd.KjobctlOptions{ConfigFlags: configFlags, IOStreams: streams})
 				kjobctlCmd.SetOut(out)
 				kjobctlCmd.SetErr(outErr)
-				kjobctlCmd.SetArgs([]string{"create", "job", "-n", ns.Name, "--profile", profile.Name, "--dry-run", "server"})
+				kjobctlCmd.SetArgs([]string{
+					"create", "job",
+					"-n", ns.Name,
+					"--profile", profile.Name,
+					"--cmd", "sleep 60s",
+					"--parallelism", "2",
+					"--completions", "3",
+					"--request", "cpu=100m,memory=4Gi",
+					"--localqueue", "lq1",
+					"--dry-run", "server",
+				})
 
 				err := kjobctlCmd.Execute()
 				gomega.Expect(err).NotTo(gomega.HaveOccurred(), "%s: %s", err, out)