Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests #16844

shpa-microsoft · 2020-11-10T22:33:58Z

Api pull request Azure/azure-rest-api-specs#11409

Do not merge, this PR is to confirm that .Net SDK code can be generated from Swagger change.

check-enforcer · 2020-11-10T22:41:01Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run net - [service] - ci

markcowl

The code changes look OK, but there are some things that are missing:

Please add in a reference to the corresponding Swagger PR for this change
running the generation script produces an updated metadata file in the eng directory, please add this in to the PR
When api versions change, you need to update Az.Sdk.RP.props to change the tag to match the new api-version
You sill need to re-record the tests and add in new tests for any new functionality

markcowl · 2020-11-14T01:30:46Z

@shpa-microsoft Also, please ensure you update this branch by pulling in the latest from master.

shpa-microsoft · 2020-11-16T21:00:43Z

@markcowl Addressed your comments with some questions on offline chat.

shpa-microsoft · 2020-11-17T19:19:41Z

Can't perform tests due to ARM API release issues. Spoke offline about it.

jsquire · 2020-12-11T14:10:10Z

Hi @shpa-microsoft. There hasn't been recent engagement on this PR. Would you please be so kind as to let us know if this is still an active work stream or if the PR should be closed out?

markcowl · 2020-12-16T20:09:25Z

sdk/compute/Microsoft.Azure.Management.Compute/src/Generated/GalleriesOperations.cs

@@ -134,7 +138,7 @@ public async Task<AzureOperationResponse<Gallery>> UpdateWithHttpMessagesAsync(s
        /// <return>
        /// A response object containing the response body and response headers.
        /// </return>
-        public async Task<AzureOperationResponse<Gallery>> GetWithHttpMessagesAsync(string resourceGroupName, string galleryName, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))
+        public async Task<AzureOperationResponse<Gallery>> GetWithHttpMessagesAsync(string resourceGroupName, string galleryName, string select = default(string), Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))


Need a customization to fix this breaking change - essentially you need a method that implements the old parameters, with 'customHeaders' being non-optional, like the following:

public partial class GalleriesOperations { public async Task<AzureOperationResponse<Gallery>> GetWithHttpMessagesAsync(string resourceGroupName, string galleryName, Dictionary<string, List<string>> customHeaders, CancellationToken cancellationToken = default(CancellationToken)) { return GetWithHttpMessagesAsync(resourceGroupName, galleryName, null, customHeaders, cancellationToken); } }

markcowl · 2020-12-16T20:10:06Z

sdk/compute/Microsoft.Azure.Management.Compute/src/Generated/GalleriesOperationsExtensions.cs

            /// <param name='cancellationToken'>
            /// The cancellation token.
            /// </param>
-            public static async Task<Gallery> GetAsync(this IGalleriesOperations operations, string resourceGroupName, string galleryName, CancellationToken cancellationToken = default(CancellationToken))
+            public static async Task<Gallery> GetAsync(this IGalleriesOperations operations, string resourceGroupName, string galleryName, string select = default(string), CancellationToken cancellationToken = default(CancellationToken))


same comment as above

markcowl · 2020-12-16T20:14:09Z

...soft.Azure.Management.Compute/src/Generated/IVirtualMachineScaleSetVMExtensionsOperations.cs

@@ -57,7 +57,7 @@ public partial interface IVirtualMachineScaleSetVMExtensionsOperations
        /// <exception cref="Microsoft.Rest.ValidationException">
        /// Thrown when a required parameter is null
        /// </exception>
-        Task<AzureOperationResponse<VirtualMachineExtension>> CreateOrUpdateWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, string vmExtensionName, VirtualMachineExtension extensionParameters, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken));


need to compensate for these breaking changes:

(1) ensure that the return type has all the properties of the previous return type
(2) add method overloads through customizing thre partial interface using the old signature

markcowl · 2020-12-16T20:14:40Z

sdk/compute/Microsoft.Azure.Management.Compute/src/Generated/IVirtualMachinesOperations.cs

@@ -154,7 +157,7 @@ public partial interface IVirtualMachinesOperations
        /// <exception cref="Microsoft.Rest.ValidationException">
        /// Thrown when a required parameter is null
        /// </exception>
-        Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmName, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken));
+        Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmName, bool? forceDeletion = default(bool?), Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken));


Need to add overloads as above to fix these breakign changes

markcowl · 2020-12-16T20:15:47Z

....Azure.Management.Compute/src/Generated/Models/GalleryApplicationVersionPublishingProfile.cs

-        [JsonProperty(PropertyName = "contentType")]
-        public string ContentType { get; set; }
+        [JsonProperty(PropertyName = "manageActions")]
+        public UserArtifactManage ManageActions { get; set; }


This is a breaking change - you will have to maintain a ContentType property, even if unused

markcowl · 2020-12-16T20:19:53Z

sdk/compute/Microsoft.Azure.Management.Compute/src/Generated/Models/UserArtifactSource.cs

-        /// Gets or sets required. The fileName of the artifact.
-        /// </summary>
-        [JsonProperty(PropertyName = "fileName")]
-        public string FileName { get; set; }


Cannot trmoeve this property - must maintain the property

markcowl · 2020-12-16T20:22:53Z

...osoft.Azure.Management.Compute/src/Generated/VirtualMachineScaleSetVMExtensionsOperations.cs

@@ -74,10 +74,10 @@ internal VirtualMachineScaleSetVMExtensionsOperations(ComputeManagementClient cl
        /// <param name='cancellationToken'>
        /// The cancellation token.
        /// </param>
-        public async Task<AzureOperationResponse<VirtualMachineExtension>> CreateOrUpdateWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, string vmExtensionName, VirtualMachineExtension extensionParameters, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))
+        public async Task<AzureOperationResponse<VirtualMachineScaleSetVMExtension>> CreateOrUpdateWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, string vmExtensionName, VirtualMachineScaleSetVMExtension extensionParameters, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))


Same issues as above:
(1) Need to verify that return tyoe contains all the properties of the previous return type
(2) Need to add a customization to implement the old parameter set, as above.

This applies for all the similar breaking method signature changes in this file

markcowl · 2020-12-16T20:23:44Z

...e.Management.Compute/src/Generated/VirtualMachineScaleSetVMExtensionsOperationsExtensions.cs

@@ -42,7 +42,7 @@ public static partial class VirtualMachineScaleSetVMExtensionsOperationsExtensio
            /// <param name='extensionParameters'>
            /// Parameters supplied to the Create Virtual Machine Extension operation.
            /// </param>
-            public static VirtualMachineExtension CreateOrUpdate(this IVirtualMachineScaleSetVMExtensionsOperations operations, string resourceGroupName, string vmScaleSetName, string instanceId, string vmExtensionName, VirtualMachineExtension extensionParameters)
+            public static VirtualMachineScaleSetVMExtension CreateOrUpdate(this IVirtualMachineScaleSetVMExtensionsOperations operations, string resourceGroupName, string vmScaleSetName, string instanceId, string vmExtensionName, VirtualMachineScaleSetVMExtension extensionParameters)


Same as in previous file - need to add customizations to fix these breaking changes

markcowl · 2020-12-16T20:24:28Z

...pute/Microsoft.Azure.Management.Compute/src/Generated/VirtualMachinesOperationsExtensions.cs

            /// <param name='cancellationToken'>
            /// The cancellation token.
            /// </param>
-            public static async Task DeleteAsync(this IVirtualMachinesOperations operations, string resourceGroupName, string vmName, CancellationToken cancellationToken = default(CancellationToken))
+            public static async Task DeleteAsync(this IVirtualMachinesOperations operations, string resourceGroupName, string vmName, bool? forceDeletion = default(bool?), CancellationToken cancellationToken = default(CancellationToken))


need to add an overload to fix this breaking change

markcowl · 2020-12-16T20:24:47Z

...pute/Microsoft.Azure.Management.Compute/src/Generated/VirtualMachinesOperationsExtensions.cs

            /// <param name='cancellationToken'>
            /// The cancellation token.
            /// </param>
-            public static async Task BeginDeleteAsync(this IVirtualMachinesOperations operations, string resourceGroupName, string vmName, CancellationToken cancellationToken = default(CancellationToken))
+            public static async Task BeginDeleteAsync(this IVirtualMachinesOperations operations, string resourceGroupName, string vmName, bool? forceDeletion = default(bool?), CancellationToken cancellationToken = default(CancellationToken))


need to add an overload to fix this breaking change

markcowl · 2020-12-16T20:25:42Z

@shpa-microsoft Marked the breaking changes that need to be fixed through customization

markcowl · 2021-01-06T01:16:29Z

@shpa-microsoft It looks like this PR is out of date, can you fix the merge conflicts?

shpa-microsoft · 2021-01-06T01:36:58Z

@markcowl all the comments made for breaking changes are unrelated to my changes; and generated automatically with autorest. Is there a way I only regenerate my code for this PR?

markcowl · 2021-01-12T22:31:16Z

@shpa-microsoft There is a discussion about these API changes, and a proposed resolution on the compute team. Please contact @Sandido for details.

shpa-microsoft requested review from allenjzhang, bquantump, m-nash, markcowl, nisha-bhatia and YalinLi0312 as code owners November 10, 2020 22:33

openapi-assignment-bot bot assigned markcowl Nov 10, 2020

ghost added the Compute label Nov 10, 2020

shpa-microsoft changed the title ~~Customer Assigned Fault Domain on VM referencing VMSS - SDK Generation~~ [DO-NOT-MERGE] Customer Assigned Fault Domain on VM referencing VMSS - SDK Generation Nov 10, 2020

Sandido added the Do Not Merge label Nov 11, 2020

markcowl requested changes Nov 14, 2020

View reviewed changes

markcowl added the needs-revision label Nov 14, 2020

markcowl assigned shpa-microsoft Nov 14, 2020

shpa-microsoft force-pushed the shpa/CustomerAssignedFaultDomain branch from 32f4839 to de7685e Compare November 16, 2020 20:58

shpa-microsoft requested a review from markcowl November 18, 2020 16:28

kimberlyyipc mentioned this pull request Nov 19, 2020

SDK PR for Add enableCrossZoneUpgrade and prioritizeUnhealthyInstances properties to RollingUpgradePolicy for VMSS #16881

Closed

jsquire added the no-recent-activity There has been no recent activity on this issue. label Dec 11, 2020

markcowl requested changes Dec 16, 2020

View reviewed changes

jsquire removed the no-recent-activity There has been no recent activity on this issue. label Dec 18, 2020

shpa-microsoft force-pushed the shpa/CustomerAssignedFaultDomain branch from de7685e to 061b946 Compare January 5, 2021 21:03

shpa-microsoft changed the title ~~[DO-NOT-MERGE] Customer Assigned Fault Domain on VM referencing VMSS - SDK Generation~~ Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests Jan 5, 2021

Customer Assigned Fault Domain on VM referencing VMSS - SDK Generation

014a747

Customer Assigned Fault Domain on VM referencing VMSS - Tests

25285c7

shpa-microsoft force-pushed the shpa/CustomerAssignedFaultDomain branch from 061b946 to 25285c7 Compare January 6, 2021 01:55

shpa-microsoft requested a review from markcowl January 7, 2021 20:07

allenjzhang self-assigned this Jan 21, 2021

markcowl removed their assignment Jan 25, 2021

allenjzhang closed this Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests #16844

Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests #16844

shpa-microsoft commented Nov 10, 2020

check-enforcer bot commented Nov 10, 2020

markcowl left a comment

markcowl commented Nov 14, 2020

shpa-microsoft commented Nov 16, 2020

shpa-microsoft commented Nov 17, 2020

jsquire commented Dec 11, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl Dec 16, 2020

markcowl commented Dec 16, 2020

markcowl commented Jan 6, 2021

shpa-microsoft commented Jan 6, 2021

markcowl commented Jan 12, 2021

Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests #16844

Customer Assigned Fault Domain on VM referencing VMSS - SDK and Tests #16844

Conversation

shpa-microsoft commented Nov 10, 2020

check-enforcer bot commented Nov 10, 2020

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

markcowl left a comment

Choose a reason for hiding this comment

markcowl commented Nov 14, 2020

shpa-microsoft commented Nov 16, 2020

shpa-microsoft commented Nov 17, 2020

jsquire commented Dec 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markcowl commented Dec 16, 2020

markcowl commented Jan 6, 2021

shpa-microsoft commented Jan 6, 2021

markcowl commented Jan 12, 2021