search

Azure / terraform

Source code for the Azure Marketplace Terraform development VM package.
MIT License
693 stars 784 forks source link

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

Closed mbender-ms closed 9 months ago

mbender-ms commented 9 months ago

@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"

TomArcherMsft commented 9 months ago

@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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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

lonegunmanb commented 9 months ago

@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 commented 9 months ago

please-close

mbender-ms commented 9 months ago

please-open

mbender-ms commented 9 months ago

@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 commented 9 months ago

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

mbender-ms commented 9 months ago

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

stemaMSFT commented 9 months ago

@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 commented 9 months ago

@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 commented 9 months ago

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: https://github.com/lonegunmanb/azterraform/commit/b18220ba498bbea2b51478dbdd49567de27df90e

I've created a special sub-test method for this example since it requires az login as testing setup: https://github.com/lonegunmanb/azterraform/commit/b18220ba498bbea2b51478dbdd49567de27df90e#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 commented 9 months ago

@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 commented 9 months ago

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

stemaMSFT commented 9 months ago

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