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

Minor v4.1.0 azurerm bugfixes #387

Merged
merged 12 commits into from
Sep 13, 2024
Merged

Minor v4.1.0 azurerm bugfixes #387

merged 12 commits into from
Sep 13, 2024

Conversation

jcma-google
Copy link
Collaborator

@jcma-google jcma-google commented Sep 12, 2024

Description

  • providers need to include subscription id. This error didn't appear during my testing of the 4.1.0 update because it only occurs when the deployment setup changes.
  • change private-ip allocation method to "Static"
    • the staging resource uses a standard SKU and "dynamic" allocation is only permissible for more expensive plans. It looks like "Static" just reserves an IP address and doesn't change it, not that I see a reason to change it after it's allocated.
  • remove DOCKER_REGISTRY_SERVER_URL
    • this is now deprecated. The now preferred way of doing it is to set it in the linux_web_app resource, which we are already doing.
      allocation_method = "Static"

Checklist

General

  • [x ] Added the correct label
  • [x ] Assigned to a specific person or civiform/deployment-system
  • [x ] Created tests which fail without the change (if possible)
  • [x ] Performed manual testing (at a minimum run bin/setup without your changes and then bin/deploy with your changes to ensure your changes don't break existing deployments)
  • [ x] Extended the README / documentation, if necessary

@jcma-google jcma-google added the bug Something isn't working label Sep 12, 2024
@jcma-google jcma-google removed the request for review from gwendolyngoetz September 12, 2024 20:14
@jcma-google jcma-google marked this pull request as draft September 12, 2024 20:14
@jcma-google jcma-google changed the title Add subscription id which is required for v4.1.0 of azurerm. Minor v4.1.0 azurerm bugfixes Sep 13, 2024
@jcma-google jcma-google marked this pull request as ready for review September 13, 2024 18:28
@@ -1,3 +1,4 @@
provider "azurerm" {
features {}
subscription_id = "4ef4ae1b-c966-4ac4-9b7c-a837ea410821"
Copy link
Contributor

Choose a reason for hiding this comment

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

(question) Is there a reason not to get this from a terraform variable and the civiform*config.sh file. I'm concerned this will get forgotten about and cause issues down the road.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried using the config variable here and it works! I didn't realize that the providers.tf could access the variables but on retrospect it makes a lot of sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wait actually it was because I forgot to push my changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay it looks like the providers.tf doesn't have access to the civiform config, but maybe I'm missing something?

@jcma-google jcma-google merged commit e00ef1e into main Sep 13, 2024
7 checks passed
@jcma-google jcma-google deleted the jcma-azure-bufix branch September 13, 2024 19:21
jcma-google added a commit that referenced this pull request Sep 16, 2024
jcma-google added a commit that referenced this pull request Sep 17, 2024
jcma-google added a commit that referenced this pull request Sep 18, 2024
jcma-google added a commit that referenced this pull request Sep 18, 2024
jcma-google added a commit that referenced this pull request Sep 19, 2024
jcma-google added a commit that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants