Skip to content

Commit 56d9b64

Browse files
committed
fix: address PR comments for mtc flags
1 parent eae72ac commit 56d9b64

10 files changed

Lines changed: 441 additions & 50 deletions

cmd/job/submit.go

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,16 @@ var (
6262
platform string
6363

6464
awaitJobCompletion bool
65-
timeoutStr string
66-
priorityClassName string
67-
isPathwaysJob bool
65+
timeout string
66+
priority string
6867
verbose bool
68+
volumeStr []string
6969

70-
volumeStr []string
71-
pathways orchestrator.PathwaysJobDefinition
70+
mtcEnabled bool
71+
mtcRamdiskDirectory string
7272

73-
mtcEnabled bool
74-
ramdiskDirectory string
73+
isPathwaysJob bool
74+
pathways orchestrator.PathwaysJobDefinition
7575

7676
gkeNapProvisioning string
7777
gkeNapReservation string
@@ -124,7 +124,7 @@ and JobSet/Kueue specific configurations like workload name, queue, nodes, and r
124124
}
125125
}
126126

127-
priorityClassName = strings.ToLower(priorityClassName)
127+
priority = strings.ToLower(priority)
128128

129129
return nil
130130
},
@@ -161,8 +161,8 @@ func init() {
161161
SubmitCmd.Flags().StringVar(&topology, "topology", "", "TPU slice topology (e.g., 2x2x1).")
162162
SubmitCmd.Flags().StringVar(&gkeScheduler, "gke-scheduler", "", "Kubernetes Scheduler name (e.g., gke.io/topology-aware-auto).")
163163
SubmitCmd.Flags().BoolVar(&awaitJobCompletion, "await-job-completion", false, "If true, gcluster will wait for the submitted job to complete.")
164-
SubmitCmd.Flags().StringVar(&timeoutStr, "timeout", "-1s", "Time to wait for job in seconds or string format (e.g. 1h, 10m). Default is max timeout (-1s).")
165-
SubmitCmd.Flags().StringVar(&priorityClassName, "priority", "", "A priority class name (e.g., low, medium, high, or any custom PriorityClass defined in the cluster). If empty, the cluster's default priority class will be used.")
164+
SubmitCmd.Flags().StringVar(&timeout, "timeout", "-1s", "Time to wait for job in seconds or string format (e.g. 1h, 10m). Default is max timeout (-1s).")
165+
SubmitCmd.Flags().StringVar(&priority, "priority", "", "A priority class name (e.g., low, medium, high, or any custom PriorityClass defined in the cluster). If empty, the cluster's default priority class will be used.")
166166
SubmitCmd.Flags().BoolVar(&verbose, "verbose", false, "Enable verbose logging for the workload (TPUs and GPUs).")
167167
SubmitCmd.Flags().StringVar(&gkeNapProvisioning, "gke-nap-provisioning", "", "Compute provisioning model for GKE NAP. Allowed values: on-demand, spot, reservation.")
168168
SubmitCmd.Flags().StringVar(&gkeNapReservation, "gke-nap-reservation", "", "Name of the Google Cloud Reservation for GKE NAP (required if --gke-nap-provisioning=reservation).")
@@ -183,8 +183,9 @@ func init() {
183183
SubmitCmd.Flags().StringArrayVar(&pathwaysWorkerEnv, "pathways-worker-env", []string{}, "Custom environment variables for the Pathways worker container in KEY=VALUE format. Can be specified multiple times.")
184184
SubmitCmd.Flags().StringVar(&pathways.ColocatedPythonSidecarImage, "pathways-colocated-python-sidecar-image", "", "Image for an optional Python-based sidecar container to run alongside the Pathways head components.")
185185
SubmitCmd.Flags().StringVar(&pathways.HeadNodePool, "pathways-head-np", "", "The node pool to use for the Pathways head job. If empty, it will be auto-detected (looking for 'cpu-np' or 'pathways-np').")
186+
186187
SubmitCmd.Flags().BoolVar(&mtcEnabled, "mtc-enabled", false, "Enable Multi-Tier Checkpointing (MTC).")
187-
SubmitCmd.Flags().StringVar(&ramdiskDirectory, "mtc-ramdisk-directory", "", "The ramdisk directory path for local checkpoints in MTC.")
188+
SubmitCmd.Flags().StringVar(&mtcRamdiskDirectory, "mtc-ramdisk-directory", "", "The ramdisk directory path for local checkpoints in MTC.")
188189

189190
_ = SubmitCmd.MarkFlagRequired("name")
190191
_ = SubmitCmd.MarkFlagRequired("compute-type")
@@ -212,7 +213,7 @@ func runSubmitCmd(cmd *cobra.Command, args []string) error {
212213
affinity["cpu-affinity"] = cpuAffinityStr
213214
}
214215

215-
if timeoutStr != "-1s" {
216+
if timeout != "-1s" {
216217
awaitJobCompletion = true
217218
}
218219

@@ -254,17 +255,17 @@ func runSubmitCmd(cmd *cobra.Command, args []string) error {
254255
GKEScheduler: gkeScheduler,
255256
AwaitJobCompletion: awaitJobCompletion,
256257
UseParallelContainers: !gkeDisableParallelContainers,
257-
Timeout: timeoutStr,
258-
PriorityClassName: priorityClassName,
258+
Timeout: timeout,
259+
PriorityClassName: priority,
260+
Verbose: verbose,
261+
MTCEnabled: mtcEnabled,
262+
MTCRamdiskDirectory: mtcRamdiskDirectory,
259263
GKENAPProvisioning: gkeNapProvisioning,
260264
GKENAPReservation: gkeNapReservation,
261265
IsPathwaysJob: isPathwaysJob,
262266
Pathways: pathways,
263-
MTCEnabled: mtcEnabled,
264-
RamdiskDirectory: ramdiskDirectory,
265267
RawMounts: volumeStr,
266268
Env: parseEnvFlags(envVars),
267-
Verbose: verbose,
268269
}
269270

270271
return orc.SubmitJob(jobDef)
@@ -381,10 +382,6 @@ func validateGKENAPFlags() error {
381382
return fmt.Errorf("--gke-nap-reservation should only be provided when --gke-nap-provisioning=reservation")
382383
}
383384

384-
if pathways.MTCEnabled {
385-
// MTC Addon validation is now handled by the GKE Orchestrator preflight checks.
386-
}
387-
388385
return nil
389386
}
390387

cmd/job/submit_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ func resetSubmitCmdFlags() {
279279
gkeScheduler = ""
280280
platform = "linux/amd64"
281281
awaitJobCompletion = false
282-
priorityClassName = "medium"
282+
priority = "medium"
283283
isPathwaysJob = false
284284
pathways = orchestrator.PathwaysJobDefinition{MaxSliceRestarts: 1}
285285
gkeNapProvisioning = ""
@@ -288,6 +288,8 @@ func resetSubmitCmdFlags() {
288288
pathwaysProxyEnv = nil
289289
pathwaysServerEnv = nil
290290
pathwaysWorkerEnv = nil
291+
mtcEnabled = false
292+
mtcRamdiskDirectory = ""
291293
}
292294

293295
type mockOrchestrator struct {
@@ -996,8 +998,8 @@ func TestSubmitCmd_PathwaysMTCFlags(t *testing.T) {
996998
t.Errorf("expected mtcEnabled to be true")
997999
}
9981000

999-
if ramdiskDirectory != "/tmp/custom_mtc_dir" {
1000-
t.Errorf("expected ramdiskDirectory to be /tmp/custom_mtc_dir, got %s", ramdiskDirectory)
1001+
if mtcRamdiskDirectory != "/tmp/custom_mtc_dir" {
1002+
t.Errorf("expected mtcRamdiskDirectory to be /tmp/custom_mtc_dir, got %s", mtcRamdiskDirectory)
10011003
}
10021004

10031005
}

0 commit comments

Comments
 (0)