New resource: azurerm_dev_test_global_shutdown_schedule #5536

sean-nixon · 2020-01-27T17:34:34Z

This resource enables the "Auto-shutdown" functionality of regular ARM-based VMs. It's basically a hack of the Dev Test Labs schedules resource known as a "Global Schedule" in the API docs. The API looks like it would support multiple types of schedules and timings, but in reality based on testing only "ComputeVMShutdownTask" schedules are supported and only with a daily recurrence. The API also seems to require a specific naming standard of the resource of "shutdown-computevm-{{VM Name}}". Anything else results in a 400 Bad Request.

Because of these restrictions, for ease of use, I opted to create a resource specifically for shutdown schedules that essentially maps to the "Auto-shutdown" blade in the Portal. If in the future, if additional types of schedules like Auto-starts are supported, my thought is new resources could be created or a more general resource could be created to support multiple types.

I'm very open to feedback on resource naming and overall approach.

References:

Feature Announcement
https://azure.microsoft.com/en-us/blog/announcing-auto-shutdown-for-vms-using-azure-resource-manager/

API Docs
https://docs.microsoft.com/en-us/rest/api/dtl/globalschedules

Example Usage Forum Post
https://social.msdn.microsoft.com/Forums/en-US/25a02403-dba9-4bcb-bdcc-1f4afcba5b65/powershell-script-to-autoshutdown-azure-virtual-machine?forum=WAVirtualMachinesforWindows

sean-nixon · 2020-02-24T19:59:57Z

@tombuildsstuff or @katbyte I know y'all have been busy prepping for the v2.0 release, but would love to get feedback on this PR when you get the chance. I'm working on setting up auto-shutdown on VMs for a client right now and we'd love to have a native Terraform resource to do so.

katbyte

Hey @sean-nixon, thank you for your patience while we get 2.0 out the door. I've given this a review and while its off to a good start i've left some comments inline that we need to address before merge.

azurerm/internal/services/devtestlabs/resource_arm_dev_test_global_shutdown_schedule.go

katbyte · 2020-03-06T18:46:23Z

azurerm/internal/services/devtestlabs/resource_arm_dev_test_global_shutdown_schedule.go

+	defer cancel()
+
+	targetResourceID := d.Get("target_resource_id").(string)
+	rid, err := azure.ParseAzureResourceID(targetResourceID)


It seems like this is expected to be a VM ID? could we write a parse ID as can be seen in #5692

Added custom parse and validate functions. Would love feedback on them.

katbyte · 2020-03-06T18:49:52Z

azurerm/internal/services/devtestlabs/resource_arm_dev_test_global_shutdown_schedule.go

+		Schema: map[string]*schema.Schema{
+			"location": azure.SchemaLocation(),
+
+			"target_resource_id": {


Is this any resource or just dev test VMs? if so could we use target_devtest_vm_id ?

It's a normal virtual machine ID, not a dev test VM. I updated it to be virtual_machine_id like with extensions which is a little clearer. Do you agree with that approach?

...m/internal/services/devtestlabs/tests/resource_arm_dev_test_global_shutdown_schedule_test.go

website/docs/r/dev_test_global_shutdown_schedule.html.markdown

...m/internal/services/devtestlabs/tests/resource_arm_dev_test_global_shutdown_schedule_test.go

sean-nixon · 2020-03-27T05:38:56Z

@katbyte Thanks for reviewing this! Appreciate all your hard work. I just got to making the updates you suggested. I also made some additional minor tweaks to the schema to address some issues that came up during my testing:

Made notification_settings.enabled required to make things simpler and not have scenarios with an empty notification_settings block
Removed validation from notification_settings.webhook_url as I was unable to find an equivalent for validate.NoEmptyStrings
Removed Computed: true from notification_settings.webhook_url to ensure that Terraform properly detects a change if the URL is set on the resource in Azure but not in the configuration

I think this should be ready for another round of review when you get the chance.

sean-nixon · 2020-04-24T17:30:39Z

@katbyte Is there anything else I can address on this PR?

katbyte

Thanks for those changes @sean-nixon,

just a few more comments and this should be good to merge. I do question the name of the resource as "global shutdown schedule" implies its global when its scoped to a VM. So i thin we should rename it to something that implies its limited in scope to a VM, maybe azurerm_dev_test_vm_[global]_shutdown_schedule?

azurerm/internal/services/devtestlabs/resource_arm_dev_test_global_shutdown_schedule.go

sean-nixon · 2020-05-14T21:32:50Z

Hi @katbyte, thanks for the great suggestions. I think I've cleaned up everything from your latest review. I think the last remaining question is the resource name. Let me know your thoughts on my inline comment above and I'll refactor the resource name accordingly.

katbyte · 2020-05-20T02:23:57Z

@sean-nixon - not sure if you saw my comment reply - but i'm thinking azurerm_dev_test_global_vm_shutdown_schedule is a great compromise/final name to run with!

sean-nixon · 2020-05-20T03:46:07Z

Hi @katbyte, thank you for reminding me to come back to this. I did see your comment earlier but hadn't gotten the chance to come back and refactor everything. I just pushed up my latest changes to use the new name. I also updated the documentation for the new resource to try to contrast it more clearly with the regular dev_test_schedule resource and to call out the resource ID format for imports. Let me know if you have any objections to those changes.

I also validated that the integration tests are still working.

$ make acctests SERVICE='devtestlabs' TESTARGS='-run=TestAccAzureRMDevTestLabGlobalVMShutdownSchedule'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
TF_ACC=1 go test -v ./azurerm/internal/services/devtestlabs/tests/ -run=TestAccAzureRMDevTestLabGlobalVMShutdownSchedule -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownBasic
=== PAUSE TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownBasic
=== RUN   TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownComplete
=== PAUSE TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownComplete
=== RUN   TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownUpdate
=== PAUSE TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownUpdate
=== CONT  TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownBasic
=== CONT  TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownUpdate
=== CONT  TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownComplete
--- PASS: TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownBasic (431.16s)
--- PASS: TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownComplete (555.11s)
--- PASS: TestAccAzureRMDevTestLabGlobalVMShutdownSchedule_autoShutdownUpdate (623.81s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/devtestlabs/tests   624.863s

Let me know if there's anything else you'd like for me to address. Would love to see this in the next release 😊

…hutdown_schedule

katbyte

Thanks @sean-nixon! LGTM now, pushed some changes to fix the build (and do a little reorg)

ghost · 2020-05-28T22:23:00Z

This has been released in version 2.12.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.12.0"
}
# ... other configuration ...

This resource enables the "Auto-shutdown" functionality of regular ARM-based VMs. It's basically a hack of the Dev Test Labs schedules resource known as a "Global Schedule" in the API docs. The API looks like it would support multiple types of schedules and timings, but in reality based on testing only "ComputeVMShutdownTask" schedules are supported and only with a daily recurrence. The API also seems to require a specific naming standard of the resource of "shutdown-computevm-{{VM Name}}". Anything else results in a 400 Bad Request. Because of these restrictions, for ease of use, I opted to create a resource specifically for shutdown schedules that essentially maps to the "Auto-shutdown" blade in the Portal. If in the future, if additional types of schedules like Auto-starts are supported, my thought is new resources could be created or a more general resource could be created to support multiple types. I'm very open to feedback on resource naming and overall approach. References: Feature Announcement azure.microsoft.com/en-us/blog/announcing-auto-shutdown-for-vms-using-azure-resource-manager API Docs docs.microsoft.com/en-us/rest/api/dtl/globalschedules Example Usage Forum Post social.msdn.microsoft.com/Forums/en-US/25a02403-dba9-4bcb-bdcc-1f4afcba5b65/powershell-script-to-autoshutdown-azure-virtual-machine?forum=WAVirtualMachinesforWindows

rapster83 · 2020-06-04T14:48:59Z

Hi guys,

thank you very much for the resource. According to the the Terraform Provider Definition, there is no parameter for "mail" or "email". Only webhook_url is possible! Could the resource please be extended with a "mail" parameter to add mail addresses for notification purposes, as the following screenshot illustrates?

Do I have to open an official issue for that? Thx.

sean-nixon · 2020-06-04T16:03:06Z

@stefan-rapp I would probably recommend creating an issue for this. The reason I didn't include email is because it would require an SDK update and I didn't want to try to introduce that together with my change as I'm not familiar with the process. Once the SDK version is updated, adding email support should be simple.

rapster83 · 2020-06-05T09:51:22Z

@sean-nixon : #7221 (comment)

ghost · 2020-06-24T14:50:01Z

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 added size/XL documentation labels Jan 27, 2020

katbyte requested changes Mar 6, 2020

View reviewed changes

katbyte added service/devtestlabs new-resource waiting-response labels Mar 6, 2020

philthynz mentioned this pull request Mar 11, 2020

Feature Request: DevTest Lab Schedules support for ComputeVmShutdownTask task type #5863

Open

Sean Nixon added 4 commits March 25, 2020 10:41

New resource: azurerm_dev_test_global_shutdown_schedule

a216b58

Fix test and website Terraform formatting

4d12fe9

Add devtestlab global schedule parse and validate funcs

ca9c6fc

Update schema and tests based on review feedback

1c7da13

sean-nixon force-pushed the r/dev_test_global_shutdown_schedule branch from b1c4419 to 1c7da13 Compare March 27, 2020 05:21

ghost added size/XXL and removed size/XL labels Mar 27, 2020

ghost removed the waiting-response label Mar 27, 2020

Fix build and tests

f2cec27

sean-nixon requested a review from katbyte May 5, 2020 20:31

katbyte requested changes May 14, 2020

View reviewed changes

katbyte added the waiting-response label May 14, 2020

Cleanup unnecessary variables

9a98a21

ghost removed the waiting-response label May 14, 2020

katbyte added this to the v2.11.0 milestone May 15, 2020

katbyte added the waiting-response label May 20, 2020

Sean Nixon added 2 commits May 19, 2020 22:28

Rename resource to azurerm_dev_test_global_vm_shutdown_schedule

27f925f

Improve docs for r/dev_test_global_vm_shutdown_schedule

48ce688

ghost removed the waiting-response label May 20, 2020

katbyte modified the milestones: v2.11.0, v2.12.0 May 22, 2020

Sean Nixon and others added 3 commits May 24, 2020 14:07

r/dev_test_global_vm_shutdown_schedule fix website link

8cff069

Merge remote-tracking branch 'origin/master' into r/dev_test_global_s…

472f2c1

…hutdown_schedule

fix linting & file reorg

de9cf65

katbyte approved these changes May 24, 2020

View reviewed changes

katbyte added 2 commits May 24, 2020 17:24

fix linting

aa88e92

update test names

afd81cb

katbyte merged commit 4226473 into hashicorp:master May 25, 2020

katbyte added a commit that referenced this pull request May 25, 2020

update CHANGELO.md to include #5536

d29460b

sean-nixon deleted the r/dev_test_global_shutdown_schedule branch May 28, 2020 22:26

pbrit pushed a commit to pbrit/terraform-provider-azurerm that referenced this pull request May 31, 2020

update CHANGELO.md to include hashicorp#5536

554a92f

rapster83 mentioned this pull request Jun 5, 2020

Support for "mail" attribut in "azurerm_dev_test_global_vm_shutdown_schedule" #7221

Closed

ghost locked and limited conversation to collaborators Jun 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource: azurerm_dev_test_global_shutdown_schedule #5536

New resource: azurerm_dev_test_global_shutdown_schedule #5536

sean-nixon commented Jan 27, 2020

sean-nixon commented Feb 24, 2020

katbyte left a comment

katbyte Mar 6, 2020

sean-nixon Mar 27, 2020

katbyte Mar 6, 2020

sean-nixon Mar 27, 2020

sean-nixon commented Mar 27, 2020

sean-nixon commented Apr 24, 2020

katbyte left a comment

sean-nixon commented May 14, 2020

katbyte commented May 20, 2020

sean-nixon commented May 20, 2020

katbyte left a comment

ghost commented May 28, 2020

rapster83 commented Jun 4, 2020

sean-nixon commented Jun 4, 2020

rapster83 commented Jun 5, 2020

ghost commented Jun 24, 2020

New resource: azurerm_dev_test_global_shutdown_schedule #5536

New resource: azurerm_dev_test_global_shutdown_schedule #5536

Conversation

sean-nixon commented Jan 27, 2020

sean-nixon commented Feb 24, 2020

katbyte left a comment

Choose a reason for hiding this comment

katbyte Mar 6, 2020

Choose a reason for hiding this comment

sean-nixon Mar 27, 2020

Choose a reason for hiding this comment

katbyte Mar 6, 2020

Choose a reason for hiding this comment

sean-nixon Mar 27, 2020

Choose a reason for hiding this comment

sean-nixon commented Mar 27, 2020

sean-nixon commented Apr 24, 2020

katbyte left a comment

Choose a reason for hiding this comment

sean-nixon commented May 14, 2020

katbyte commented May 20, 2020

sean-nixon commented May 20, 2020

katbyte left a comment

Choose a reason for hiding this comment

ghost commented May 28, 2020

rapster83 commented Jun 4, 2020

sean-nixon commented Jun 4, 2020

rapster83 commented Jun 5, 2020

ghost commented Jun 24, 2020