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.

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

mgmt, azure migrate test proxy #36103

Merged

Conversation

weidongxu-microsoft
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft commented Jul 26, 2023

There is a few tests take too long, or being too complex to get fixed in this PR.

Temporary skip the playback on them via @DoNotRecord(skipInPlayback = true) // TODO(weidxu)
Include LoadBalancer and VirtualNetworkGateway.

Will re-enable them in following month.

@weidongxu-microsoft weidongxu-microsoft added Mgmt This issue is related to a management-plane library. test-proxy Anything relating to test-proxy requests or issues. labels Jul 26, 2023
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Comment on lines +22 to +23
private final String templateUri = "https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/quickstarts/microsoft.storage/storage-account-create/azuredeploy.json";
private final String parametersUri = "https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/quickstarts/microsoft.storage/storage-account-create/azuredeploy.parameters.json";
Copy link
Member Author

Choose a reason for hiding this comment

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

use storage ARM template

.defineRequestRoutingRule("rule1")
.fromPrivateFrontend()
.fromPublicFrontend()
Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Jul 26, 2023

Choose a reason for hiding this comment

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

V1 deprecated
V2 only allow public endpoint

Comment on lines +156 to 157
.withAzureActiveDirectoryGroup(group.id())
.disableLocalAccounts()
Copy link
Member Author

Choose a reason for hiding this comment

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

AKS requires having AAD when disable local accounts.

Comment on lines +179 to +203
// // update private endpoint
// StorageAccount storageAccount2 = azureResourceManager.storageAccounts().define(saName2)
// .withRegion(region)
// .withNewResourceGroup(rgName)
// .create();
//
// privateEndpoint.update()
// .updatePrivateLinkServiceConnection(pecName)
// .withRequestMessage("request2")
// .parent()
// .apply();
//
// Assertions.assertEquals("Pending", privateEndpoint.privateLinkServiceConnections().get(pecName).state().status());
// Assertions.assertEquals("request2", privateEndpoint.privateLinkServiceConnections().get(pecName).requestMessage());
//
// privateEndpoint.update()
// .withoutPrivateLinkServiceConnection(pecName)
// .definePrivateLinkServiceConnection(pecName2)
// .withResource(storageAccount2)
// .withSubResource(PrivateLinkSubResourceName.STORAGE_FILE)
// .attach()
// .apply();
//
// Assertions.assertEquals(Collections.singletonList(PrivateLinkSubResourceName.STORAGE_FILE), privateEndpoint.privateLinkServiceConnections().get(pecName2).subResourceNames());
// Assertions.assertEquals("Approved", privateEndpoint.privateLinkServiceConnections().get(pecName2).state().status());
Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Jul 26, 2023

Choose a reason for hiding this comment

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

this part "update a private endpoint connection" seems no longer allowed (or not allowed by storage). may do more investigation later.

Comment on lines -290 to -294
// Start the creation...
creationThread.start();

// ...But bail out after 30 sec, as it is enough to test the results
creationThread.join();
Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Jul 26, 2023

Choose a reason for hiding this comment

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

no idea why previously use thread...

config.put("StrictHostKeyChecking", "no");
// jsch.addIdentity(sshFile, filePassword);
session = jsch.getSession("testuser", publicIpDnsLabel + "." + "eastus.cloudapp.azure.com", 22);
session.setPassword(password);
Copy link
Member Author

Choose a reason for hiding this comment

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

Delete this SSH part. No idea why create VM with SSH then use password to connect.

@weidongxu-microsoft weidongxu-microsoft marked this pull request as ready for review July 26, 2023 12:13
@Test
public void testAppGatewaysPublicUrlPathBased() throws Exception {
new TestApplicationGateway().new UrlPathBased(azureResourceManager.resourceGroups().manager().internalContext())
.runTest(azureResourceManager.applicationGateways(), azureResourceManager.resourceGroups());
}

@Disabled("WAF_V2 currently does not support private endpoint, and we do not want to have accessible public IP on VM. Enable it after WAF_V2 supports private endpoint.")
Copy link
Member

Choose a reason for hiding this comment

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

should this have a link to a tracking github issue

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Issue logged #36122

@joshfree joshfree requested a review from samvaity July 26, 2023 16:40
@weidongxu-microsoft weidongxu-microsoft merged commit 2fca963 into Azure:main Jul 27, 2023
@weidongxu-microsoft weidongxu-microsoft deleted the mgmt_azure-test-proxy branch July 27, 2023 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library. test-proxy Anything relating to test-proxy requests or issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants