virtual network manager - New Terraform Template - Quickstart Create virtual network manager with Management Group Scope #298

mbender-ms · 2023-11-16T19:28:39Z

No description provided.

quickstart/101-virtual-network-manager-create-management-group-scope/readme.md

quickstart/101-virtual-network-manager-create-management-group-scope/providers.tf

lonegunmanb

Thanks @mbender-ms for opening this pr! Some comments.

quickstart/101-virtual-network-manager-create-management-group-scope/main.tf

quickstart/101-virtual-network-manager-create-management-group-scope/providers.tf

mbender-ms · 2023-11-17T15:32:40Z

@TomArcherMsft I re-using what was in the current template. I did not re-create this from scratch based off of the templates recommended in the updated guide.

As I recall, those values were specified to me by someone on the terraform team during the creatin process of the original. I did

I've updated to use the following:

version = "~> 3.56.0, < 4.0"

mbender-ms

Applied all review suggestions.

TomArcherMsft · 2023-11-20T18:47:58Z

@mbender-ms

If you're going to explicitly state a min & max, then you don't need the pessimistic operator. Personally, I'd use "~>3.56" (without the max), which states min=3.56 and max is anything less than 4.

However, the way you have it now (and had it before) both function. Therefore, for me, this is not a blocking issue.

@lonegunmanb @stemaMSFT I believe that @grayzu is OOF for Thanksgiving. Could one of you merge this PR?

lonegunmanb · 2023-11-21T02:06:50Z

@mbender-ms

If you're going to explicitly state a min & max, then you don't need the pessimistic operator. Personally, I'd use "~>3.56" (without the max), which states min=3.56 and max is anything less than 4.

However, the way you have it now (and had it before) both function. Therefore, for me, this is not a blocking issue.

@lonegunmanb @stemaMSFT I believe that @grayzu is OOF for Thanksgiving. Could one of you merge this PR?

Sure. Yesterday this e2e test failed because we don't have az cli in our testing container, so I upgraded the container image. I'll rerun the test now @TomArcherMsft @mbender-ms .

lonegunmanb · 2023-11-21T06:17:22Z

We've got the following error:

│ Error running command '         az provider register --namespace
        	            	│ 'Microsoft.Network' -m 5ef28019-2c13-43a9-83dc-b4568723ec6c
        	            	│ ': exit status 1. Output: ERROR: Please run 'az login' to setup account.
        	            	│ 
        	            	╵}

I'll see what we can do to fix this issue.

mbender-ms

approved

mbender-ms · 2023-11-28T02:57:59Z

@lonegunmanb It looks like pre-check is now failing. Any ideas?

lonegunmanb · 2023-11-28T09:41:30Z

@lonegunmanb It looks like pre-check is now failing. Any ideas?

Hi @mbender-ms it looks like your branch is outdated, would you please rebase your branch with the latest master branch and try again? Thanks!

mbender-ms · 2023-11-28T18:13:44Z

#please-close

mbender-ms · 2023-11-28T18:13:58Z

#please-open

mbender-ms · 2023-11-29T16:01:53Z

@TomArcherMsft @lonegunmanb Anything I'm missing to get this rolling? My branch is updated. Looks like there is still a change request but that has been completed. Thanks!

stemaMSFT · 2023-11-29T19:56:23Z

@mbender-ms looks like the prepr-check is failing, if you could get that resolved I can kick off tests again.

mbender-ms · 2023-11-29T20:18:49Z

@stemaMSFT Looks like I was missing some quotes. Fixed that so let's try again!

stemaMSFT · 2023-11-30T00:21:07Z

@mbender-ms the e2e tests failed here, take a look with Tom if needed and resolve and we can re-kick off the tests

TomArcherMsft · 2023-11-30T02:12:36Z

@mbender-ms the e2e tests failed here, take a look with Tom if needed and resolve and we can re-kick off the tests

Unfortunately, I know nothing about management groups. If Michael doesn't know, maybe @lonegunmanb can help.

lonegunmanb · 2023-11-30T02:37:26Z

Hi @mbender-ms @TomArcherMsft , I want explain this testing error and my solution step by step here, so we can fix this issue and get this pr merged.

The testing error

╷
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ Error: local-exec provisioner error
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ 
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │   with null_resource.register_rp_to_mg,
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │   on main.tf line 61, in resource "null_resource" "register_rp_to_mg":
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │   61:   provisioner "local-exec" {
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ 
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ Error running command '         az provider register --namespace
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ 'Microsoft.Network' -m 43b729bc-8452-44b2-a53a-fb3de05bee86
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ ': exit status 1. Output: ERROR: Please run 'az login' to setup account.
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: │ 
Test_Quickstarts/quickstart/101-virtual-network-manager-create-management-group-scope 2023-11-30T00:18:34Z command.go:185: ╵

Root cause: Please run 'az login' to setup account.

Because we use az cli innull_resource's provisioner, this error complained that we must execute az login first.

How can we `az login` in e2e test?

I've tried to fix this pr and I've passed the test, my branch is: https://github.com/lonegunmanb/azterraform/tree/fix-vnet-manager-group, you can check my attempt pr at #300 . In this pr you'll see This branch was successfully deployed, which means it passed the test.

I just rebased Michael's branch with the latest master branch, and all my fixes are in this commit: lonegunmanb@b18220b

I've created a special sub-test method for this example since it requires az login as testing setup: lonegunmanb@b18220b#diff-7e867c4be98de0c4deb27c03c13de85fc7383a011fcb50a9058232dedfde2b64R146-R175 , I executed az login with required flags before the test, and az logout after the test. I also added a discardWriter type to discard all stdout from az login to avoid potential secret leaks.

How to reuse my code

You can pull the branch from my repo, I have rebased it with the latest master branch, you can just force push to your branch, then try again. Please feel free to contact me if you have any further questions @mbender-ms .

mbender-ms · 2023-11-30T20:36:30Z

@lonegunmanb Thanks for your help on this.

I'm currently unable to follow your steps to pull your repo into my branch. I'm nuking my local and going to see if that fixes things. I'll keep you posted.

mbender-ms · 2023-11-30T20:51:47Z

@lonegunmanb Force pushed your changes into my branch. Please let me know if this fixes it and lets PR pass.

stemaMSFT · 2023-11-30T21:56:40Z

@mbender-ms thanks to your efforts, this PR is merged!

TomArcherMsft reviewed Nov 16, 2023

View reviewed changes

quickstart/101-virtual-network-manager-create-management-group-scope/readme.md Outdated Show resolved Hide resolved

TomArcherMsft reviewed Nov 16, 2023

View reviewed changes

quickstart/101-virtual-network-manager-create-management-group-scope/providers.tf Outdated Show resolved Hide resolved

lonegunmanb requested changes Nov 17, 2023

View reviewed changes

quickstart/101-virtual-network-manager-create-management-group-scope/main.tf Show resolved Hide resolved

quickstart/101-virtual-network-manager-create-management-group-scope/providers.tf Outdated Show resolved Hide resolved

mbender-ms temporarily deployed to acctests November 20, 2023 01:51 — with GitHub Actions Inactive

mbender-ms commented Nov 20, 2023

View reviewed changes

mbender-ms temporarily deployed to acctests November 21, 2023 02:05 — with GitHub Actions Inactive

mbender-ms commented Nov 27, 2023

View reviewed changes

mbender-ms requested a review from lonegunmanb November 28, 2023 18:16

mbender-ms temporarily deployed to acctests November 30, 2023 00:12 — with GitHub Actions Inactive

Michael Bender and others added 4 commits November 30, 2023 10:39

update terraform

75c6fa2

finished testing

3e66747

updates from Review

5895b4f

fix example and e2e test

99037bb

mbender-ms force-pushed the avnm-create-management-group-scope branch from 32d9cca to 99037bb Compare November 30, 2023 20:47

mbender-ms temporarily deployed to acctests November 30, 2023 21:01 — with GitHub Actions Inactive

stemaMSFT merged commit 73a62df into Azure:master Nov 30, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

virtual network manager - New Terraform Template - Quickstart Create virtual network manager with Management Group Scope #298

virtual network manager - New Terraform Template - Quickstart Create virtual network manager with Management Group Scope #298

mbender-ms commented Nov 16, 2023

lonegunmanb left a comment

mbender-ms commented Nov 17, 2023

mbender-ms left a comment

TomArcherMsft commented Nov 20, 2023 •

edited

Loading

lonegunmanb commented Nov 21, 2023

lonegunmanb commented Nov 21, 2023

mbender-ms left a comment

mbender-ms commented Nov 28, 2023

lonegunmanb commented Nov 28, 2023

mbender-ms commented Nov 28, 2023

mbender-ms commented Nov 28, 2023

mbender-ms commented Nov 29, 2023

stemaMSFT commented Nov 29, 2023

mbender-ms commented Nov 29, 2023

stemaMSFT commented Nov 30, 2023

TomArcherMsft commented Nov 30, 2023

lonegunmanb commented Nov 30, 2023 •

edited

Loading

mbender-ms commented Nov 30, 2023

mbender-ms commented Nov 30, 2023

stemaMSFT commented Nov 30, 2023

virtual network manager - New Terraform Template - Quickstart Create virtual network manager with Management Group Scope #298

virtual network manager - New Terraform Template - Quickstart Create virtual network manager with Management Group Scope #298

Conversation

mbender-ms commented Nov 16, 2023

lonegunmanb left a comment

Choose a reason for hiding this comment

mbender-ms commented Nov 17, 2023

mbender-ms left a comment

Choose a reason for hiding this comment

TomArcherMsft commented Nov 20, 2023 • edited Loading

lonegunmanb commented Nov 21, 2023

lonegunmanb commented Nov 21, 2023

mbender-ms left a comment

Choose a reason for hiding this comment

mbender-ms commented Nov 28, 2023

lonegunmanb commented Nov 28, 2023

mbender-ms commented Nov 28, 2023

mbender-ms commented Nov 28, 2023

mbender-ms commented Nov 29, 2023

stemaMSFT commented Nov 29, 2023

mbender-ms commented Nov 29, 2023

stemaMSFT commented Nov 30, 2023

TomArcherMsft commented Nov 30, 2023

lonegunmanb commented Nov 30, 2023 • edited Loading

The testing error

How can we az login in e2e test?

How to reuse my code

mbender-ms commented Nov 30, 2023

mbender-ms commented Nov 30, 2023

stemaMSFT commented Nov 30, 2023

TomArcherMsft commented Nov 20, 2023 •

edited

Loading

lonegunmanb commented Nov 30, 2023 •

edited

Loading

How can we `az login` in e2e test?