From c259dfa801986b93e5bc7e9eaf9a96619f19f251 Mon Sep 17 00:00:00 2001 From: Thomas Leterme Date: Wed, 7 Oct 2026 09:21:12 +0000 Subject: [PATCH 1/2] feat(ovhcloud-cli): migrate instance lifecycle commands to API v2 - Migrate start/stop/shelve/unshelve to the v2 declarative powerState update - Migrate reboot/reboot-rescue/exit-rescue to the v2 instance action route - Add lock/unlock commands - Add --wait to start/stop/shelve/unshelve/reboot - Remove resume, which has no v2 equivalent - Resync the instance action route in cloud_v2.json with the live spec Signed-off-by: Thomas Leterme --- doc/ovhcloud_cloud_instance.md | 3 +- doc/ovhcloud_cloud_instance_lock.md | 39 ++++ doc/ovhcloud_cloud_instance_reboot.md | 1 + doc/ovhcloud_cloud_instance_shelve.md | 1 + doc/ovhcloud_cloud_instance_start.md | 1 + doc/ovhcloud_cloud_instance_stop.md | 1 + ...e.md => ovhcloud_cloud_instance_unlock.md} | 9 +- doc/ovhcloud_cloud_instance_unshelve.md | 1 + internal/assets/api-schemas/cloud_v2.json | 23 +- internal/cmd/cloud_instance.go | 53 +++-- internal/cmd/cloud_instance_test.go | 122 ++++++++++ internal/services/cloud/cloud_instance.go | 218 +++++++----------- 12 files changed, 300 insertions(+), 172 deletions(-) create mode 100644 doc/ovhcloud_cloud_instance_lock.md rename doc/{ovhcloud_cloud_instance_resume.md => ovhcloud_cloud_instance_unlock.md} (87%) diff --git a/doc/ovhcloud_cloud_instance.md b/doc/ovhcloud_cloud_instance.md index 28fed3f2..918f0583 100644 --- a/doc/ovhcloud_cloud_instance.md +++ b/doc/ovhcloud_cloud_instance.md @@ -44,14 +44,15 @@ Manage instances in the given cloud project * [ovhcloud cloud instance image](ovhcloud_cloud_instance_image.md) - List available images in the given cloud project * [ovhcloud cloud instance interface](ovhcloud_cloud_instance_interface.md) - Manage interfaces of the given instance * [ovhcloud cloud instance list](ovhcloud_cloud_instance_list.md) - List your instances +* [ovhcloud cloud instance lock](ovhcloud_cloud_instance_lock.md) - Lock the given instance to prevent accidental actions and modifications * [ovhcloud cloud instance reboot](ovhcloud_cloud_instance_reboot.md) - Reboot the given instance * [ovhcloud cloud instance reboot-rescue](ovhcloud_cloud_instance_reboot-rescue.md) - Reboot the given instance in rescue mode * [ovhcloud cloud instance reinstall](ovhcloud_cloud_instance_reinstall.md) - Reinstall the given instance -* [ovhcloud cloud instance resume](ovhcloud_cloud_instance_resume.md) - Resume the given suspended instance * [ovhcloud cloud instance set-flavor](ovhcloud_cloud_instance_set-flavor.md) - Migrate the given instance to the specified flavor * [ovhcloud cloud instance set-name](ovhcloud_cloud_instance_set-name.md) - Set the name of the given instance * [ovhcloud cloud instance shelve](ovhcloud_cloud_instance_shelve.md) - Shelve the given instance * [ovhcloud cloud instance start](ovhcloud_cloud_instance_start.md) - Start the given instance * [ovhcloud cloud instance stop](ovhcloud_cloud_instance_stop.md) - Stop the given instance +* [ovhcloud cloud instance unlock](ovhcloud_cloud_instance_unlock.md) - Unlock the given instance * [ovhcloud cloud instance unshelve](ovhcloud_cloud_instance_unshelve.md) - Unshelve the given instance diff --git a/doc/ovhcloud_cloud_instance_lock.md b/doc/ovhcloud_cloud_instance_lock.md new file mode 100644 index 00000000..dafa1f86 --- /dev/null +++ b/doc/ovhcloud_cloud_instance_lock.md @@ -0,0 +1,39 @@ +## ovhcloud cloud instance lock + +Lock the given instance to prevent accidental actions and modifications + +``` +ovhcloud cloud instance lock [flags] +``` + +### Options + +``` + -h, --help help for lock + --wait Wait for the instance to be locked before exiting +``` + +### Options inherited from parent commands + +``` + --cloud-project string Cloud project ID + -d, --debug Activate debug mode (will log all HTTP requests details) + -e, --ignore-errors Ignore errors in API calls when it is not fatal to the execution + -o, --output string Output format: json, yaml, interactive, or a custom format expression (using https://github.com/PaesslerAG/gval syntax) + Examples: + --output json + --output yaml + --output interactive + --output 'id' (to extract a single field) + --output 'nested.field.subfield' (to extract a nested field) + --output '[id, "name"]' (to extract multiple fields as an array) + --output '{"newKey": oldKey, "otherKey": nested.field}' (to extract and rename fields in an object) + --output 'name+","+type' (to extract and concatenate fields in a string) + --output '(nbFieldA + nbFieldB) * 10' (to compute values from numeric fields) + --profile string Use a specific profile from the configuration file +``` + +### SEE ALSO + +* [ovhcloud cloud instance](ovhcloud_cloud_instance.md) - Manage instances in the given cloud project + diff --git a/doc/ovhcloud_cloud_instance_reboot.md b/doc/ovhcloud_cloud_instance_reboot.md index 3f1db066..b819789d 100644 --- a/doc/ovhcloud_cloud_instance_reboot.md +++ b/doc/ovhcloud_cloud_instance_reboot.md @@ -11,6 +11,7 @@ ovhcloud cloud instance reboot [flags] ``` -h, --help help for reboot -t, --type string Reboot type: hard or soft (default is soft) (default "soft") + --wait Wait for the instance to be rebooted before exiting ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_cloud_instance_shelve.md b/doc/ovhcloud_cloud_instance_shelve.md index 2b54515e..2e854bfc 100644 --- a/doc/ovhcloud_cloud_instance_shelve.md +++ b/doc/ovhcloud_cloud_instance_shelve.md @@ -17,6 +17,7 @@ ovhcloud cloud instance shelve [flags] ``` -h, --help help for shelve + --wait Wait for the instance to be shelved before exiting ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_cloud_instance_start.md b/doc/ovhcloud_cloud_instance_start.md index e1bd3d54..98515a0a 100644 --- a/doc/ovhcloud_cloud_instance_start.md +++ b/doc/ovhcloud_cloud_instance_start.md @@ -10,6 +10,7 @@ ovhcloud cloud instance start [flags] ``` -h, --help help for start + --wait Wait for the instance to be started before exiting ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_cloud_instance_stop.md b/doc/ovhcloud_cloud_instance_stop.md index 7de78b12..22988128 100644 --- a/doc/ovhcloud_cloud_instance_stop.md +++ b/doc/ovhcloud_cloud_instance_stop.md @@ -10,6 +10,7 @@ ovhcloud cloud instance stop [flags] ``` -h, --help help for stop + --wait Wait for the instance to be stopped before exiting ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_cloud_instance_resume.md b/doc/ovhcloud_cloud_instance_unlock.md similarity index 87% rename from doc/ovhcloud_cloud_instance_resume.md rename to doc/ovhcloud_cloud_instance_unlock.md index f841268e..578861af 100644 --- a/doc/ovhcloud_cloud_instance_resume.md +++ b/doc/ovhcloud_cloud_instance_unlock.md @@ -1,15 +1,16 @@ -## ovhcloud cloud instance resume +## ovhcloud cloud instance unlock -Resume the given suspended instance +Unlock the given instance ``` -ovhcloud cloud instance resume [flags] +ovhcloud cloud instance unlock [flags] ``` ### Options ``` - -h, --help help for resume + -h, --help help for unlock + --wait Wait for the instance to be unlocked before exiting ``` ### Options inherited from parent commands diff --git a/doc/ovhcloud_cloud_instance_unshelve.md b/doc/ovhcloud_cloud_instance_unshelve.md index 446754a1..75a36598 100644 --- a/doc/ovhcloud_cloud_instance_unshelve.md +++ b/doc/ovhcloud_cloud_instance_unshelve.md @@ -16,6 +16,7 @@ ovhcloud cloud instance unshelve [flags] ``` -h, --help help for unshelve + --wait Wait for the instance to be unshelved before exiting ``` ### Options inherited from parent commands diff --git a/internal/assets/api-schemas/cloud_v2.json b/internal/assets/api-schemas/cloud_v2.json index 79316452..122645e8 100644 --- a/internal/assets/api-schemas/cloud_v2.json +++ b/internal/assets/api-schemas/cloud_v2.json @@ -2334,27 +2334,28 @@ }, "publicCloud.instance.InstanceActionParameters": { "type": "object", - "description": "Optional parameters for an instance action", + "description": "Instance action parameters\n\nOptional parameters refining an imperative instance action. Each field applies only to a specific action type (see InstanceActionTypeEnum) and is ignored for other actions.", "properties": { "hard": { "type": "boolean", - "description": "Whether to perform a hard reboot (only for REBOOT action)", + "description": "Hard reboot\n\nApplies only to the REBOOT action. When true, performs a hard reboot (power cycle) instead of a graceful, soft reboot. Defaults to a soft reboot when omitted.", "nullable": true }, "imageId": { "type": "string", - "description": "Rescue image ID (only for RESCUE action)", + "description": "Rescue image\n\nApplies only to the RESCUE action. Identifier of the image to boot the instance from while in rescue mode. When omitted, the backend selects a default rescue image.", "nullable": true } } }, "publicCloud.instance.InstanceActionRequest": { "type": "object", - "description": "Payload to perform an imperative action on an instance", + "description": "Instance action request\n\nPayload for triggering an imperative action on an instance (for example lock, reboot or rescue). Unlike the declarative PUT update, an action directly commands an administrative or operational state change.\n\nThe checksum from the last GET may be supplied to opt in to optimistic concurrency control; omitting it forces the action.", "properties": { "checksum": { "type": "string", - "description": "Current resource checksum for optimistic concurrency control" + "description": "Concurrency-control checksum\n\nOptional checksum obtained from the last GET of the instance. When supplied, it must still match the instance's current checksum or the action is rejected with a 409 checksum-mismatch conflict, protecting against a concurrent modification. When omitted or empty, the concurrency check is skipped and the action is forced (last-write-wins).", + "nullable": true }, "parameters": { "allOf": [ @@ -2362,7 +2363,7 @@ "$ref": "#/components/schemas/publicCloud.instance.InstanceActionParameters" } ], - "description": "Optional parameters for the action", + "description": "Parameters\n\nOptional parameters refining the requested action (see InstanceActionParameters). Only relevant for actions that accept options, such as a hard REBOOT or a RESCUE with a specific image; omit for actions that take none.", "nullable": true }, "type": { @@ -2371,17 +2372,16 @@ "$ref": "#/components/schemas/publicCloud.instance.InstanceActionTypeEnum" } ], - "description": "The action to perform" + "description": "Action type\n\nThe imperative action to perform on the instance (see InstanceActionTypeEnum: LOCK, UNLOCK, REBOOT, RESCUE, UNRESCUE)." } }, "required": [ - "checksum", "type" ] }, "publicCloud.instance.InstanceActionTypeEnum": { "type": "string", - "description": "Supported imperative action types for an instance", + "description": "Instance action type\n\nImperative action that can be triggered on an instance to change its administrative or operational state.\n\nValues\nLOCK — Lock the instance to prevent further actions on it, protecting it from accidental or unauthorized operations.\nUNLOCK — Remove a previously set lock, re-enabling actions on the instance.\nREBOOT — Restart the instance.\nRESCUE — Boot the instance into rescue mode from a rescue image so its root disk can be inspected or repaired.\nUNRESCUE — Return the instance from rescue mode to normal operation.", "enum": [ "LOCK", "REBOOT", @@ -13601,6 +13601,7 @@ "/publicCloud/project/{projectId}/compute/instance/{instanceId}/action": { "post": { "summary": "Perform an action on a Public Cloud instance (reboot, rescue, unrescue)", + "description": "### Perform an instance action\n\nImperative endpoint for **power and lifecycle actions** on an instance. This is the **one place** such imperative operations live — distinct from the declarative `PUT` on the instance, which reconciles desired state from a `targetSpec` diff. Actions are **asynchronous**: the instance transitions through the relevant transient state and the response echoes the instance; poll `GET .../instance/{instanceId}` until `resourceStatus` is `READY` and `currentTasks` is empty.\n\n### Action `type`\nThe `type` field selects the operation:\n* `REBOOT` — reboot the instance (`parameters.hard` chooses a hard vs. soft reboot).\n* `START` / `STOP` — power the instance on / off.\n* `SHELVE` / `UNSHELVE` — shelve the instance (freeing hypervisor resources) and restore it.\n* `RESCUE` / `UNRESCUE` — boot into rescue mode (optionally from `parameters.imageId`) and return to normal boot.\n* `LOCK` / `UNLOCK` — lock the instance against accidental actions / modifications, and unlock it.\n\n### Optimistic concurrency (`checksum`)\nThe `checksum` is **optional**. When you send it, it must match the current server-side value or the action is rejected with a checksum-mismatch error — re-`GET` and retry. When you omit it (or send an empty string), the check is skipped and the action is forced (last-write-wins). An action requested while the instance is in an incompatible state is rejected with `ResourceInInvalidState`.", "security": [ { "oAuth2AuthCode": [] @@ -13777,8 +13778,8 @@ ], "x-badges": [ { - "color": "orange", - "label": "Internal use only" + "color": "blue", + "label": "Alpha version" } ], "x-iam-actions": [ diff --git a/internal/cmd/cloud_instance.go b/internal/cmd/cloud_instance.go index 84776344..3d08b419 100644 --- a/internal/cmd/cloud_instance.go +++ b/internal/cmd/cloud_instance.go @@ -184,23 +184,27 @@ func initInstanceCommand(cloudCmd *cobra.Command) { setNameCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be ready before exiting") instanceCmd.AddCommand(setNameCmd) - instanceCmd.AddCommand(&cobra.Command{ + startCmd := &cobra.Command{ Use: "start ", Short: "Start the given instance", ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), Run: cloud.StartInstance, Args: cobra.ExactArgs(1), - }) + } + startCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be started before exiting") + instanceCmd.AddCommand(startCmd) - instanceCmd.AddCommand(&cobra.Command{ + stopCmd := &cobra.Command{ Use: "stop ", Short: "Stop the given instance", ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), Run: cloud.StopInstance, Args: cobra.ExactArgs(1), - }) + } + stopCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be stopped before exiting") + instanceCmd.AddCommand(stopCmd) - instanceCmd.AddCommand(&cobra.Command{ + shelveCmd := &cobra.Command{ Use: "shelve ", Short: "Shelve the given instance", Long: `The resources dedicated to the Public Cloud instance are released. @@ -210,9 +214,11 @@ The Snapshot Storage used to store the instance's data will be billed.`, ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), Run: cloud.ShelveInstance, Args: cobra.ExactArgs(1), - }) + } + shelveCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be shelved before exiting") + instanceCmd.AddCommand(shelveCmd) - instanceCmd.AddCommand(&cobra.Command{ + unshelveCmd := &cobra.Command{ Use: "unshelve ", Short: "Unshelve the given instance", Long: `The resources dedicated to the Public Cloud instance are restored. @@ -221,15 +227,9 @@ Instance billing will get back to normal and the snapshot used to store the inst ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), Run: cloud.UnshelveInstance, Args: cobra.ExactArgs(1), - }) - - instanceCmd.AddCommand(&cobra.Command{ - Use: "resume ", - Short: "Resume the given suspended instance", - ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), - Run: cloud.ResumeInstance, - Args: cobra.ExactArgs(1), - }) + } + unshelveCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be unshelved before exiting") + instanceCmd.AddCommand(unshelveCmd) rebootCmd := &cobra.Command{ Use: "reboot ", @@ -239,8 +239,29 @@ Instance billing will get back to normal and the snapshot used to store the inst Args: cobra.ExactArgs(1), } rebootCmd.Flags().StringVarP(&cloud.InstanceRebootType, "type", "t", "soft", "Reboot type: hard or soft (default is soft)") + rebootCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be rebooted before exiting") instanceCmd.AddCommand(rebootCmd) + lockCmd := &cobra.Command{ + Use: "lock ", + Short: "Lock the given instance to prevent accidental actions and modifications", + ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), + Run: cloud.LockInstance, + Args: cobra.ExactArgs(1), + } + lockCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be locked before exiting") + instanceCmd.AddCommand(lockCmd) + + unlockCmd := &cobra.Command{ + Use: "unlock ", + Short: "Unlock the given instance", + ValidArgsFunction: completion.CloudResources("/v2/publicCloud/project/%s/compute/instance"), + Run: cloud.UnlockInstance, + Args: cobra.ExactArgs(1), + } + unlockCmd.Flags().BoolVar(&flags.WaitForTask, "wait", false, "Wait for the instance to be unlocked before exiting") + instanceCmd.AddCommand(unlockCmd) + reinstallCmd := &cobra.Command{ Use: "reinstall ", Short: "Reinstall the given instance", diff --git a/internal/cmd/cloud_instance_test.go b/internal/cmd/cloud_instance_test.go index 2c17e78b..79466acc 100644 --- a/internal/cmd/cloud_instance_test.go +++ b/internal/cmd/cloud_instance_test.go @@ -6,6 +6,7 @@ package cmd_test import ( "net/http" + "strings" "github.com/jarcoal/httpmock" "github.com/maxatome/go-testdeep/td" @@ -451,3 +452,124 @@ func (ms *MockSuite) TestCloudInstanceSetNameWaitCmd(assert, require *td.T) { assert.Cmp(out, td.Contains("renamed to renamed-instance")) assert.Cmp(httpmock.GetCallCountInfo()["GET https://eu.api.ovh.com/v2/publicCloud/project/fakeProjectID/compute/instance/fakeInstanceID"], 2) } + +const instanceV2Endpoint = "https://eu.api.ovh.com/v2/publicCloud/project/fakeProjectID/compute/instance/fakeInstanceID" + +// instanceV2ResponseWithPowerState returns instanceV2Response with both the +// target and current power states set to the given value. +func instanceV2ResponseWithPowerState(powerState string) string { + return strings.ReplaceAll(instanceV2Response, `"powerState": "ACTIVE"`, `"powerState": "`+powerState+`"`) +} + +// expectedInstancePutBody is the PUT body expected when only the power state +// of instanceV2Response changes. +func expectedInstancePutBody(powerState string) td.TestDeep { + return td.JSON(`{ + "checksum": "abc123", + "targetSpec": { + "name": "my-instance", + "flavor": { "id": "flavor-d2-2" }, + "image": { "id": "image-debian-12" }, + "networks": [ + { "autoAssignPublicIp": true }, + { "id": "private-net-id", "subnetId": "private-subnet-id", "ip": "10.1.2.155" } + ], + "powerState": $1, + "securityGroups": [ { "id": "sg-default" } ], + "volumes": [ { "id": "volume-1" } ] + } + }`, powerState) +} + +func (ms *MockSuite) TestCloudInstancePowerStateCmds(assert, require *td.T) { + for _, tc := range []struct { + command string + currentPower string + targetPower string + message string + }{ + {"start", "SHUTOFF", "ACTIVE", "⚡️ Instance starting…"}, + {"stop", "ACTIVE", "SHUTOFF", "⚡️ Instance stopping…"}, + {"shelve", "ACTIVE", "SHELVED", "⚡️ Instance is being shelved…"}, + {"unshelve", "SHELVED", "ACTIVE", "⚡️ Instance is being unshelved…"}, + } { + assert.RunAssertRequire(tc.command, func(assert, require *td.T) { + defer cmd.PostExecute() + + httpmock.RegisterResponder(http.MethodGet, instanceV2Endpoint, + httpmock.NewStringResponder(200, instanceV2ResponseWithPowerState(tc.currentPower))) + httpmock.RegisterMatcherResponder(http.MethodPut, instanceV2Endpoint, + tdhttpmock.JSONBody(expectedInstancePutBody(tc.targetPower)), + httpmock.NewStringResponder(202, `{"id": "fakeInstanceID", "resourceStatus": "UPDATING"}`)) + + out, err := cmd.Execute("cloud", "instance", tc.command, "fakeInstanceID", "--cloud-project", "fakeProjectID") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains(tc.message)) + }) + } +} + +func (ms *MockSuite) TestCloudInstanceStartWaitCmd(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, instanceV2Endpoint, + httpmock.NewStringResponder(200, instanceV2Response)) + httpmock.RegisterResponder(http.MethodPut, instanceV2Endpoint, + httpmock.NewStringResponder(202, `{"id": "fakeInstanceID", "resourceStatus": "UPDATING"}`)) + + out, err := cmd.Execute("cloud", "instance", "start", "fakeInstanceID", "--wait", "--cloud-project", "fakeProjectID") + + require.CmpNoError(err) + assert.Cmp(out, td.Contains("✅ Instance fakeInstanceID started")) + assert.Cmp(httpmock.GetCallCountInfo()["GET "+instanceV2Endpoint], 2) +} + +func (ms *MockSuite) TestCloudInstanceStopLockedCmd(assert, require *td.T) { + httpmock.RegisterResponder(http.MethodGet, instanceV2Endpoint, + httpmock.NewStringResponder(200, instanceV2Response)) + httpmock.RegisterResponder(http.MethodPut, instanceV2Endpoint, + httpmock.NewStringResponder(409, `{"class": "Client::Conflict", "message": "instance is locked — unlock it before making changes: resource in invalid state"}`)) + + _, err := cmd.Execute("cloud", "instance", "stop", "fakeInstanceID", "--cloud-project", "fakeProjectID") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains("instance is locked")) +} + +func (ms *MockSuite) TestCloudInstanceActionCmds(assert, require *td.T) { + for _, tc := range []struct { + name string + args []string + body string + message string + }{ + {"reboot soft", []string{"reboot", "fakeInstanceID"}, `{"type": "REBOOT", "parameters": {"hard": false}}`, "⚡️ Instance is rebooting…"}, + {"reboot hard", []string{"reboot", "fakeInstanceID", "--type", "hard"}, `{"type": "REBOOT", "parameters": {"hard": true}}`, "⚡️ Instance is rebooting…"}, + {"rescue", []string{"reboot-rescue", "fakeInstanceID"}, `{"type": "RESCUE"}`, "⚡️ Instance is being rebooted in rescue mode…"}, + {"rescue with image", []string{"reboot-rescue", "fakeInstanceID", "--image", "rescue-image-id"}, `{"type": "RESCUE", "parameters": {"imageId": "rescue-image-id"}}`, "⚡️ Instance is being rebooted in rescue mode…"}, + {"exit rescue", []string{"exit-rescue", "fakeInstanceID"}, `{"type": "UNRESCUE"}`, "⚡️ Instance is exiting rescue mode…"}, + {"lock", []string{"lock", "fakeInstanceID"}, `{"type": "LOCK"}`, "⚡️ Instance is being locked…"}, + {"unlock", []string{"unlock", "fakeInstanceID"}, `{"type": "UNLOCK"}`, "⚡️ Instance is being unlocked…"}, + } { + assert.RunAssertRequire(tc.name, func(assert, require *td.T) { + defer cmd.PostExecute() + + httpmock.RegisterMatcherResponder(http.MethodPost, instanceV2Endpoint+"/action", + tdhttpmock.JSONBody(td.JSON(tc.body)), + httpmock.NewStringResponder(202, `{"id": "fakeInstanceID", "resourceStatus": "UPDATING"}`)) + + args := append([]string{"cloud", "instance"}, tc.args...) + out, err := cmd.Execute(append(args, "--cloud-project", "fakeProjectID")...) + + require.CmpNoError(err) + assert.Cmp(out, td.Contains(tc.message)) + }) + } +} + +func (ms *MockSuite) TestCloudInstanceRebootInvalidTypeCmd(assert, require *td.T) { + _, err := cmd.Execute("cloud", "instance", "reboot", "fakeInstanceID", "--type", "warm", "--cloud-project", "fakeProjectID") + + require.CmpError(err) + assert.Cmp(err.Error(), td.Contains(`invalid reboot type: "warm"`)) + assert.Cmp(httpmock.GetTotalCallCount(), 0) +} diff --git a/internal/services/cloud/cloud_instance.go b/internal/services/cloud/cloud_instance.go index 3e973e82..b2527e88 100644 --- a/internal/services/cloud/cloud_instance.go +++ b/internal/services/cloud/cloud_instance.go @@ -175,28 +175,18 @@ func GetInstance(_ *cobra.Command, args []string) { common.ManageObjectRequest(instanceV2Endpoint(projectID), args[0], cloudInstanceTemplate) } -func SetInstanceName(_ *cobra.Command, args []string) { - projectID, err := getConfiguredCloudProject() - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return - } - - endpoint := fmt.Sprintf("%s/%s", instanceV2Endpoint(projectID), url.PathEscape(args[0])) +func updateInstanceTargetSpec(projectID, instanceID string, change func(targetSpec map[string]any)) (map[string]any, error) { + endpoint := fmt.Sprintf("%s/%s", instanceV2Endpoint(projectID), url.PathEscape(instanceID)) - // The v2 update is declarative: send back the whole editable targetSpec - // (and the checksum) with only the name changed. var instance map[string]any if err := httpLib.Client.Get(endpoint, &instance); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error fetching instance %q: %s", args[0], err) - return + return nil, fmt.Errorf("error fetching instance %q: %w", instanceID, err) } targetSpec, ok := instance["targetSpec"].(map[string]any) if !ok { - display.OutputError(&flags.OutputFormatConfig, "instance %q has no targetSpec", args[0]) - return + return nil, fmt.Errorf("instance %q has no targetSpec", instanceID) } - targetSpec["name"] = args[1] + change(targetSpec) body, err := openapi.FilterEditableFields( assets.CloudV2OpenapiSchema, @@ -205,112 +195,121 @@ func SetInstanceName(_ *cobra.Command, args []string) { instance, ) if err != nil { - display.OutputError(&flags.OutputFormatConfig, "failed to extract writable properties: %s", err) - return + return nil, fmt.Errorf("failed to extract writable properties: %w", err) } if err := httpLib.Client.Put(endpoint, body, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error renaming instance %q: %s", args[0], err) - return + return nil, fmt.Errorf("error updating instance %q: %w", instanceID, err) } - if !flags.WaitForTask { - display.OutputInfo(&flags.OutputFormatConfig, nil, "✅ Instance %s renamed to %s", args[0], args[1]) - return + return waitForInstanceIfRequested(endpoint) +} + +// runInstanceAction triggers an imperative action (reboot, rescue, lock…) on +// the instance. When --wait is set, it returns the instance once ready. +func runInstanceAction(projectID, instanceID, actionType string, parameters map[string]any) (map[string]any, error) { + endpoint := fmt.Sprintf("%s/%s", instanceV2Endpoint(projectID), url.PathEscape(instanceID)) + + body := map[string]any{"type": actionType} + if len(parameters) > 0 { + body["parameters"] = parameters } - ready, err := waitForCloudResourceReady(endpoint, 10*time.Minute) - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "failed to wait for instance to be ready: %s", err) - return + if err := httpLib.Client.Post(endpoint+"/action", body, nil); err != nil { + return nil, fmt.Errorf("error running action %s on instance %q: %w", actionType, instanceID, err) } - display.OutputInfo(&flags.OutputFormatConfig, ready, "✅ Instance %s renamed to %s", args[0], args[1]) + return waitForInstanceIfRequested(endpoint) } -func StartInstance(_ *cobra.Command, args []string) { - projectID, err := getConfiguredCloudProject() +func waitForInstanceIfRequested(endpoint string) (map[string]any, error) { + if !flags.WaitForTask { + return nil, nil + } + + ready, err := waitForCloudResourceReady(endpoint, 10*time.Minute) if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return + return nil, fmt.Errorf("failed to wait for instance to be ready: %w", err) } - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/start", projectID, url.PathEscape(args[0])) + return ready, nil +} - if err := httpLib.Client.Post(endpoint, nil, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error starting instance %q: %s", args[0], err) +// outputInstanceResult prints the message matching the --wait mode. +func outputInstanceResult(ready map[string]any, startedMessage, doneMessage string, args ...any) { + if !flags.WaitForTask { + display.OutputInfo(&flags.OutputFormatConfig, nil, startedMessage, args...) return } - - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance starting…") + display.OutputInfo(&flags.OutputFormatConfig, ready, doneMessage, args...) } -func StopInstance(_ *cobra.Command, args []string) { +func SetInstanceName(_ *cobra.Command, args []string) { projectID, err := getConfiguredCloudProject() if err != nil { display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/stop", projectID, url.PathEscape(args[0])) - - if err := httpLib.Client.Post(endpoint, nil, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error stopping instance %q: %s", args[0], err) + ready, err := updateInstanceTargetSpec(projectID, args[0], func(targetSpec map[string]any) { + targetSpec["name"] = args[1] + }) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance stopping…") + outputInstanceResult(ready, "✅ Instance %s renamed to %s", "✅ Instance %s renamed to %s", args[0], args[1]) } -func ShelveInstance(_ *cobra.Command, args []string) { +func setInstancePowerState(args []string, powerState, startedMessage, doneMessage string) { projectID, err := getConfiguredCloudProject() if err != nil { display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/shelve", projectID, url.PathEscape(args[0])) - - if err := httpLib.Client.Post(endpoint, nil, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error shelving instance %q: %s", args[0], err) + ready, err := updateInstanceTargetSpec(projectID, args[0], func(targetSpec map[string]any) { + targetSpec["powerState"] = powerState + }) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is being shelved…") + outputInstanceResult(ready, startedMessage, doneMessage, args[0]) } -func UnshelveInstance(_ *cobra.Command, args []string) { - projectID, err := getConfiguredCloudProject() - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return - } +func StartInstance(_ *cobra.Command, args []string) { + setInstancePowerState(args, "ACTIVE", "⚡️ Instance starting…", "✅ Instance %s started") +} - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/unshelve", projectID, url.PathEscape(args[0])) +func StopInstance(_ *cobra.Command, args []string) { + setInstancePowerState(args, "SHUTOFF", "⚡️ Instance stopping…", "✅ Instance %s stopped") +} - if err := httpLib.Client.Post(endpoint, nil, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error unshelving instance %q: %s", args[0], err) - return - } +func ShelveInstance(_ *cobra.Command, args []string) { + setInstancePowerState(args, "SHELVED", "⚡️ Instance is being shelved…", "✅ Instance %s shelved") +} - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is being unshelved…") +func UnshelveInstance(_ *cobra.Command, args []string) { + setInstancePowerState(args, "ACTIVE", "⚡️ Instance is being unshelved…", "✅ Instance %s unshelved") } -func ResumeInstance(_ *cobra.Command, args []string) { +func instanceAction(args []string, actionType string, parameters map[string]any, startedMessage, doneMessage string) { projectID, err := getConfiguredCloudProject() if err != nil { display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/resume", projectID, url.PathEscape(args[0])) - - if err := httpLib.Client.Post(endpoint, nil, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error resuming instance %q: %s", args[0], err) + ready, err := runInstanceAction(projectID, args[0], actionType, parameters) + if err != nil { + display.OutputError(&flags.OutputFormatConfig, "%s", err) return } - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is being resumed…") + outputInstanceResult(ready, startedMessage, doneMessage, args[0]) } func RebootInstance(_ *cobra.Command, args []string) { @@ -319,23 +318,16 @@ func RebootInstance(_ *cobra.Command, args []string) { return } - projectID, err := getConfiguredCloudProject() - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return - } - - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/reboot", projectID, url.PathEscape(args[0])) - body := map[string]any{ - "type": InstanceRebootType, - } + instanceAction(args, "REBOOT", map[string]any{"hard": InstanceRebootType == "hard"}, + "⚡️ Instance is rebooting…", "✅ Instance %s rebooted") +} - if err := httpLib.Client.Post(endpoint, body, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error rebooting instance %q: %s", args[0], err) - return - } +func LockInstance(_ *cobra.Command, args []string) { + instanceAction(args, "LOCK", nil, "⚡️ Instance is being locked…", "✅ Instance %s locked") +} - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is rebooting…") +func UnlockInstance(_ *cobra.Command, args []string) { + instanceAction(args, "UNLOCK", nil, "⚡️ Instance is being unlocked…", "✅ Instance %s unlocked") } func CreateInstance(cmd *cobra.Command, args []string) { @@ -743,71 +735,17 @@ func DeleteInstanceInterface(_ *cobra.Command, args []string) { } func EnableInstanceInRescueMode(_ *cobra.Command, args []string) { - projectID, err := getConfiguredCloudProject() - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return - } - - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/rescueMode", projectID, url.PathEscape(args[0])) - body := map[string]any{ - "rescue": true, - } - + var parameters map[string]any if InstanceImageID != "" { - body["imageId"] = InstanceImageID - } - - if err := httpLib.Client.Post(endpoint, body, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error setting instance %q in rescue mode: %s", args[0], err) - return + parameters = map[string]any{"imageId": InstanceImageID} } - log.Println("⚡️ Instance is being rebooted in rescue mode…") - - if !flags.WaitForTask { - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is being rebooted in rescue mode…") - return - } - - if err := waitForInstanceStatus(projectID, args[0], "RESCUE"); err != nil { - display.OutputError(&flags.OutputFormatConfig, "failed to wait for instance to be in rescue mode %s", err) - return - } - - display.OutputInfo(&flags.OutputFormatConfig, nil, "✅ Instance is now in rescue mode") + instanceAction(args, "RESCUE", parameters, + "⚡️ Instance is being rebooted in rescue mode…", "✅ Instance %s is now in rescue mode") } func DisableInstanceRescueMode(_ *cobra.Command, args []string) { - projectID, err := getConfiguredCloudProject() - if err != nil { - display.OutputError(&flags.OutputFormatConfig, "%s", err) - return - } - - endpoint := fmt.Sprintf("/v1/cloud/project/%s/instance/%s/rescueMode", projectID, url.PathEscape(args[0])) - body := map[string]any{ - "rescue": false, - } - - if err := httpLib.Client.Post(endpoint, body, nil); err != nil { - display.OutputError(&flags.OutputFormatConfig, "error unsetting instance %q from rescue mode: %s", args[0], err) - return - } - - log.Println("⚡️ Instance is exiting rescue mode…") - - if !flags.WaitForTask { - display.OutputInfo(&flags.OutputFormatConfig, nil, "⚡️ Instance is exiting rescue mode…") - return - } - - if err := waitForInstanceStatus(projectID, args[0], "ACTIVE"); err != nil { - display.OutputError(&flags.OutputFormatConfig, "failed to wait for instance to exit rescue mode %s", err) - return - } - - display.OutputInfo(&flags.OutputFormatConfig, nil, "✅ Instance is no longer in rescue mode") + instanceAction(args, "UNRESCUE", nil, "⚡️ Instance is exiting rescue mode…", "✅ Instance %s is no longer in rescue mode") } func SetInstanceFlavor(_ *cobra.Command, args []string) { From 76a078daae816a0dc5fa4d71f85f5b69085aaf2c Mon Sep 17 00:00:00 2001 From: Thomas Leterme Date: Thu, 8 Oct 2026 08:10:08 +0000 Subject: [PATCH 2/2] fix(ovhcloud-cli): fix instance lifecycle messages without --wait Started messages were formatted with the instance ID but had no verb, which printed a "%!(EXTRA string=...)" suffix. Signed-off-by: Thomas Leterme --- internal/cmd/cloud_instance_test.go | 33 ++++++++++++----------- internal/services/cloud/cloud_instance.go | 18 ++++++------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/internal/cmd/cloud_instance_test.go b/internal/cmd/cloud_instance_test.go index 79466acc..8199cb5b 100644 --- a/internal/cmd/cloud_instance_test.go +++ b/internal/cmd/cloud_instance_test.go @@ -5,6 +5,7 @@ package cmd_test import ( + "encoding/json" "net/http" "strings" @@ -488,10 +489,10 @@ func (ms *MockSuite) TestCloudInstancePowerStateCmds(assert, require *td.T) { targetPower string message string }{ - {"start", "SHUTOFF", "ACTIVE", "⚡️ Instance starting…"}, - {"stop", "ACTIVE", "SHUTOFF", "⚡️ Instance stopping…"}, - {"shelve", "ACTIVE", "SHELVED", "⚡️ Instance is being shelved…"}, - {"unshelve", "SHELVED", "ACTIVE", "⚡️ Instance is being unshelved…"}, + {"start", "SHUTOFF", "ACTIVE", "⚡️ Instance fakeInstanceID starting…"}, + {"stop", "ACTIVE", "SHUTOFF", "⚡️ Instance fakeInstanceID stopping…"}, + {"shelve", "ACTIVE", "SHELVED", "⚡️ Instance fakeInstanceID is being shelved…"}, + {"unshelve", "SHELVED", "ACTIVE", "⚡️ Instance fakeInstanceID is being unshelved…"}, } { assert.RunAssertRequire(tc.command, func(assert, require *td.T) { defer cmd.PostExecute() @@ -502,10 +503,11 @@ func (ms *MockSuite) TestCloudInstancePowerStateCmds(assert, require *td.T) { tdhttpmock.JSONBody(expectedInstancePutBody(tc.targetPower)), httpmock.NewStringResponder(202, `{"id": "fakeInstanceID", "resourceStatus": "UPDATING"}`)) - out, err := cmd.Execute("cloud", "instance", tc.command, "fakeInstanceID", "--cloud-project", "fakeProjectID") + out, err := cmd.Execute("cloud", "instance", tc.command, "fakeInstanceID", "--cloud-project", "fakeProjectID", "-o", "json") require.CmpNoError(err) - assert.Cmp(out, td.Contains(tc.message)) + // Exact match: the message must not carry unused format arguments. + assert.Cmp(json.RawMessage(out), td.JSON(`{"message": $1}`, tc.message)) }) } } @@ -542,13 +544,13 @@ func (ms *MockSuite) TestCloudInstanceActionCmds(assert, require *td.T) { body string message string }{ - {"reboot soft", []string{"reboot", "fakeInstanceID"}, `{"type": "REBOOT", "parameters": {"hard": false}}`, "⚡️ Instance is rebooting…"}, - {"reboot hard", []string{"reboot", "fakeInstanceID", "--type", "hard"}, `{"type": "REBOOT", "parameters": {"hard": true}}`, "⚡️ Instance is rebooting…"}, - {"rescue", []string{"reboot-rescue", "fakeInstanceID"}, `{"type": "RESCUE"}`, "⚡️ Instance is being rebooted in rescue mode…"}, - {"rescue with image", []string{"reboot-rescue", "fakeInstanceID", "--image", "rescue-image-id"}, `{"type": "RESCUE", "parameters": {"imageId": "rescue-image-id"}}`, "⚡️ Instance is being rebooted in rescue mode…"}, - {"exit rescue", []string{"exit-rescue", "fakeInstanceID"}, `{"type": "UNRESCUE"}`, "⚡️ Instance is exiting rescue mode…"}, - {"lock", []string{"lock", "fakeInstanceID"}, `{"type": "LOCK"}`, "⚡️ Instance is being locked…"}, - {"unlock", []string{"unlock", "fakeInstanceID"}, `{"type": "UNLOCK"}`, "⚡️ Instance is being unlocked…"}, + {"reboot soft", []string{"reboot", "fakeInstanceID"}, `{"type": "REBOOT", "parameters": {"hard": false}}`, "⚡️ Instance fakeInstanceID is rebooting…"}, + {"reboot hard", []string{"reboot", "fakeInstanceID", "--type", "hard"}, `{"type": "REBOOT", "parameters": {"hard": true}}`, "⚡️ Instance fakeInstanceID is rebooting…"}, + {"rescue", []string{"reboot-rescue", "fakeInstanceID"}, `{"type": "RESCUE"}`, "⚡️ Instance fakeInstanceID is being rebooted in rescue mode…"}, + {"rescue with image", []string{"reboot-rescue", "fakeInstanceID", "--image", "rescue-image-id"}, `{"type": "RESCUE", "parameters": {"imageId": "rescue-image-id"}}`, "⚡️ Instance fakeInstanceID is being rebooted in rescue mode…"}, + {"exit rescue", []string{"exit-rescue", "fakeInstanceID"}, `{"type": "UNRESCUE"}`, "⚡️ Instance fakeInstanceID is exiting rescue mode…"}, + {"lock", []string{"lock", "fakeInstanceID"}, `{"type": "LOCK"}`, "⚡️ Instance fakeInstanceID is being locked…"}, + {"unlock", []string{"unlock", "fakeInstanceID"}, `{"type": "UNLOCK"}`, "⚡️ Instance fakeInstanceID is being unlocked…"}, } { assert.RunAssertRequire(tc.name, func(assert, require *td.T) { defer cmd.PostExecute() @@ -558,10 +560,11 @@ func (ms *MockSuite) TestCloudInstanceActionCmds(assert, require *td.T) { httpmock.NewStringResponder(202, `{"id": "fakeInstanceID", "resourceStatus": "UPDATING"}`)) args := append([]string{"cloud", "instance"}, tc.args...) - out, err := cmd.Execute(append(args, "--cloud-project", "fakeProjectID")...) + out, err := cmd.Execute(append(args, "--cloud-project", "fakeProjectID", "-o", "json")...) require.CmpNoError(err) - assert.Cmp(out, td.Contains(tc.message)) + // Exact match: the message must not carry unused format arguments. + assert.Cmp(json.RawMessage(out), td.JSON(`{"message": $1}`, tc.message)) }) } } diff --git a/internal/services/cloud/cloud_instance.go b/internal/services/cloud/cloud_instance.go index b2527e88..ad484f73 100644 --- a/internal/services/cloud/cloud_instance.go +++ b/internal/services/cloud/cloud_instance.go @@ -281,19 +281,19 @@ func setInstancePowerState(args []string, powerState, startedMessage, doneMessag } func StartInstance(_ *cobra.Command, args []string) { - setInstancePowerState(args, "ACTIVE", "⚡️ Instance starting…", "✅ Instance %s started") + setInstancePowerState(args, "ACTIVE", "⚡️ Instance %s starting…", "✅ Instance %s started") } func StopInstance(_ *cobra.Command, args []string) { - setInstancePowerState(args, "SHUTOFF", "⚡️ Instance stopping…", "✅ Instance %s stopped") + setInstancePowerState(args, "SHUTOFF", "⚡️ Instance %s stopping…", "✅ Instance %s stopped") } func ShelveInstance(_ *cobra.Command, args []string) { - setInstancePowerState(args, "SHELVED", "⚡️ Instance is being shelved…", "✅ Instance %s shelved") + setInstancePowerState(args, "SHELVED", "⚡️ Instance %s is being shelved…", "✅ Instance %s shelved") } func UnshelveInstance(_ *cobra.Command, args []string) { - setInstancePowerState(args, "ACTIVE", "⚡️ Instance is being unshelved…", "✅ Instance %s unshelved") + setInstancePowerState(args, "ACTIVE", "⚡️ Instance %s is being unshelved…", "✅ Instance %s unshelved") } func instanceAction(args []string, actionType string, parameters map[string]any, startedMessage, doneMessage string) { @@ -319,15 +319,15 @@ func RebootInstance(_ *cobra.Command, args []string) { } instanceAction(args, "REBOOT", map[string]any{"hard": InstanceRebootType == "hard"}, - "⚡️ Instance is rebooting…", "✅ Instance %s rebooted") + "⚡️ Instance %s is rebooting…", "✅ Instance %s rebooted") } func LockInstance(_ *cobra.Command, args []string) { - instanceAction(args, "LOCK", nil, "⚡️ Instance is being locked…", "✅ Instance %s locked") + instanceAction(args, "LOCK", nil, "⚡️ Instance %s is being locked…", "✅ Instance %s locked") } func UnlockInstance(_ *cobra.Command, args []string) { - instanceAction(args, "UNLOCK", nil, "⚡️ Instance is being unlocked…", "✅ Instance %s unlocked") + instanceAction(args, "UNLOCK", nil, "⚡️ Instance %s is being unlocked…", "✅ Instance %s unlocked") } func CreateInstance(cmd *cobra.Command, args []string) { @@ -741,11 +741,11 @@ func EnableInstanceInRescueMode(_ *cobra.Command, args []string) { } instanceAction(args, "RESCUE", parameters, - "⚡️ Instance is being rebooted in rescue mode…", "✅ Instance %s is now in rescue mode") + "⚡️ Instance %s is being rebooted in rescue mode…", "✅ Instance %s is now in rescue mode") } func DisableInstanceRescueMode(_ *cobra.Command, args []string) { - instanceAction(args, "UNRESCUE", nil, "⚡️ Instance is exiting rescue mode…", "✅ Instance %s is no longer in rescue mode") + instanceAction(args, "UNRESCUE", nil, "⚡️ Instance %s is exiting rescue mode…", "✅ Instance %s is no longer in rescue mode") } func SetInstanceFlavor(_ *cobra.Command, args []string) {