-
Notifications
You must be signed in to change notification settings - Fork 368
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
feat: mig labels & most disruptive update policy action update #381
Conversation
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. |
/gcbrun |
@rauny-brandao - thanks for the PR. Can you please take a look at the lint failures? |
@g-awmalik linting is fixed. Is there a way to see the failures for: |
/gcbrun |
@g-awmalik please let me know if we need something else for this to be merged, thank you! |
/gcbrun |
Since this update, I encounter this error :
As stated in the official documentation, the
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 NB2 : the optional TF parameter was released on Sept. 2022 (1.3.0 TF version) |
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"
... |
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 thedate
and setdefaultValue: null
in the metadata.yml files. Please let me know if I should keep them.