servers modify bugfix

spec flags were implicitly being filled with default values and could cause unexpected changes in the server configuration still up for review if the options should just be positional args since they're required anyway (you can't cherry-pick which specs you want to change)
caguiclajmg · Jul 30, 2022 · e37a05a · e37a05a
1 parent d0cc56c
commit e37a05a
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 60 deletions.
diff --git a/README.md b/README.md
@@ -60,10 +60,11 @@ tensordock-cli servers manage server_id
 
 ```sh
 tensordock-cli servers deploy \
-    [--gpuModel gpu_model \]
     [--location location \]
     [--instanceType instance_type \]
+    [--gpuModel gpu_model \]
     [--gpuCount gpu_count \]
+    [--cpuModel cpu_model \]
     [--vcpus vcpus \]
     [--storage storage \]
     [--storageClass storage_class \]
@@ -74,6 +75,8 @@ tensordock-cli servers deploy \
     admin_pass
 ```
 
+**Tip**: try `tensordock-cli stock list [--type cpu]` to find out available values for `gpu_model`, `location` and `cpu_model` 
+
 #### Deploy a GPU Server
 
 ```sh
@@ -86,16 +89,29 @@ tensordock-cli servers deploy server_name admin_user admin_pass --gpuCount 2 --g
 tensordock-cli servers deploy server_name admin_user admin_pass --instanceType cpu --cpuModel Intel_Xeon_V4
 ```
 
+#### Modify a server
+
+```sh
+tensordock-cli servers modify server_id \
+    --instanceType instance_type \
+    --gpuModel gpu_mdoel \
+    --gpuCount gpu_count \
+    --cpuModel cpu_model \
+    --storage storage \
+    --vcpus vcpus \
+    --ram ram
+```
+
 #### Convert a server to a CPU instance
 
 ```sh
-tensordock-cli servers modify server_id --instanceType cpu --cpuModel Intel_Xeon_V4
+tensordock-cli servers modify server_id --instanceType cpu --cpuModel Intel_Xeon_V4 --storage 20 --vcpus 2 --ram 4
 ```
 
 #### Convert a server to a GPU instance
 
 ```sh
-tensordock-cli servers modify server_id --instanceType gpu --gpuModel Quadro_4000 --gpuCount 2
+tensordock-cli servers modify server_id --instanceType gpu --gpuModel Quadro_4000 --gpuCount 2 --storage 20 --vcpus 2 --ram 4
 ```
 
 ### Get billing info

diff --git a/api/client.go b/api/client.go
@@ -53,14 +53,14 @@ type DeployServerRequest struct {
 }
 
 type ModifyServerRequest struct {
-	ServerId     string `mapstructure:"server_id"`
-	InstanceType string `mapstructure:"instance_type"`
-	GPUModel     string `mapstructure:"gpu_model,omitempty"`
-	GPUCount     int    `mapstructure:"gpu_count,omitempty"`
-	CPUModel     string `mapstructure:"cpu_model,omitempty"`
-	VCPUs        int    `mapstructure:"vcpus"`
-	RAM          int    `mapstructure:"ram"`
-	Storage      int    `mapstructure:"storage"`
+	ServerId     string  `mapstructure:"server_id"`
+	InstanceType *string `mapstructure:"instance_type"`
+	GPUModel     *string `mapstructure:"gpu_model,omitempty"`
+	GPUCount     *int    `mapstructure:"gpu_count,omitempty"`
+	CPUModel     *string `mapstructure:"cpu_model,omitempty"`
+	VCPUs        *int    `mapstructure:"vcpus"`
+	RAM          *int    `mapstructure:"ram"`
+	Storage      *int    `mapstructure:"storage"`
 }
 
 type DeployServerResponse struct {
@@ -87,10 +87,6 @@ type ListCpuStockResponse struct {
 	} `json:"stock"`
 }
 
-type ModifyServerResponse struct {
-	Response
-}
-
 type Client struct {
 	BaseUrl  string
 	ApiKey   string
@@ -368,25 +364,32 @@ func (client *Client) ListCpuStock() (*ListCpuStockResponse, error) {
 	return &res, nil
 }
 
-func (client *Client) ModifyServer(req ModifyServerRequest) (*ModifyServerResponse, error) {
+func (client *Client) ModifyServer(req ModifyServerRequest) (*Response, error) {
 	var rawBody map[string]interface{}
 	err := mapstructure.Decode(req, &rawBody)
 	if err != nil {
 		return nil, err
 	}
 
+	// conver to map[string]string skipping nil pointers
 	body := map[string]string{}
 	for key, elem := range rawBody {
-		str := fmt.Sprintf("%v", elem)
-		body[key] = str
+		val := reflect.ValueOf(elem)
+		if val.Kind() == reflect.Ptr {
+			if val.IsNil() {
+				continue
+			}
+			val = val.Elem()
+		}
+		body[key] = fmt.Sprintf("%v", val)
 	}
 
 	raw, err := client.post("modify/single/custom", body, true)
 	if err != nil {
 		return nil, err
 	}
 
-	var res ModifyServerResponse
+	var res Response
 	if err := json.Unmarshal(*raw, &res); err != nil {
 		return nil, err
 	}

diff --git a/commands/servers.go b/commands/servers.go
@@ -64,10 +64,11 @@ var (
 		RunE:  manageServer,
 	}
 	restartCmd = &cobra.Command{
-		Use:   "restart [flags] server_id",
-		Short: "Restart a server",
-		Args:  cobra.ExactArgs(1),
-		RunE:  restartServer,
+		Use:     "restart [flags] server_id",
+		Short:   "Restart a server",
+		Args:    cobra.ExactArgs(1),
+		RunE:    restartServer,
+		PostRun: logAction("success"),
 	}
 	modifyCmd = &cobra.Command{
 		Use:     "modify [flags] server_id",
@@ -106,8 +107,8 @@ func init() {
 	serversCmd.AddCommand(restartCmd)
 
 	serversCmd.AddCommand(modifyCmd)
-	modifyCmd.Flags().String("gpuModel", "Quadro_4000", "The GPU model that you would like to provision")
 	modifyCmd.Flags().String("instanceType", "gpu", "Either \"gpu\" or \"cpu\"")
+	modifyCmd.Flags().String("gpuModel", "Quadro_4000", "The GPU model that you would like to provision")
 	modifyCmd.Flags().Int("gpuCount", 1, "The number of GPUs of the model you specified earlier")
 	modifyCmd.Flags().String("cpuModel", "Intel_Xeon_v4", "The CPU model that you would like to provision")
 	modifyCmd.Flags().Int("vcpus", 2, "Number of vCPUs that you would like")
@@ -360,41 +361,73 @@ func modifyServer(cmd *cobra.Command, args []string) error {
 
 	serverId := args[0]
 
-	instanceType, err := flags.GetString("instanceType")
-	if err != nil {
-		return err
-	}
-
-	gpuModel, err := flags.GetString("gpuModel")
-	if err != nil {
-		return err
-	}
-
-	gpuCount, err := flags.GetInt("gpuCount")
-	if err != nil {
-		return err
-	}
-
-	cpuModel, err := flags.GetString("cpuModel")
-	if err != nil {
-		return err
-	}
-
-	vcpus, err := flags.GetInt("vcpus")
-	if err != nil {
-		return err
-	}
-
-	ram, err := flags.GetInt("ram")
-	if err != nil {
-		return err
-	}
-
-	storage, err := flags.GetInt("storage")
-	if err != nil {
-		return err
-	}
-
+	var instanceType *string = nil
+	if flags.Changed("instanceType") {
+		instanceTypeVal, err := flags.GetString("instanceType")
+		if err != nil {
+			return err
+		}
+		instanceType = &instanceTypeVal
+	}
+
+	var gpuModel *string = nil
+	if flags.Changed("gpuModel") {
+		gpuModelVal, err := flags.GetString("gpuModel")
+		if err != nil {
+			return err
+		}
+		gpuModel = &gpuModelVal
+	}
+
+	var gpuCount *int = nil
+	if flags.Changed("gpuCount") {
+		gpuCountVal, err := flags.GetInt("gpuCount")
+		if err != nil {
+			return err
+		}
+		gpuCount = &gpuCountVal
+	}
+
+	var cpuModel *string = nil
+	if flags.Changed("cpuModel") {
+		cpuModelVal, err := flags.GetString("cpuModel")
+		if err != nil {
+			return err
+		}
+		cpuModel = &cpuModelVal
+	}
+
+	var vcpus *int = nil
+	if flags.Changed("vcpus") {
+		vcpusVal, err := flags.GetInt("vcpus")
+		if err != nil {
+			return err
+		}
+		vcpus = &vcpusVal
+	}
+
+	var ram *int = nil
+	if flags.Changed("ram") {
+		ramVal, err := flags.GetInt("ram")
+		if err != nil {
+			return err
+		}
+		ram = &ramVal
+	}
+
+	var storage *int = nil
+	if flags.Changed("storage") {
+		storageVal, err := flags.GetInt("storage")
+		if err != nil {
+			return err
+		}
+		storage = &storageVal
+	}
+
+	// based on tests, it seems that the endpoint does not
+	// support specifying only parts of the spec to modify
+	// (e.g. adjust VCPUs only) and that you need to specify
+	// the entirety of the server configuration on every call
 	req := api.ModifyServerRequest{
 		ServerId:     serverId,
 		InstanceType: instanceType,
@@ -403,7 +436,7 @@ func modifyServer(cmd *cobra.Command, args []string) error {
 		Storage:      storage,
 	}
 
-	switch instanceType {
+	switch *instanceType {
 	case "cpu":
 		req.CPUModel = cpuModel
 	case "gpu":