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.

Sign up for GitHub

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

Jump to bottom

Resource Renaming/Duplication: azurerm_monitor_autoscale_setting / azurerm_log_analytics_linked_service #2768

Merged
merged 17 commits into from
Jan 28, 2019
Merged

Resource Renaming/Duplication: azurerm_monitor_autoscale_setting / azurerm_log_analytics_linked_service #2768

merged 17 commits into from
Jan 28, 2019

Conversation

tombuildsstuff
Copy link
Contributor

This PR spans a few different areas, as a result of passing through the entire codebase looking for deprecations:

  • Introduces a new resource azurerm_monitor_autoscale_setting to replace the azurerm_autoscale_setting resource.
  • Introduces a new resource azurerm_log_analytics_linked_service to replace the azurerm_log_analytics_workspace_linked_service resource.
  • azurerm_postgresql_server - support for version 10 and 10.2
  • azurerm_application_gateway - deprecating the fqdn_list field in favour of fqdns
  • azurerm_application_gateway - deprecating the ip_address_list field in favour of ip_addresses

@tombuildsstuff
New Resource: azurerm_monitor_autoscale_setting
37965f6 
This is a direct copy of the `azurerm_autoscale_setting` resource
which allows us to deprecate the old one

Including a guide on how to migrate from one resource to the other
@tombuildsstuff
Updating to include the changes in #2760
5521dfe
@tombuildsstuff
documenting the replacement monitor metric alert rule resources
f5ee333
@tombuildsstuff
Updating the deprecation message for the metric_alertrule resource
f4d27de
@tombuildsstuff
refactoring the azurerm_monitor_metric_alertrule resource
e3cd9bf
@tombuildsstuff
cdn_profile: switching to use the enum
30ab72f
@tombuildsstuff
r/application_gateway: deprecating the fqdn_list and `ip_address_li…
1617314 
…st` fields
@tombuildsstuff
r/log_analytics_workspace_linked_service: deprecating the `resource_i…
10eaaa9 
…d` field in the block in favour of the top level field
@tombuildsstuff
Newly renamed resource: `azurerm_log_analtyics_linked_service
a41c6ed
@tombuildsstuff
Removing oms from the highlight name
0e01027
@tombuildsstuff
Deprecating the older `azurerm_log_analytics_workspace_linked_service…
c1f3829 
…` resource
@tombuildsstuff
renaming the renaming resources guide
c681476
@tombuildsstuff
Adding the deprecation message to the `log_analytics_workspace_linked…
74f6f30 
…_service` resource
@tombuildsstuff
PostgreSQL Server: support for 10/10.0/10.2
847de4b
@tombuildsstuff
azurerm_servicebus_queue: ensuring deprecated fields are marked as …
7039eaf 
…optional
@tombuildsstuff
Adding some additional comments
b323abc
@tombuildsstuff tombuildsstuff added this to the 1.22.0 milestone Jan 25, 2019
metacpp
metacpp reviewed Jan 25, 2019
View reviewed changes
Copy link
Contributor

@metacpp metacpp left a comment

Choose a reason for hiding this comment

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

@tombuildsstuff Thanks for the effort to rename the resources.

I did a quick look through the PR.

The only difference between resourceArmAutoScaleSetting and resourceArmMonitorAutoScaleSetting is the extra deprecate message. Maybe we can just refactor resourceArmAutoScaleSetting a bit to support renaming scenario considering that the internal schema and functionality between these 2 functions are exactly the same ? Just my 2 cents.

katbyte
katbyte approved these changes Jan 25, 2019
View reviewed changes
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

aside from a couple comments LGTM @tombuildsstuff 👍

azurerm/resource_arm_log_analytics_linked_service.go Show resolved Hide resolved
azurerm/resource_arm_network_interface.go
@@ -146,6 +146,7 @@ func resourceArmNetworkInterface() *schema.Resource {
Set: schema.HashString,
},

// TODO: does this want deprecating & a virtual resource for 2.0?
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

website/docs/r/autoscale_setting.html.markdown Show resolved Hide resolved
website/docs/r/log_analytics_workspace_linked_service.html.markdown Show resolved Hide resolved
website/docs/r/servicebus_queue.html.markdown Show resolved Hide resolved
metacpp
metacpp approved these changes Jan 25, 2019
View reviewed changes
Copy link
Contributor

@metacpp metacpp left a comment

Choose a reason for hiding this comment

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

I've gone through all the changes, mostly LGTM and ready to be merged if we can address the comments in line.

website/docs/r/servicebus_queue.html.markdown Show resolved Hide resolved
website/docs/r/log_analytics_workspace.html.markdown Show resolved Hide resolved
website/docs/r/application_gateway.html.markdown Outdated Show resolved Hide resolved
website/docs/r/application_gateway.html.markdown Outdated Show resolved Hide resolved
metacpp
metacpp previously requested changes Jan 25, 2019
View reviewed changes
Copy link
Contributor

@metacpp metacpp left a comment

Choose a reason for hiding this comment

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

Please address the suggestions in line.

@tombuildsstuff tombuildsstuff dismissed metacpp’s stale review January 28, 2019 15:46

dismissing since changes have been pushed

@tombuildsstuff tombuildsstuff merged commit f367512 into master Jan 28, 2019
@tombuildsstuff tombuildsstuff deleted the f/renaming-resources branch January 28, 2019 16:02
tombuildsstuff added a commit that referenced this pull request Jan 28, 2019
@tombuildsstuff
Updating to include #2768
16d7bad
@ghost
Copy link

ghost commented Mar 5, 2019

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!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@katbyte katbyte katbyte approved these changes

@metacpp metacpp metacpp left review comments

Assignees
No one assigned
Labels
documentation new-resource refactor size/XXL technical-debt
Projects
None yet
Milestone
v1.22.0
Development

Successfully merging this pull request may close these issues.
3 participants
@tombuildsstuff @katbyte @metacpp