Azure-sdk-for-go: Compute Cluster operation does not work in Machinelearningservices version 2020-04-01

[gro1m]

Bug Report

• import path of package in question: "github.com/Azure/azure-sdk-for-go/services/machinelearningservices/mgmt/2020-04-01/machinelearningservices"
• SDK version: v54.2.0+incompatible
• output of go version: 1.16.3

• What happened?
  Tried to update scale settings of compute cluster -> did not work.
• What did you expect or want to happen?
  Successful update of scale settings of compute cluster.
• How can we reproduce it?
  Try updating of compute cluster with Azure SDK for Go.
• Anything we should know about your environment.
  Blocks my PR for Terraform Azure Provider: https://github.com/terraform-providers/terraform-provider-azurerm/pull/11675
  Related to feature request: https://github.com/Azure/azure-sdk-for-go/issues/14666.
  I assume updating the version to 2021-04-01 would fix this, as the response sent through the portal has a slightly different structure than what the Go SDK sends.

Azure portal payload:

{"properties":
    {"properties":
        {"scaleSettings":
            {"minNodeCount":2,
              "maxNodeCount":3,
              "nodeIdleTimeBeforeScaleDown":"PT900S"
             }
         }
     }
}

Go SDK payload:

{"properties":
    {"scaleSettings":
        {"maxNodeCount":2,
         "minNodeCount":1,
         "nodeIdleTimeBeforeScaleDown":"PT60S"
        }
     }
}

[ArcturusZhang]

Hi @gro1m thanks for opening this issue!

Unfortunately this schema error comes from the corresponding swagger here: https://github.com/Azure/azure-rest-api-specs/blob/03a759c3b3c86cecc1c6030cc5e4d6940437706a/specification/machinelearningservices/resource-manager/Microsoft.MachineLearningServices/stable/2021-04-01/machineLearningServices.json#L3942
Please note this comes from the latest stable api-version 2021-04-01, which means releasing a new api-version cannot solve this.
Therefore I have tagged this issue so that the corresponding service team could take a look and fix their swagger or change their payload format so that the current swagger could work.
In the meantime, we must pause the release of a new api-version 2021-04-01 as requested in this issue

[msftbot[bot]]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @azureml-github.

Issue Details

---

Bug Report

• import path of package in question: "github.com/Azure/azure-sdk-for-go/services/machinelearningservices/mgmt/2020-04-01/machinelearningservices"
• SDK version: v54.2.0+incompatible
• output of go version: 1.16.3

• What happened?
  Tried to update scale settings of compute cluster -> did not work.
• What did you expect or want to happen?
  Successful update of scale settings of compute cluster.
• How can we reproduce it?
  Try updating of compute cluster with Azure SDK for Go.
• Anything we should know about your environment.
  Blocks my PR for Terraform Azure Provider: https://github.com/terraform-providers/terraform-provider-azurerm/pull/11675
  Related to feature request: https://github.com/Azure/azure-sdk-for-go/issues/14666.
  I assume updating the version to 2021-04-01 would fix this, as the response sent through the portal has a slightly different structure than what the Go SDK sends.

Azure portal payload:

{"properties":
    {"properties":
        {"scaleSettings":
            {"minNodeCount":2,
              "maxNodeCount":3,
              "nodeIdleTimeBeforeScaleDown":"PT900S"
             }
         }
     }
}

Go SDK payload:

{"properties":
    {"scaleSettings":
        {"maxNodeCount":2,
         "minNodeCount":1,
         "nodeIdleTimeBeforeScaleDown":"PT60S"
        }
     }
}

Author:	gro1m
Assignees:	-
Labels:	`Machine Learning Compute`, `Service Attention`, `bug`, `customer-reported`, `needs-triage`
Milestone:	-

[favoretti]

@ArcturusZhang Hmm, the swagger does show double property level, that's the payload that actually works (i.e. the one portal sends in the issue). When I use REST API UI - that payload works with any API version. How come Go SDK marshals it incorrectly though?

[ArcturusZhang]

@ArcturusZhang Hmm, the swagger does show double property level, that's the payload that actually works (i.e. the one portal sends in the issue). When I use REST API UI - that payload works with any API version. How come Go SDK marshals it incorrectly though?

The swagger does not have the double properties. Please see the comment below

    "ClusterUpdateParameters": {
      "properties": { # This `properties` is a keyword, indicating the following are a map of the properties.
        "properties": { # This is the first property of ClusterUpdateParameters, it happens to have a name `properties`.
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/ClusterUpdateProperties",
          "description": "The properties of the amlCompute."
        }
      },
      "description": "AmlCompute update parameters."
    },

If we want something with double properties in the swagger, we will have to write this:

    "ClusterUpdateParameters": {
      "properties": {
        "properties": {
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/ClusterUpdateParametersProperties",
          "description": "The properties of the amlCompute."
        }
      },
      "description": "AmlCompute update parameters."
    },
    "ClusterUpdateParametersProperties": {
      "properties": {
        "properties": {
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/ClusterUpdateProperties",
          "description": "The properties of the amlCompute."
        }
      },
      "description": "AmlCompute update parameters."
    },

Or if you find the name properties is confusing, you could try to write a swagger schema with two levels of something and it should looks like this:

"ClusterUpdateParameters": {
      "properties": {
        "something": {
          "$ref": "#/definitions/ClusterUpdateParametersProperties",
          "description": "The properties of the amlCompute."
        }
      },
      "description": "AmlCompute update parameters."
    },
    "ClusterUpdateParametersProperties": {
      "properties": {
        "something": {
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/ClusterUpdateProperties",
          "description": "The properties of the amlCompute."
        }
      },
      "description": "AmlCompute update parameters."
    }

[favoretti]

Oh nevermind, I should learn to read swagger in the morning. Yep, thank you very much for the detailed explanation. I indeed got confused by the "properties" keyword of swagger itself.

[ArcturusZhang]

Oh nevermind, I should learn to read swagger in the morning. Yep, thank you very much for the detailed explanation. I indeed got confused by the "properties" keyword of swagger itself.

I have written some internal emails to inform the service team about this bug, waiting for their responses now.