Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vmss: support for passing a health probe to update #7355

Merged
merged 1 commit into from
Oct 10, 2019
Merged

vmss: support for passing a health probe to update #7355

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions specification/compute/resource-manager/Microsoft.Compute/stable/2019-03-01/compute.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8435,6 +8435,10 @@
},
"VirtualMachineScaleSetUpdateNetworkProfile": {
"properties": {
"healthProbe": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, please include this in a new api version

See 'New property added to response':
https://microsoft.sharepoint.com/:w:/t/azureresourcemanagerteam/EWXsAQ1yx25KkyYCeeWGUgwBSBxEdUDEbHi6FZ__U8EOQw?e=xEcppo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryansbenson this is in the existing API version - it's just missing from the Swagger - please see the examples in the PR description

In addition it appears that link isn't publicaly available:

Screenshot 2019-10-01 at 10 31 10

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryansbenson since this is already in the API (but is just missing from the Swagger) I'm assuming adding this optional property to an update (to match the API definition) should be fine?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tombuildsstuff for your information, the doc above lists various types of breaking changes, which contains on entry of

New property added to response 

If a new property/field is added to the response an API, the GET-PUT pipeline will be broken. Consider the case where from portal a customer updates the value of a new property "A". Another customer does a GET of this resource using the SDK. The SDK will ignore the property since it does not understand it. From the SDK, the customer does a PUT using the model that was returned from the GET. This will overwrite the change made by the first customer from the portal.

Per this entry, the addition of this property is a breaking change to the existing swagger.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ryansbenson per what @tombuildsstuff claims, this property has already existed in the API but just missing in swagger, will this condition needs a new api version?
More to ask, would proposing a new api version be done by customer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the live api version accepts and supports this property, it doesn't constitute a breaking api version change (as it already exists in the api version). This is essentially a documentation issue. If these properties came after the api version was released and we're trying to document it now, your customers have already gone through the side affects of the breaking change (this isn't a good experience but not something we can solve in swagger PRs).

Approving from ARMs side

"$ref": "#/definitions/ApiEntityReference",
"description": "A reference to a load balancer probe used to determine the health of an instance in the virtual machine scale set. The reference will be in the form: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/loadBalancers/{loadBalancerName}/probes/{probeName}'."
},
"networkInterfaceConfigurations": {
"type": "array",
"items": {
Expand Down