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

Fixing issue 1286 : We are unable to disassociate a PIP once attached #1295

Merged
merged 5 commits into from Jun 1, 2018

Conversation

VaijanathB
Copy link
Contributor

@VaijanathB VaijanathB commented May 24, 2018

I have run all the NIC test cases. Most of them pass and couple failed due to unrelated issues.

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -run=TestAccAzureRMNetwork -timeout 180m
=== RUN   TestAccAzureRMNetworkInterface_importBasic
--- PASS: TestAccAzureRMNetworkInterface_importBasic (105.28s)
=== RUN   TestAccAzureRMNetworkInterface_importIPForwarding
--- PASS: TestAccAzureRMNetworkInterface_importIPForwarding (104.16s)
=== RUN   TestAccAzureRMNetworkInterface_importWithTags
--- PASS: TestAccAzureRMNetworkInterface_importWithTags (105.04s)
=== RUN   TestAccAzureRMNetworkInterface_importMultipleLoadBalancers
--- PASS: TestAccAzureRMNetworkInterface_importMultipleLoadBalancers (115.82s)
=== RUN   TestAccAzureRMNetworkInterface_importApplicationGateway
--- PASS: TestAccAzureRMNetworkInterface_importApplicationGateway (1682.79s)
=== RUN   TestAccAzureRMNetworkInterface_importPublicIP
--- PASS: TestAccAzureRMNetworkInterface_importPublicIP (105.56s)
=== RUN   TestAccAzureRMNetworkInterface_importApplicationSecurityGroup
--- PASS: TestAccAzureRMNetworkInterface_importApplicationSecurityGroup (108.86s)
=== RUN   TestAccAzureRMNetworkSecurityGroup_importBasic
--- PASS: TestAccAzureRMNetworkSecurityGroup_importBasic (75.94s)
=== RUN   TestAccAzureRMNetworkSecurityGroup_importSingleRule
--- PASS: TestAccAzureRMNetworkSecurityGroup_importSingleRule (72.72s)
=== RUN   TestAccAzureRMNetworkSecurityGroup_importMultipleRules
--- PASS: TestAccAzureRMNetworkSecurityGroup_importMultipleRules (72.90s)
=== RUN   TestAccAzureRMNetworkSecurityRule_importBasic
--- PASS: TestAccAzureRMNetworkSecurityRule_importBasic (85.64s)
=== RUN   TestAccAzureRMNetworkInterface_basic
--- PASS: TestAccAzureRMNetworkInterface_basic (103.60s)
=== RUN   TestAccAzureRMNetworkInterface_disappears
--- PASS: TestAccAzureRMNetworkInterface_disappears (104.81s)
=== RUN   TestAccAzureRMNetworkInterface_setNetworkSecurityGroupId
--- PASS: TestAccAzureRMNetworkInterface_setNetworkSecurityGroupId (109.83s)
=== RUN   TestAccAzureRMNetworkInterface_removeNetworkSecurityGroupId
--- FAIL: TestAccAzureRMNetworkInterface_removeNetworkSecurityGroupId (107.59s)
	testing.go:513: Step 1 error: Error applying: 1 error(s) occurred:
		
		* azurerm_network_security_group.test (destroy): 1 error(s) occurred:
		
		* azurerm_network_security_group.test: Error deleting Network Security Group "acctestnsg-3735975067294333692" (Resource Group "acctest-rg-3735975067294333692"): network.SecurityGroupsClient#Delete: Failure sending request: StatusCode=400 -- Original Error: Code="InUseNetworkSecurityGroupCannotBeDeleted" Message="Network security group /subscriptions/cba4e087-aceb-44f0-970e-65e96eff4081/resourceGroups/acctest-rg-3735975067294333692/providers/Microsoft.Network/networkSecurityGroups/acctestnsg-3735975067294333692 cannot be deleted because it is in use by the following resources: /subscriptions/cba4e087-aceb-44f0-970e-65e96eff4081/resourceGroups/acctest-rg-3735975067294333692/providers/Microsoft.Network/networkInterfaces/acctestni-3735975067294333692." Details=[]
=== RUN   TestAccAzureRMNetworkInterface_multipleSubnets
--- PASS: TestAccAzureRMNetworkInterface_multipleSubnets (103.12s)
=== RUN   TestAccAzureRMNetworkInterface_multipleSubnetsPrimary
--- PASS: TestAccAzureRMNetworkInterface_multipleSubnetsPrimary (111.85s)
=== RUN   TestAccAzureRMNetworkInterface_enableIPForwarding
--- PASS: TestAccAzureRMNetworkInterface_enableIPForwarding (103.90s)
=== RUN   TestAccAzureRMNetworkInterface_enableAcceleratedNetworking
--- PASS: TestAccAzureRMNetworkInterface_enableAcceleratedNetworking (104.70s)
=== RUN   TestAccAzureRMNetworkInterface_multipleLoadBalancers
--- PASS: TestAccAzureRMNetworkInterface_multipleLoadBalancers (106.81s)
=== RUN   TestAccAzureRMNetworkInterface_applicationGateway
--- PASS: TestAccAzureRMNetworkInterface_applicationGateway (1520.50s)
=== RUN   TestAccAzureRMNetworkInterface_withTags
--- PASS: TestAccAzureRMNetworkInterface_withTags (110.16s)
=== RUN   TestAccAzureRMNetworkInterface_bug7986
--- PASS: TestAccAzureRMNetworkInterface_bug7986 (107.25s)
=== RUN   TestAccAzureRMNetworkInterface_applicationSecurityGroups
--- PASS: TestAccAzureRMNetworkInterface_applicationSecurityGroups (93.98s)
=== RUN   TestAccAzureRMNetworkInterface_internalFQDN
--- FAIL: TestAccAzureRMNetworkInterface_internalFQDN (102.52s)
	testing.go:513: Step 0 error: Check failed: Check 2/2 error: azurerm_network_interface.test: Attribute 'internal_fqdn' expected "acctestnic-4928547999410651021.example.com", got ""

Fixes #1286

Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hi @VaijanathB

Thanks for this PR :)

Whilst the fix for this looks good - can we add an acceptance test to prove this works; we can do this by first creating a VM with these values, then removing them and checking they work (the TestAccAzureRMPublicIpStatic_withTags test shows an example of this).

Thanks!

@tombuildsstuff tombuildsstuff added this to the Soon milestone May 25, 2018
Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

One minor thing to fix before we can run the tests & merge this (which I'm going to push a commit to fix) - but this otherwise LGTM :)

{
Config: testAccAzureRMNetworkInterface_withIPAddressesUpdate(rInt, testLocation()),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMNetworkInterfaceExists(resourceName),
Copy link
Member

Choose a reason for hiding this comment

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

we can also verify these are set to nothing (e.g. there's no Public/Private IP Addresses set) via:

resource.TestCheckResourceAttr(resourceName, "ip_configuration.0.private_ip_address", ""),
resource.TestCheckResourceAttr(resourceName, "ip_configuration.0. public_ip_address_id", ""),

@tombuildsstuff tombuildsstuff modified the milestones: Soon, 1.7.0 Jun 1, 2018
@tombuildsstuff
Copy link
Member

Ignoring a couple of intermittent test failures, the tests pass:

screen shot 2018-06-01 at 12 41 48

@tombuildsstuff tombuildsstuff merged commit 5a91175 into master Jun 1, 2018
@tombuildsstuff tombuildsstuff deleted the issue1286 branch June 1, 2018 10:45
tombuildsstuff added a commit that referenced this pull request Jun 1, 2018
@jakubigla
Copy link

can't wait guys! :)

@ghost
Copy link

ghost commented Mar 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants