Functionality for the 2020-12-01 Compute Rest API release #18498

Sandido · 2021-02-05T21:41:11Z

Most of the Swagger edits are here: Azure/azure-rest-api-specs#12334

This PR introduced the VMGuestPatch breaking changes. It was approved by Azure Breaking Change Review board as the feature is in preview.

Notable Changes:

CloudServices is not part of this release, they will stay in the -preview releases, 43.0.0.0-preview.1. We discussed this plan with Allen and Mark since we do not want to include a preview feature in a stable release.
ContainerService is being removed as it moved out of Compute years ago, and was fully deprecated last year after 3 years of notice. https://azure.microsoft.com/en-us/updates/azure-container-service-will-retire-on-january-31-2020/

All SDK Contribution checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

Please open PR in Draft mode if it is:
- Work in progress or not intended to be merged.
- Encountering multiple pipeline failures and working on fixes.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code. (Track 2 only)
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK. Please double check nuget.org current release version.

Additional management plane SDK specific contribution checklist:

Note: Only applies to Microsoft.Azure.Management.[RP] or Azure.ResourceManager.[RP]

Include updated management metadata.
Update AzureRP.props to add/remove version info to maintain up to date API versions.

Management plane SDK Troubleshooting

If this is very first SDK for a services and you are adding new service folders directly under /SDK, please add new service label and/or contact assigned reviewer.
If the check fails at the Verify Code Generation step, please ensure:
- Do not modify any code in generated folders.
- Do not selectively include/remove generated files in the PR.
- Do use generate.ps1/cmd to generate this PR instead of calling autorest directly.
  Please pay attention to the @microsoft.csharp version output after running generate.ps1. If it is lower than current released version (2.3.82), please run it again as it should pull down the latest version,

Old outstanding PR cleanup

Please note:
If PRs (including draft) has been out for more than 60 days and there are no responses from our query or followups, they will be closed to maintain a concise list for our reviewers.

sync forked repo

sync .Net SDK repo to master

* Delete TestVMExtensionOperations.json * Delete TestVMScaleSetExtensions.json

* Update SdkInfo_ComputeManagementClient.cs * Update VirtualMachineRunCommandsOperations.cs * Update VirtualMachineScaleSetVMRunCommandsOperations.cs * Create TestVMExtensionOperations.json * Delete TestVMExtensionOperations.json * Create TestVMExtensionOperations.json * Create TestVMScaleSetExtensions.json

* adding 2020-12-01 compute generated files * update default sku * update tests * test updates * records * tests fixes * all the recorded tests * more update * more update * more update * last test fixes * Updated TestVMIdentitySystemAssignedUserAssigned SDK Test to create User Assigned Identity using SDK (#17861) * Updated TestVMIdentitySystemAssignedUserAssigned SDK Test to create User Assigned MI using MSI SDK * Resolved Merge Conflicts and updated the test Co-authored-by: Pavan Rachapudy <vrachapu@microsoft.com> * Delete AppGW tests (#17977) * Delete VMScaleSetAppGwWithAS.json * Delete VMScaleSetAppGwWithAS_A1V2.json * Delete VMScaleSetAppGwWithAS_SPG_False.json * Delete VMScaleSetAppGwWithAS_Spot.json * Delete VMScaleSetAppGWTests.cs * Delete deprecated ContainerService tests (#17976) * Delete ContainerServiceOperationalTests.cs * Delete ContainerServiceTestsBase.cs * Delete ContainerServiceUpdateTests.cs * Delete TestDCOSOperations.json * Delete TestSwarmOperations.json * Delete TestContainerServiceUpdateOperations.json * test * remove test text Co-authored-by: pavanrachapudy <pavanrachapudy@gmail.com> Co-authored-by: Pavan Rachapudy <vrachapu@microsoft.com> Co-authored-by: Adam Sandor <adsandor@microsoft.com>

* Delete CloudServiceExtensionTests.cs * Delete CloudServiceOperationTests.cs * Delete CloudServiceRoleInstanceTests.cs * Delete CloudServiceScenarioTests.cs * Delete CloudServiceTestsBase.cs * Delete CloudServiceUpdateTests.cs * Delete MultiRole_CreateUpdateGetAndDeleteWithExtension_WorkerAndWebRole.json * Delete MultiRole_CreateUpdateGetAndDeleteWithExtension_WorkerAndWebRole_MultiRoleExtension.json * Delete Test_Create_PowerOff_Start_CloudServiceOperation.json * Delete Test_ListCloudServicesOperation.json * Delete Test_CloudServiceRoleInstanceOperations.json * Delete TestCloudServiceScenarioOperations.json * Delete TestCloudServiceScenarioOperations_DeleteCloudService.json * Delete TestCloudServiceScenarioOperations_ExtensionProfile.json * Delete TestCloudServiceScenarioOperations_InstanceView.json * Delete TestCloudServiceOperations_ScaleIn_ScaleOut.json * Delete TestCloudServiceUpdateOperations.json * Update AzSdk.RP.props * Update AzSdk.RP.props remove containerService * Delete ServiceConfigurationHelpers.cs

Sandido · 2021-02-08T14:55:06Z

I just regenerated the .Net SDK from Master, and it showed no changes to what is currently in this PR. The CI failure seems incorrect.

Sandido · 2021-02-09T21:07:25Z

The CI failure seems incorrect.

check-enforcer · 2021-02-09T21:34:31Z

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, please perform following steps:

For data-plane/track 2 SDKs 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

For track 1 management-plane SDKs

Please open a separate PR and to your service SDK path in this file. Once that PR has been merged, you can re-run the pipeline to trigger the verification.

markcowl · 2021-02-10T00:21:54Z

@Sandido looks like there is no change to management metadata file in /eng/mgmt/. This can cause the ci generation to not use the same commit in the rest-api-specs repo that you are using. Please include the updated management metadata file.

cleanup

c;eamup2

fix build error? removed Plan

resolve errors

markcowl · 2021-02-17T02:35:55Z

...Microsoft.Azure.Management.Compute/src/Customizations/VirtualMachineScaleSetVMsOperations.cs

+        /// <param name='cancellationToken'>
+        /// The cancellation token.
+        /// </param>
+        public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))


Suggested change

public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))

public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders)

Also need to add one more overload:

public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders, CancellationToken cancellationToken)

In this way, calls like:

Delete(string, string, string) // generated overload Delete(string, string, string, headers) // added overload 1 Delete(string, string, string, null, token) // added overload2

All map to exactly one overload

The same applies to begin* below.

To verify, I would suggest a simple test that calls each of the overloads - it should be fairly simple

Ignoring these comments as directed.

markcowl · 2021-02-18T19:39:32Z

sdk/compute/Microsoft.Azure.Management.Compute/src/Customizations/VirtualMachineImage.cs

+        public VirtualMachineImage(string name, string location, string id, IDictionary<string, string> tags, PurchasePlan plan, OSDiskImage osDiskImage, IList<DataDiskImage> dataDiskImages, AutomaticOSUpgradeProperties automaticOSUpgradeProperties, string hyperVGeneration, DisallowedConfiguration disallowed)
+            : base(name, location, id, tags)
+        {
+            Plan = plan;


This looks OK, but I would have thought you would call this(...al parameters) rather than tre-implementing the constructor

markcowl · 2021-02-18T19:43:03Z

...Microsoft.Azure.Management.Compute/src/Customizations/VirtualMachineScaleSetVMsOperations.cs

+        public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders)
+        {
+            // Send request
+            CancellationToken cancellationToken = default(CancellationToken);


Note that, as above, you can call the generated method here, rather than re-implementing the body

@markcowl , is there any special syntax to call the generated method?
Or just call the method again like this: #17341 (comment) ?

markcowl · 2021-02-18T19:44:05Z

...Microsoft.Azure.Management.Compute/src/Customizations/VirtualMachineScaleSetVMsOperations.cs

@@ -281,21 +275,26 @@ public async Task<AzureOperationResponse<CloudServiceRole>> GetWithHttpMessagesA
        /// <return>
        /// A response object containing the response body and response headers.
        /// </return>
-        public async Task<AzureOperationResponse<IPage<CloudServiceRole>>> ListWithHttpMessagesAsync(string resourceGroupName, string cloudServiceName, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))
+        public async Task<AzureOperationResponse> BeginDeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders = null)
        {
            if (resourceGroupName == null)


Especially here, calling the underlying method would be more attractive than duplicating the method body, but I will not block the check-in for this

markcowl · 2021-02-18T19:48:24Z

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

@@ -12,12 +12,10 @@ namespace Microsoft.Azure.Management.Compute.Models
 {

    /// <summary>
-    /// Defines values for CloudServiceUpgradeMode.


Yes, this is a git diff oddity

markcowl · 2021-02-18T19:50:20Z

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

@@ -16,11 +16,7 @@ namespace Microsoft.Azure.Management.Compute.Models
    /// </summary>
    public static class PatchAssessmentState
    {
-        public const string Installed = "Installed";


Is this part of the approved breakign change?

@markcowl , yes, the other values were removed here: https://github.com/Azure/azure-rest-api-specs/pull/11768/files#diff-221803bb4dd158b19c43e5b4bc886f69fcf8529ff1c195063f6a8b0840faf724L7860

markcowl · 2021-02-18T20:18:07Z

sdk/compute/Microsoft.Azure.Management.Compute/tests/DiskRPTests/DiskRPEncryptionTests.cs

-                string testVaultId = @"/subscriptions/0296790d-427c-48ca-b204-8b729bbd8670/resourceGroups/longrunningrg-centraluseuap/providers/Microsoft.KeyVault/vaults/swaggerKeyVault5";
-                string encryptionKeyUri = @"https://swaggerkeyvault5.vault.azure.net/keys/swaggerKey/52317b056a2c49d0b8673a67d072655e";
-                string secretUri = @"https://swaggerkeyvault5.vault.azure.net/secrets/swaggerSecret/0e01a1ab339e40ff9f01ea98ae8ee6b5";
+                string testVaultId = @"/subscriptions/e37510d7-33b6-4676-886f-ee75bcc01871/resourceGroups/RGforSDKtestResources/providers/Microsoft.KeyVault/vaults/testingkvsdk";


This pattern is highly discouraged. It is best if tests do their own setup, bothto minimize code changes when updating the api-version and to reduce the amount of knowledge you need to have about the tests before running them live.

@markcowl , agreed, we were not in charge of these tests and don't currently have the knowledge to change it. The DiskRP team needs to modify this. We will be curating these tests in the future.

markcowl · 2021-02-18T20:19:37Z

sdk/compute/Microsoft.Azure.Management.Compute/tests/ScenarioTests/ImageTests.cs

@@ -48,7 +48,7 @@ public void TestCreateImage_without_DiskEncryptionSet()
            {
                using (MockContext context = MockContext.Start(this.GetType()))
                {
-                    Environment.SetEnvironmentVariable("AZURE_VM_TEST_LOCATION", "FranceCentral");
+                    Environment.SetEnvironmentVariable("AZURE_VM_TEST_LOCATION", "eastus2");


same comment as above

@markcowl , not sure if it is the same issue. It's something that will need to be modified when next re-recorded. We don't have the knowledge to improve it right now.

markcowl · 2021-02-18T20:21:27Z

...ute/Microsoft.Azure.Management.Compute/tests/ScenarioTests/VMInstanceViewPatchStatusTests.cs

@@ -13,8 +13,8 @@ namespace Microsoft.Azure.Management.Compute.Tests.ScenarioTests
 {
    public class VMInstanceViewPatchStatusTests : VMTestBase
    {
-        private const string RgName = "PatchStatusRg";
-        private const string VmName = "testVm";
+        private const string RgName = "RGforSDKtestResources";


Hopefully these are base names using the random name generator. Otherwise, this is an anti-pattern - hard coding names makes rerunning tests a big problem for the next developer. Again, FYI

@markcowl , agreed, we were not in charge of these tests and don't currently have the knowledge to change it. We will be curating these tests in the future.

markcowl · 2021-02-18T20:22:20Z

...ute/Microsoft.Azure.Management.Compute/tests/ScenarioTests/VMInstanceViewPatchStatusTests.cs

@@ -31,28 +31,6 @@ public void GetVMInstanceViewWithPatchStatus()

                Assert.NotNull(vmInstanceView);
                Assert.NotNull(vmInstanceView.PatchStatus);
-                Assert.NotNull(vmInstanceView.PatchStatus.AvailablePatchSummary);


It is dangerous to remove assertions - if the underlyign type has changed, there should be analogous assertiosn for the new type structure

We are doing this for now as we do not have the knowledge to manually setup the test to have these values and complete the test. I will add them back in but commented out.

markcowl · 2021-02-18T20:23:59Z

sdk/compute/Microsoft.Azure.Management.Compute/tests/ScenarioTests/VMTestBase.cs

@@ -161,7 +164,7 @@ protected DiskEncryptionSettings GetEncryptionSettings(bool addKek = false)

        protected string getDefaultDiskEncryptionSetId()
        {
-            return "/subscriptions/0296790d-427c-48ca-b204-8b729bbd8670/resourceGroups/longrunning-eastus2RG/providers/Microsoft.Compute/diskEncryptionSets/longlivedBvtDES";
+            return "/subscriptions/e37510d7-33b6-4676-886f-ee75bcc01871/resourceGroups/RGforSDKtestResources/providers/Microsoft.Compute/diskEncryptionSets/EncryptionSetforTest";


Not sure what this is for. This seems to be bypassing the test recording system, and is another thing the next dev will have to change when the api-version is updated again.

I think this is due to us not knowing how to create a DiskEncryptionSetId dynamically. Similar issue to above. This needs to be changed when tests are re-recorded. These tests will be curated and improved in the future.

customizations

additional customizations

Small fixes to swagger in Microsoft.Sql AdvancedThreatProtectionSettings API for 2021-11-01-preview (Azure#18498)

Sandido and others added 13 commits October 1, 2020 16:09

Merge pull request #1 from Azure/master

f2a583e

sync forked repo

Merge pull request #2 from Azure/master

4ece171

sync .Net SDK repo to master

Delete test json files (#18003)

5988450

* Delete TestVMExtensionOperations.json * Delete TestVMScaleSetExtensions.json

new recording files

cf303a0

2020-12 release features generated, tests pass

7ef6f13

Update DiskRPTestsBase.cs (#18355)

ca9fb7b

sync with master, worried about some tests

3b5b0b4

regenerate on released 2020-12-01 swagger

d63c9b4

re-recorded test jsons

388463c

test fixes

beb6c2c

ghost added the Compute label Feb 5, 2021

Sandido marked this pull request as ready for review February 9, 2021 21:07

Sandido requested review from allenjzhang, bquantump, m-nash, markcowl, nisha-bhatia and YalinLi0312 as code owners February 9, 2021 21:07

openapi-assignment-bot bot assigned markcowl Feb 9, 2021

Sandido added 2 commits February 9, 2021 16:24

Update AssemblyInfo.cs

07185a1

Update AzSdk.RP.props

5bc4005

Sandido added 2 commits February 9, 2021 16:43

Update Microsoft.Azure.Management.Compute.csproj

e484b33

Merge branch 'master' into compute-2020-12

5a7be8b

Sandido added 14 commits February 18, 2021 10:00

Update VirtualMachine.cs

5ac81ce

cleanup

Update VirtualMachine.cs

369b5eb

c;eamup2

Update VirtualMachineUpdate.cs

f24f056

fix build error? removed Plan

Update VirtualMachineUpdate.cs

6c567b2

resolve errors

Update VirtualMachine.cs

c8124c1

resolve errors

Update VirtualMachineImage.cs

d3590e4

resolve errors

Update VirtualMachineScaleSet.cs

27f50b0

resolve errors

Update VirtualMachine.cs

82b9336

Update VirtualMachineImage.cs

7928f67

Update VirtualMachineScaleSet.cs

4ff159f

Update VirtualMachineUpdate.cs

1242b7d

Update VirtualMachineScaleSet.cs

93eeed4

Update VirtualMachineScaleSetVMsOperations.cs

406287d

Update VirtualMachineUpdate.cs

a16dc7d

markcowl reviewed Feb 18, 2021

View reviewed changes

markcowl requested changes Feb 18, 2021

View reviewed changes

Sandido added 4 commits February 18, 2021 16:49

Create IVirtualMachineScaleSetVMsOperations.cs

0447def

customizations

Update IVirtualMachineScaleSetVMsOperations.cs

9ffe1af

additional customizations

Create VirtualMachineScaleSetVMsOperationsExtensions.cs

6418f98

Update VMInstanceViewPatchStatusTests.cs

b780c79

markcowl approved these changes Feb 18, 2021

View reviewed changes

markcowl merged commit e8cc86f into master Feb 18, 2021

markcowl deleted the compute-2020-12 branch February 18, 2021 23:40

Sandido restored the compute-2020-12 branch February 19, 2021 15:53

Sandido mentioned this pull request Mar 8, 2021

autorest/C# Add new optional parameters to the end of method signatures Azure/autorest#3957

Closed

ArthurMa1978 deleted the compute-2020-12 branch January 13, 2022 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functionality for the 2020-12-01 Compute Rest API release #18498

Functionality for the 2020-12-01 Compute Rest API release #18498

Sandido commented Feb 5, 2021 •

edited

Loading

Sandido commented Feb 8, 2021

Sandido commented Feb 9, 2021

check-enforcer bot commented Feb 9, 2021

markcowl commented Feb 10, 2021

markcowl Feb 17, 2021

markcowl Feb 17, 2021

markcowl Feb 17, 2021

Sandido Feb 18, 2021

markcowl Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021 •

edited

Loading

markcowl Feb 18, 2021

markcowl Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021 •

edited

Loading

markcowl Feb 18, 2021

Sandido Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021

markcowl Feb 18, 2021

Sandido Feb 18, 2021

	public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))
	public async Task<AzureOperationResponse> DeleteWithHttpMessagesAsync(string resourceGroupName, string vmScaleSetName, string instanceId, Dictionary<string, List<string>> customHeaders)

Functionality for the 2020-12-01 Compute Rest API release #18498

Functionality for the 2020-12-01 Compute Rest API release #18498

Conversation

Sandido commented Feb 5, 2021 • edited Loading

All SDK Contribution checklist:

Additional management plane SDK specific contribution checklist:

Management plane SDK Troubleshooting

Old outstanding PR cleanup

Sandido commented Feb 8, 2021

Sandido commented Feb 9, 2021

check-enforcer bot commented Feb 9, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

For data-plane/track 2 SDKs Issue the following command as a pull request comment:

For track 1 management-plane SDKs

markcowl commented Feb 10, 2021

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

Sandido Feb 18, 2021 • edited Loading

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

Sandido Feb 18, 2021 • edited Loading

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

Sandido commented Feb 5, 2021 •

edited

Loading

Sandido Feb 18, 2021 •

edited

Loading

Sandido Feb 18, 2021 •

edited

Loading