azurerm_container_group - adding restart_policy
#827
Conversation
metacpp
requested review from
tombuildsstuff,
mbfrahry,
JunyiYi and
WodansSon
|
Depends on #830 |
There was a problem hiding this comment.
hey @metacpp
Thanks for this PR - I've taken a look through and this mostly LGTM, if we can fix up the minor issues this should be good to merge.
Thanks!
| @@ -41,6 +41,7 @@ resource "azurerm_container_group" "aci-helloworld" { | |||
| resource_group_name = "${azurerm_resource_group.aci-rg.name}" | |||
| ip_address_type = "public" | |||
| os_type = "linux" | |||
| restart_policy = "Always" | |||
There was a problem hiding this comment.
minor since this is defaulted, I don't think we need to specify it in the example
| @@ -43,8 +43,8 @@ | |||
| "versionExact": "v12.5.0-beta" | |||
| }, | |||
| { | |||
| "checksumSHA1": "e8WF/zKkHC6sKeiPaP+ulB/NSq4=", | |||
| "path": "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2017-08-01-preview/containerinstance", | |||
There was a problem hiding this comment.
can we remove the old API version from the vendor directory too, since it should be unused now?
| @@ -139,6 +140,7 @@ func TestAccAzureRMContainerGroup_windowsComplete(t *testing.T) { | |||
| resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo", "bar"), | |||
| resource.TestCheckResourceAttr(resourceName, "container.0.environment_variables.foo1", "bar1"), | |||
| resource.TestCheckResourceAttr(resourceName, "os_type", "Windows"), | |||
| resource.TestCheckResourceAttr(resourceName, "restart_policy", "OnFailure"), | |||
There was a problem hiding this comment.
can we make this Never to be able to test that setting is valid? (since Always and OnFailure are tested above, implicitly and explicitly respectively)
|
@tombuildsstuff updated. |
|
Tests pass:
|
tombuildsstuff
changed the title
azurerm_container_group - adding restart_policy
|
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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
This change enables restart_policy for container instances running in the container group.
Test result at 2018-02-15:
=== RUN TestAccAzureRMContainerGroup_windowsComplete
--- PASS: TestAccAzureRMContainerGroup_windowsComplete (71.23s)
PASS
=== RUN TestAccAzureRMContainerGroup_windowsBasic
--- PASS: TestAccAzureRMContainerGroup_windowsBasic (71.39s)
PASS
=== RUN TestAccAzureRMContainerGroup_linuxBasic
--- PASS: TestAccAzureRMContainerGroup_linuxBasic (73.26s)
PASS
=== RUN TestAccAzureRMContainerGroup_linuxBasicUpdate
--- PASS: TestAccAzureRMContainerGroup_linuxBasicUpdate (87.33s)
PASS
=== RUN TestAccAzureRMContainerGroup_linuxComplete
--- PASS: TestAccAzureRMContainerGroup_linuxComplete (119.41s)
PASS
ok ../azurerm/test-azurerm 119.423s