From 1215133e96b0bcbb89d3fd1eaad572dcd5461f6f Mon Sep 17 00:00:00 2001 From: Nirupam Bhowmick <48842933+heydryft@users.noreply.github.com> Date: Sun, 8 Nov 2026 13:52:47 +0000 Subject: [PATCH] revert: instances create --volume-mode copy|mount_ro (#15) Volumes go back to copy only: the API no longer offers a read-only mount, so the flag, its validation, the request field, the schedule guard, the README text and the tests go. The CLI behaves as v0.11.0 for volumes. --- README.md | 12 --- internal/api/instances.go | 23 ++-- internal/api/instances_test.go | 29 ----- internal/commands/instances.go | 53 +--------- internal/commands/instances_test.go | 157 ++-------------------------- internal/commands/schedule.go | 2 +- internal/commands/schedule_test.go | 39 ------- 7 files changed, 22 insertions(+), 293 deletions(-) diff --git a/README.md b/README.md index b5293f3..5e7769a 100644 --- a/README.md +++ b/README.md @@ -51,18 +51,6 @@ runcrate instances info train # Max runtime, Idle stop, Auto stop at, Stop rea runcrate instances auto-stop train --max-runtime 8h --idle-stop off ``` -`--volume ` attaches a volume at `/workspace`. By default it is copied to the -machine's disk at boot and your changes are copied back (`--volume-mode copy`). With -`--volume-mode mount_ro` the volume is mounted read-only at `/workspace` and ready in -seconds, nothing is copied, and what you write goes to `/root/workspace`, which is not -saved. A machine or volume that cannot mount is refused with `VOLUME_MODE_UNSUPPORTED`; -it is never quietly turned into a copy. `--volume-mode` needs `--volume`, and -`schedule create` takes it for an instance schedule too (jobs always copy). - -```bash -runcrate instances create --name infer --gpu H100 --volume --volume-mode mount_ro -``` - Point the CLI at a different API origin with `runcrate config set url ` or the `RUNCRATE_API_URL` env var (defaults to `https://runcrate.ai`). diff --git a/internal/api/instances.go b/internal/api/instances.go index 5c01fb1..2939f36 100644 --- a/internal/api/instances.go +++ b/internal/api/instances.go @@ -55,19 +55,16 @@ type InstanceStatus struct { // CreateInstanceRequest is the payload for creating an instance. type CreateInstanceRequest struct { - Name string `json:"name"` - GPUType string `json:"gpu_type,omitempty"` - GPUCount int `json:"gpu_count,omitempty"` - Region string `json:"region,omitempty"` - SSHKeyID string `json:"ssh_key_id,omitempty"` - Template string `json:"template,omitempty"` - InstanceTypeID string `json:"instance_type_id,omitempty"` - StorageID string `json:"storage_id,omitempty"` - // VolumeMode is how the volume reaches the machine, with StorageID: "copy" or "mount_ro". Omitted - // when empty, which the API reads as "copy". - VolumeMode string `json:"volume_mode,omitempty"` - EnvVars []EnvVar `json:"env_vars,omitempty"` - StartupCmds []string `json:"startup_commands,omitempty"` + Name string `json:"name"` + GPUType string `json:"gpu_type,omitempty"` + GPUCount int `json:"gpu_count,omitempty"` + Region string `json:"region,omitempty"` + SSHKeyID string `json:"ssh_key_id,omitempty"` + Template string `json:"template,omitempty"` + InstanceTypeID string `json:"instance_type_id,omitempty"` + StorageID string `json:"storage_id,omitempty"` + EnvVars []EnvVar `json:"env_vars,omitempty"` + StartupCmds []string `json:"startup_commands,omitempty"` // Optional auto stop guardrails, in whole minutes. Omitted when nil. MaxRuntimeMinutes *int `json:"max_runtime_minutes,omitempty"` diff --git a/internal/api/instances_test.go b/internal/api/instances_test.go index d9e0c80..0fbc50a 100644 --- a/internal/api/instances_test.go +++ b/internal/api/instances_test.go @@ -229,35 +229,6 @@ func TestCreateInstanceSendsVolumeAsStorageID(t *testing.T) { } } -// volume_mode goes in the body only when set: an empty one leaves the API to its default, copy. -func TestCreateInstanceSendsVolumeMode(t *testing.T) { - var bodies []map[string]any - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var body map[string]any - json.NewDecoder(r.Body).Decode(&body) - bodies = append(bodies, body) - w.WriteHeader(http.StatusCreated) - json.NewEncoder(w).Encode(map[string]any{"data": Instance{ID: "inst-new"}}) - })) - defer server.Close() - client := NewClient("rc_live_test", server.URL) - - for _, req := range []CreateInstanceRequest{ - {Name: "mounted", GPUType: "H100", StorageID: "vol-1", VolumeMode: "mount_ro"}, - {Name: "default", GPUType: "H100", StorageID: "vol-1"}, - } { - if _, err := client.CreateInstance(req); err != nil { - t.Fatalf("CreateInstance(%s) error: %v", req.Name, err) - } - } - if got := bodies[0]["volume_mode"]; got != "mount_ro" { - t.Errorf("volume_mode = %v, want mount_ro", got) - } - if _, ok := bodies[1]["volume_mode"]; ok { - t.Errorf("volume_mode sent without being set: %v", bodies[1]) - } -} - func TestDeleteInstance(t *testing.T) { server := mockInstancesServer(t) defer server.Close() diff --git a/internal/commands/instances.go b/internal/commands/instances.go index 9a612df..3b1b955 100644 --- a/internal/commands/instances.go +++ b/internal/commands/instances.go @@ -241,45 +241,12 @@ var instancesTypesCmd = &cobra.Command{ // shared by instances create and schedule create. func addInstanceOnlyFlags(cmd *cobra.Command) { cmd.Flags().String("type-id", "", "Instance type ID from 'runcrate instances types'") - cmd.Flags().String("volume-mode", "", - "How --volume reaches the instance: copy (the volume is copied to the machine's disk at boot and changes are copied back, the default) or mount_ro (the volume is mounted read-only at /workspace, ready in seconds)") cmd.Flags().String("idle-stop", "", "Stop the instance after this long with GPU use under 5% and nobody connected (SSH, web terminal or editor), 5m to 24h (e.g. 30m), GPU instances only. Recommended for unattended work") } -// volumeModes are the values --volume-mode accepts, as the API spells them. -var volumeModes = []string{"copy", "mount_ro"} - -// parseVolumeModeFlag checks --volume-mode before any network call: one of volumeModes in any case, -// and only beside --volume. An empty value is the flag left out, so the body carries no volume_mode. -func parseVolumeModeFlag(value, volumeID string) (string, error) { - if value == "" { - return "", nil - } - mode := strings.ToLower(strings.TrimSpace(value)) - valid := false - for _, m := range volumeModes { - valid = valid || mode == m - } - if !valid { - return "", fmt.Errorf("invalid --volume-mode %q: use %s", value, strings.Join(volumeModes, " or ")) - } - if volumeID == "" { - return "", fmt.Errorf("--volume-mode needs --volume") - } - return mode, nil -} - -// volumeLine is the Volume line of the create output: the mode is named only when the user chose one. -func volumeLine(volumeID, volumeMode string) string { - if volumeMode == "" { - return fmt.Sprintf(" Volume: %s, at /workspace", volumeID) - } - return fmt.Sprintf(" Volume: %s, at /workspace (%s)", volumeID, volumeMode) -} - // buildInstanceRequest turns the create flags (name, gpu, gpu-count, region, -// template, type-id, volume, volume-mode, max-runtime, idle-stop) into the create body. +// template, type-id, volume, max-runtime, idle-stop) into the create body. func buildInstanceRequest(cmd *cobra.Command) (api.CreateInstanceRequest, error) { var req api.CreateInstanceRequest name, _ := cmd.Flags().GetString("name") @@ -289,7 +256,6 @@ func buildInstanceRequest(cmd *cobra.Command) (api.CreateInstanceRequest, error) template, _ := cmd.Flags().GetString("template") typeID, _ := cmd.Flags().GetString("type-id") volumeID, _ := cmd.Flags().GetString("volume") - volumeModeRaw, _ := cmd.Flags().GetString("volume-mode") maxRuntimeRaw, _ := cmd.Flags().GetString("max-runtime") idleStopRaw, _ := cmd.Flags().GetString("idle-stop") @@ -302,11 +268,6 @@ func buildInstanceRequest(cmd *cobra.Command) (api.CreateInstanceRequest, error) return req, err } - volumeMode, err := parseVolumeModeFlag(volumeModeRaw, volumeID) - if err != nil { - return req, err - } - gpuType, gpuCount, err := parseGPUFlag(gpuRaw, gpuCountRaw, cmd.Flags().Changed("gpu-count")) if err != nil { return req, err @@ -336,7 +297,6 @@ func buildInstanceRequest(cmd *cobra.Command) (api.CreateInstanceRequest, error) MaxRuntimeMinutes: maxRuntime, IdleShutdownMinutes: idleStop, StorageID: volumeID, - VolumeMode: volumeMode, }, nil } @@ -347,13 +307,8 @@ var instancesCreateCmd = &cobra.Command{ For unattended or agent-run work, set --max-runtime and --idle-stop so the instance does not keep billing after the job ends. Auto stop terminates the instance, so keep -anything you need on a storage volume. - -With --volume, --volume-mode mount_ro mounts the volume read-only at /workspace, ready in -seconds, instead of copying it to the machine's disk; write outputs to /root/workspace -(not saved). A machine or volume that cannot mount is refused, never copied instead.`, - Example: ` runcrate instances create --name train --gpu H100 --max-runtime 4h --idle-stop 30m - runcrate instances create --name infer --gpu H100 --volume --volume-mode mount_ro`, +anything you need on a storage volume.`, + Example: " runcrate instances create --name train --gpu H100 --max-runtime 4h --idle-stop 30m", RunE: func(cmd *cobra.Command, args []string) error { req, err := buildInstanceRequest(cmd) if err != nil { @@ -398,7 +353,7 @@ seconds, instead of copying it to the machine's disk; write outputs to /root/wor fmt.Printf(" Max runtime: %s\n", formatOptionalMinutes(instance.MaxRuntimeMinutes)) fmt.Printf(" Idle stop: %s\n", formatOptionalMinutes(instance.IdleShutdownMinutes)) if volumeID != "" { - fmt.Println(volumeLine(volumeID, req.VolumeMode)) + fmt.Printf(" Volume: %s, at /workspace\n", volumeID) } fmt.Println() fmt.Println(" Provisioning in the background. Track with:") diff --git a/internal/commands/instances_test.go b/internal/commands/instances_test.go index 49f1b99..490c635 100644 --- a/internal/commands/instances_test.go +++ b/internal/commands/instances_test.go @@ -2,12 +2,7 @@ package commands import ( "encoding/json" - "io" - "net/http" - "net/http/httptest" - "os" "strings" - "sync/atomic" "testing" ) @@ -194,15 +189,13 @@ func TestParseAutoStopFlag(t *testing.T) { // resetCreateFlags puts the flags of `instances create` back to their defaults after a test sets some. func resetCreateFlags(t *testing.T) { t.Helper() - t.Cleanup(putCreateFlagsBack) -} - -func putCreateFlagsBack() { - for _, name := range []string{"name", "gpu", "gpu-count", "type-id", "volume", "volume-mode"} { - f := instancesCreateCmd.Flags().Lookup(name) - f.Value.Set(f.DefValue) - f.Changed = false - } + t.Cleanup(func() { + for _, name := range []string{"name", "gpu", "gpu-count", "type-id"} { + f := instancesCreateCmd.Flags().Lookup(name) + f.Value.Set(f.DefValue) + f.Changed = false + } + }) } // --gpu-count defaults to 1, and the server refuses a gpu_count beside a type id that is not the type's @@ -241,139 +234,3 @@ func TestBuildInstanceRequestGPUCountBesideTypeID(t *testing.T) { }) } } - -// --volume-mode is sent as volume_mode only when the user gave it, in the API's spelling whatever the case. -func TestBuildInstanceRequestVolumeMode(t *testing.T) { - tests := []struct { - name string - flags map[string]string - wantMode string - wantSent bool - }{ - {"no flag sends no volume_mode", map[string]string{"volume": "vol-1"}, "", false}, - {"mount_ro", map[string]string{"volume": "vol-1", "volume-mode": "mount_ro"}, "mount_ro", true}, - {"copy", map[string]string{"volume": "vol-1", "volume-mode": "copy"}, "copy", true}, - {"any case", map[string]string{"volume": "vol-1", "volume-mode": "Mount_RO"}, "mount_ro", true}, - {"spaces around it", map[string]string{"volume": "vol-1", "volume-mode": " COPY "}, "copy", true}, - {"an empty value is no flag", map[string]string{"volume": "vol-1", "volume-mode": ""}, "", false}, - {"no volume and no flag", map[string]string{}, "", false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - resetCreateFlags(t) - instancesCreateCmd.Flags().Set("name", "box") - instancesCreateCmd.Flags().Set("gpu", "H100") - for name, value := range tt.flags { - if err := instancesCreateCmd.Flags().Set(name, value); err != nil { - t.Fatalf("set --%s: %v", name, err) - } - } - req, err := buildInstanceRequest(instancesCreateCmd) - if err != nil { - t.Fatalf("buildInstanceRequest: %v", err) - } - if req.VolumeMode != tt.wantMode { - t.Errorf("VolumeMode = %q, want %q", req.VolumeMode, tt.wantMode) - } - body, _ := json.Marshal(req) - var sent map[string]any - json.Unmarshal(body, &sent) - if got, ok := sent["volume_mode"]; ok != tt.wantSent || (ok && got != tt.wantMode) { - t.Errorf("body %s: volume_mode = %v, sent %v, want %q sent %v", body, got, ok, tt.wantMode, tt.wantSent) - } - }) - } -} - -func TestBuildInstanceRequestVolumeModeRefusals(t *testing.T) { - tests := []struct { - name string - flags map[string]string - wantErr string - }{ - {"an unknown mode", map[string]string{"volume": "vol-1", "volume-mode": "rw"}, "copy or mount_ro"}, - {"a mode nobody knows without a volume", map[string]string{"volume-mode": "rw"}, "copy or mount_ro"}, - {"a mode without a volume", map[string]string{"volume-mode": "mount_ro"}, "--volume-mode needs --volume"}, - {"copy without a volume", map[string]string{"volume-mode": "copy"}, "--volume-mode needs --volume"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - resetCreateFlags(t) - instancesCreateCmd.Flags().Set("name", "box") - instancesCreateCmd.Flags().Set("gpu", "H100") - for name, value := range tt.flags { - instancesCreateCmd.Flags().Set(name, value) - } - _, err := buildInstanceRequest(instancesCreateCmd) - if err == nil || !strings.Contains(err.Error(), tt.wantErr) { - t.Errorf("err = %v, want it to mention %q", err, tt.wantErr) - } - }) - } -} - -// A refused --volume-mode never reaches the network: the whole command runs against a server that counts -// requests, and a good one sends volume_mode in the create body. -func TestInstancesCreateVolumeModeBeforeAnyCall(t *testing.T) { - var calls atomic.Int32 - var created map[string]any - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - calls.Add(1) - switch { - case r.Method == http.MethodPost && r.URL.Path == "/api/v1/instances": - raw, _ := io.ReadAll(r.Body) - json.Unmarshal(raw, &created) - w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{"data":{"id":"inst-1","name":"box","gpu_type":"H100","gpu_count":1}}`)) - default: - w.Write([]byte(`{"data":[]}`)) - } - })) - defer server.Close() - t.Setenv("HOME", t.TempDir()) - t.Setenv("RUNCRATE_API_KEY", "rc_live_test") - t.Setenv("RUNCRATE_API_URL", server.URL) - - t.Cleanup(putCreateFlagsBack) - run := func(flags map[string]string) error { - putCreateFlagsBack() - instancesCreateCmd.Flags().Set("name", "box") - instancesCreateCmd.Flags().Set("gpu", "H100") - for name, value := range flags { - instancesCreateCmd.Flags().Set(name, value) - } - stdout := os.Stdout - devnull, _ := os.Open(os.DevNull) - os.Stdout = devnull - defer func() { os.Stdout = stdout; devnull.Close() }() - return instancesCreateCmd.RunE(instancesCreateCmd, nil) - } - - for _, flags := range []map[string]string{ - {"volume": "vol-1", "volume-mode": "rw"}, - {"volume-mode": "mount_ro"}, - } { - if err := run(flags); err == nil { - t.Errorf("flags %v: expected an error", flags) - } - if n := calls.Load(); n != 0 { - t.Fatalf("flags %v: %d requests were made before the refusal", flags, n) - } - } - - if err := run(map[string]string{"volume": "vol-1", "volume-mode": "mount_ro"}); err != nil { - t.Fatalf("create with mount_ro: %v", err) - } - if created["storage_id"] != "vol-1" || created["volume_mode"] != "mount_ro" { - t.Errorf("create body = %v, want storage_id vol-1 and volume_mode mount_ro", created) - } -} - -func TestVolumeLine(t *testing.T) { - if got, want := volumeLine("vol-1", ""), " Volume: vol-1, at /workspace"; got != want { - t.Errorf("without a mode = %q, want %q", got, want) - } - if got, want := volumeLine("vol-1", "mount_ro"), " Volume: vol-1, at /workspace (mount_ro)"; got != want { - t.Errorf("with a mode = %q, want %q", got, want) - } -} diff --git a/internal/commands/schedule.go b/internal/commands/schedule.go index 0947e34..e77a3d8 100644 --- a/internal/commands/schedule.go +++ b/internal/commands/schedule.go @@ -144,7 +144,7 @@ func resolveTimezone(name string) (*time.Location, error) { // Flags that only one kind takes. var ( - instanceOnlyScheduleFlags = []string{"type-id", "idle-stop", "volume-mode"} + instanceOnlyScheduleFlags = []string{"type-id", "idle-stop"} jobOnlyScheduleFlags = []string{"image", "env"} ) diff --git a/internal/commands/schedule_test.go b/internal/commands/schedule_test.go index d4359cd..908567a 100644 --- a/internal/commands/schedule_test.go +++ b/internal/commands/schedule_test.go @@ -362,42 +362,3 @@ func TestConfirm(t *testing.T) { t.Error("confirm with no input should fail so delete asks for --yes") } } - -// An instance schedule carries volume_mode in its spec (the schedules API passes it to the launch); -// a job always copies, so a job schedule refuses the flag. -func TestBuildScheduleRequestVolumeMode(t *testing.T) { - t.Setenv("TZ", "UTC") - req, _, err := parseScheduleCreate(t, "--cron", "0 2 * * *", "--name", "box", "--gpu", "H100", - "--max-runtime", "4h", "--volume", "vol-2", "--volume-mode", "MOUNT_RO") - if err != nil { - t.Fatalf("instance: %v", err) - } - spec, _ := asJSON(t, req)["spec"].(map[string]any) - if spec["storage_id"] != "vol-2" || spec["volume_mode"] != "mount_ro" { - t.Errorf("spec = %v, want storage_id vol-2 and volume_mode mount_ro", spec) - } - - req, _, err = parseScheduleCreate(t, "--cron", "0 2 * * *", "--name", "box", "--gpu", "H100", - "--max-runtime", "4h", "--volume", "vol-2") - if err != nil { - t.Fatalf("instance without a mode: %v", err) - } - if spec, _ := asJSON(t, req)["spec"].(map[string]any); spec["volume_mode"] != nil { - t.Errorf("volume_mode sent without the flag: %v", spec) - } - - for name, argv := range map[string][]string{ - "needs a volume": {"--cron", "0 2 * * *", "--name", "box", "--gpu", "H100", "--max-runtime", "4h", "--volume-mode", "mount_ro"}, - "unknown value": {"--cron", "0 2 * * *", "--name", "box", "--gpu", "H100", "--max-runtime", "4h", "--volume", "vol-2", "--volume-mode", "rw"}, - "on a job": {"--cron", "0 2 * * *", "--gpu", "H100", "--max-runtime", "1h", "--volume", "vol-2", "--volume-mode", "mount_ro", "--", "true"}, - } { - if _, _, err := parseScheduleCreate(t, argv...); err == nil { - t.Errorf("%s: expected an error", name) - } - } - _, _, err = parseScheduleCreate(t, "--cron", "0 2 * * *", "--gpu", "H100", "--max-runtime", "1h", - "--volume", "vol-2", "--volume-mode", "mount_ro", "--", "true") - if err == nil || !strings.Contains(err.Error(), "--volume-mode only applies to instance") { - t.Errorf("job err = %v, want it to say --volume-mode only applies to instance schedules", err) - } -}