-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
azurerm_automation_job_schedule
: update jobSchedule resource id format
#22164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR!
I've taken a look through and left some comments inline. The main ask is that this solution is built on an assumption that the schedules linked to a runbook must have different names, would you please help confirm whether this is the case?
internal/services/automation/migration/automation_job_schedule_migration_v0_to_v1.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_runbook_resource_test.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource_test.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
The Schedule name should be unique inside the same automation account, and there MUST be only 1 link between the runbook and the schedule. so the new id schema should work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR!
I have left some additional mostly minor comments that once addressed this should be good to merge 👍
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_job_schedule_resource_test.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_runbook_resource_test.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_runbook_resource_test.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_runbook_resource_test.go
Outdated
Show resolved
Hide resolved
9be75ad
to
48e1117
Compare
7713e60
to
b783f72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @wuxu92 I had a look though this and left a couple of comments inline, but once those are addressed we can take another look. Thanks!
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes @wuxu92 - I tested this and still got an error in some cases, could you take another look? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this up @wuxu92 - I spotted a couple of minor things on looking through this again but otherwise this is looking good. Thanks!
internal/services/automation/automation_job_schedule_resource.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the additional changes @wuxu92 - LGTM!
<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.108.0" to "3.109.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.109.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.109.0
FEATURES:

* **New Data Source:** `azurerm_automation_runbook` ([#26359](hashicorp/terraform-provider-azurerm#26359 **New Resource:** `azurerm_data_protection_backup_instance_postgresql_flexible_server` ([#26249](hashicorp/terraform-provider-azurerm#26249 **New Resource:** `azurerm_email_communication_service_domain` ([#26179](hashicorp/terraform-provider-azurerm#26179 **New Resource:** `azurerm_system_center_virtual_machine_manager_cloud` ([#25429](hashicorp/terraform-provider-azurerm#25429 **New Resource:** `azurerm_system_center_virtual_machine_manager_virtual_machine_template` ([#25449](hashicorp/terraform-provider-azurerm#25449 **New Resource:** `azurerm_system_center_virtual_machine_manager_virtual_network` ([#25451](https://github.com/hashicorp/terraform-provider-azurerm/issues/25451))

ENHANCEMENTS:

* Data Source: `azurerm_hdinsight_cluster` - export the `cluster_id` attribute ([#26228](hashicorp/terraform-provider-azurerm#26228 `azurerm_cosmosdb_sql_container` - support for the `partition_key_kind` and `partition_key_paths` properties ([#26372](hashicorp/terraform-provider-azurerm#26372 `azurerm_data_protection_backup_instance_blob_storage` - support for the `storage_account_container_names` property ([#26232](hashicorp/terraform-provider-azurerm#26232 `azurerm_virtual_network_peering` - support for the `peer_complete_virtual_networks_enabled`, `only_ipv6_peering_enabled`, `local_subnet_names`, and `remote_subnet_names` properties ([#26229](hashicorp/terraform-provider-azurerm#26229 `azurerm_virtual_desktop_host_pool` - changing the `preferred_app_group_type` property no longer creates a new resource ([#26333](hashicorp/terraform-provider-azurerm#26333 `azurerm_maps_account` - support for the `location`, `identity`, `cors` and `data_store` properties ([#26397](https://github.com/hashicorp/terraform-provider-azurerm/issues/26397))

BUG FIXES:

* `azurerm_automation_job_schedule` - updates `azurerm_automation_job_schedule` to use a composite resource id and allows `azurerm_automation_runbook` to be updated without causing `azurerm_automation_job_schedule` to recreate ([#22164](hashicorp/terraform-provider-azurerm#22164 `azurerm_databricks_workspace`- correctly allow disabling the default firewall ([#26339](hashicorp/terraform-provider-azurerm#26339 `azurerm_virtual_hub_*` - spliting create and update so lifecycle ignore changes works correctly ([#26310](https://github.com/hashicorp/terraform-provider-azurerm/issues/26310))

DEPRECATIONS:

* Data Source: `azurerm_mariadb_server` - deprecated since the service is retiring. Please use `azurerm_mysql_flexible_server` instead ([#26354](hashicorp/terraform-provider-azurerm#26354 `azurerm_mariadb_configuration` - deprecated since the service is retiring. Please use `azurerm_mysql_flexible_server_configuration` instead ([#26354](hashicorp/terraform-provider-azurerm#26354 `azurerm_mariadb_database` - deprecated since the service is retiring. Please use `azurerm_mysql_flexible_database` instead ([#26354](hashicorp/terraform-provider-azurerm#26354 `azurerm_mariadb_firewall_rule` - deprecated since the service is retiring. Please use `azurerm_mysql_flexible_server_firewall_rule` instead ([#26354](hashicorp/terraform-provider-azurerm#26354 `azurerm_mariadb_server` - deprecated since the service is retiring. Please use `azurerm_mysql_flexible_server` instead ([#26354](hashicorp/terraform-provider-azurerm#26354 `azurerm_mariadb_virtual_network_rule` - deprecated since the service is retiring ([#26354](https://github.com/hashicorp/terraform-provider-azurerm/issues/26354))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/256/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
The job schdule is a link between a runbook and a schdule. there should be only one such link between the runbook and schedule pair. When we update a runbook, request a PUT of runbook resource, the job schedule id linked to this runbook will be automatically updated, and the old job schedule id should be NOT FOUND then.
The current implementation introduced in #7555 will try to delete all job schedules linked to the runbook and then it need a next
terraform apply
to recreate these job schedules. This is not neccessary at first, and it make theterraform plan
can not report whatterraform apply
really does, it makes users confused when update a runbook.This PR is to replace the
azurerm_automation_job_schedule
resource id with a customized one. The new one contains the runbook name and the schedule resource name, so we can query the job schdule id in runtime and made the id responded from API as computed attributeresource_manager_id
. this PR also introduces a schema migration to make sure not break exsiting resources.the old resource id format:
terraform import azurerm_automation_job_schedule.example /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group1/providers/Microsoft.Automation/automationAccounts/account1/jobSchedules/10000000-1001-1001-1001-000000000001
the new resourec id format:
/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group1/providers/Microsoft.Automation/automationAccounts/account1/runBook/book1/schedule/schedule1
resolves: #8634
Test Pass in TC