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

feat(TPG>=5.5)!: support setting maintenance_interval in instance_template #357

Merged

Conversation

tpdownes
Copy link
Member

Enable setting maintenance_interval in the scheduling block of instance templates. Must use google-beta provider but does not otherwise impact version. This might make it a breaking change from your perspective. I can edit commit and PR title appropriately.

@tpdownes tpdownes requested a review from a team as a code owner October 24, 2023 02:43
@tpdownes
Copy link
Member Author

FYI: in some further testing, I'm seeing an error upon updating the field from PERIODIC to null. It is trying to do the update in-place. This maybe a TPG problem or something we could guide with a replacement trigger.

@tpdownes
Copy link
Member Author

I filed an issue in TPG: hashicorp/terraform-provider-google#16345

Can you help me understand the integration test failure? It seems unrelated.

@tpdownes tpdownes marked this pull request as draft October 24, 2023 15:01
@tpdownes
Copy link
Member Author

tpdownes commented Dec 8, 2023

hashicorp/terraform-provider-google#16345 resolves this problem, I believe but requires upgrade to TPG 5.x. Future work to add to this PR.

@tpdownes tpdownes force-pushed the maintenance_interval branch from 4a6f31f to b433932 Compare December 8, 2023 19:14
@tpdownes tpdownes force-pushed the maintenance_interval branch from b433932 to 1756881 Compare December 8, 2023 19:16
@tpdownes
Copy link
Member Author

tpdownes commented Dec 8, 2023

@terraform-google-modules/cft-admins I would appreciate feedback on this PR as it changes to google-beta and bumps to 5.x. I could answer why this might be a support burden.

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @tpdownes!

modules/instance_template/versions.tf Outdated Show resolved Hide resolved
@apeabody
Copy link
Contributor

apeabody commented Dec 8, 2023

From the CI:

STDERR: Error: cannot determine self_link for subnetwork "cft-vm-test-9wb5": network_interface.0.subnetwork_project: required field is not set

  with module.preemptible_and_regular_instance_templates.module.preemptible_and_regular_instance_templates.module.preemptible.google_compute_instance_template.tpl,
  on ../../../../modules/instance_template/main.tf line 65, in resource "google_compute_instance_template" "tpl":
  65: resource "google_compute_instance_template" "tpl" {

@apeabody apeabody changed the title feat: support setting maintenance_interval in instance_template feat!: support setting maintenance_interval in instance_template Dec 8, 2023
@apeabody apeabody changed the title feat!: support setting maintenance_interval in instance_template feat(TPG>=5.5)!: support setting maintenance_interval in instance_template Dec 8, 2023
@tpdownes
Copy link
Member Author

From the CI:

STDERR: Error: cannot determine self_link for subnetwork "cft-vm-test-9wb5": network_interface.0.subnetwork_project: required field is not set

  with module.preemptible_and_regular_instance_templates.module.preemptible_and_regular_instance_templates.module.preemptible.google_compute_instance_template.tpl,
  on ../../../../modules/instance_template/main.tf line 65, in resource "google_compute_instance_template" "tpl":
  65: resource "google_compute_instance_template" "tpl" {

It's unclear to me if CFT believes the correct solution here is to set the project ID at provider level or to explicitly supply subnetwork_project. Not exactly clear why this is showing up as part of this PR as it's not a documented breaking change for TPG 5.0.

https://registry.terraform.io/providers/hashicorp/google/latest/docs/guides/version_5_upgrade

@tpdownes tpdownes requested a review from apeabody December 12, 2023 14:38
@g-awmalik g-awmalik self-assigned this Feb 8, 2024
@tpdownes tpdownes marked this pull request as ready for review February 16, 2024 20:55
@g-awmalik
Copy link
Contributor

@apeabody - can you LGTM this?

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @tpdownes!

@g-awmalik g-awmalik merged commit c7b47bc into terraform-google-modules:master Feb 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants