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: mig labels & most disruptive update policy action update #381

Merged
merged 7 commits into from
Feb 23, 2024
Merged

Conversation

rauny-brandao
Copy link
Contributor

@rauny-brandao rauny-brandao commented Feb 20, 2024

Adds support on mig to most_disruptive_allowed_action and to all_instances_config labels

To create this PR, I ran make build, which updated the date and set defaultValue: null in the metadata.yml files. Please let me know if I should keep them.

@rauny-brandao rauny-brandao requested a review from a team as a code owner February 20, 2024 10:07
Copy link

google-cla bot commented Feb 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

@rauny-brandao - thanks for the PR. Can you please take a look at the lint failures?

@rauny-brandao
Copy link
Contributor Author

@g-awmalik linting is fixed. Is there a way to see the failures for: terraform-google-vm-int-trigger?

@g-awmalik
Copy link
Contributor

/gcbrun

@rauny-brandao
Copy link
Contributor Author

@g-awmalik please let me know if we need something else for this to be merged, thank you!

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik g-awmalik merged commit 61ba9bf into terraform-google-modules:master Feb 23, 2024
4 checks passed
@jlenuffgsoi
Copy link

jlenuffgsoi commented Apr 2, 2024

Since this update, I encounter this error :

│ Error: Invalid value for input variable
│ 
│   on .terraform/modules/mymod/modules/gcp-mig-mymod/mig.tf line 44, in module "mig":
│   44:   update_policy = [{
│   45:     instance_redistribution_type = "PROACTIVE"
│   46:     max_surge_fixed              = 100
│   47:     max_surge_percent            = null
│   48:     max_unavailable_fixed        = null
│   49:     max_unavailable_percent      = null
│   50:     min_ready_sec                = null
│   51:     minimal_action               = "REPLACE"
│   52:     replacement_method           = null
│   53:     type                         = "PROACTIVE"
│   54:   }]
│ 
│ The given value is not suitable for module.mymod.module.mig.var.update_policy declared at .terraform/modules/mymod.mig/modules/mig/variables.tf:100,1-25: element 0: attribute "most_disruptive_allowed_action" is required.
╵

As stated in the official documentation, the update_policy block have these inputs :

type (required)
instance_redistribution_type (optional)
minimal_action (required)
most_disruptive_allowed_action (optional)
max_surge_percent  (optional)
max_unavailable_fixed  (optional)
min_ready_sec  (optional)
replacement_method (optional)

So, why can we replace this variable block :

variable "update_policy" {
  description = "The rolling update policy. https://www.terraform.io/docs/providers/google/r/compute_region_instance_group_manager#rolling_update_policy"
  type = list(object({
    max_surge_fixed                = number
    instance_redistribution_type   = string
    max_surge_percent              = number
    max_unavailable_fixed          = number
    max_unavailable_percent        = number
    min_ready_sec                  = number
    replacement_method             = string
    minimal_action                 = string
    type                           = string
    most_disruptive_allowed_action = string
  }))
  default = []
}

by :

variable "update_policy" {
  description = "The rolling update policy. https://www.terraform.io/docs/providers/google/r/compute_region_instance_group_manager#rolling_update_policy"
  type = list(object({
    instance_redistribution_type   = optional(string)
    max_surge_percent              = optional(number)
    max_unavailable_fixed          = optional(number)
    min_ready_sec                  = optional(number)
    replacement_method             = optional(string)
    minimal_action                 = string
    type                           = string
    most_disruptive_allowed_action = optional(string)
  }))
  default = []
}

?

With this code, the error disappears.

NB : the max_surge_fixed and the max_unavailable_percent are not used anymore

NB2 : the optional TF parameter was released on Sept. 2022 (1.3.0 TF version)

@jlenuffgsoi
Copy link

jlenuffgsoi commented Apr 2, 2024

I think this is because you want to be as much compliant as possible : https://github.com/terraform-google-modules/terraform-google-vm/blob/master/modules/mig/versions.tf#L18C3-L18C32

...
terraform {
  required_version = ">=0.13.0"
...

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