Add support for maintenance window on google_container_cluster #670
Conversation
michaelbannister
changed the title
|
@danawillow I can merge master to my branch to resolve conflicts - or would you prefer I rebase it? (I'm aware of the dangers of doing this if someone has already cloned it.) |
|
I'm fine with either! |
|
Great - I'll do it this evening (UK time) |
michaelbannister
force-pushed
the
container-cluster-maintenance-window
branch
from
2d81bde to
6bc7cc8
Compare
michaelbannister
force-pushed
the
container-cluster-maintenance-window
branch
from
6bc7cc8 to
25bb227
Compare
|
@danawillow I've rebased and look forward to any feedback you or others may have. |
There was a problem hiding this comment.
This looks great, @michaelbannister! Just a few super-quick changes and then this will be good to go.
After making the test change I noted:
$ make testacc TEST=./google TESTARGS='-run=TestAccContainerCluster_withMaintenanceWindow'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./google -v -run=TestAccContainerCluster_withMaintenanceWindow -timeout 120m
=== RUN TestAccContainerCluster_withMaintenanceWindow
--- PASS: TestAccContainerCluster_withMaintenanceWindow (357.51s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google 357.679s
| Schema: map[string]*schema.Schema{ | ||
| "daily_maintenance_window": { | ||
| Type: schema.TypeList, | ||
| Required: true, |
There was a problem hiding this comment.
Just for the sake of future-proofing, let's go ahead and ForceNew: true this one too
| Config: testAccContainerCluster_withMaintenanceWindow("03:00"), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckContainerCluster( | ||
| "google_container_cluster.primary"), |
There was a problem hiding this comment.
this should be "google_container_cluster.with_maintenance_window" to match the resource name in the test config
| @@ -243,6 +260,10 @@ exported: | |||
| * `instance_group_urls` - List of instance group URLs which have been assigned | |||
| to the cluster. | |||
|
|
|||
| * `maintenance_policy.daily_maintenance_window.duration` - Duration of the time window, automatically chosen to be | |||
There was a problem hiding this comment.
Nested fields are fun! This is actually maintenance_policy.0.daily_maintenance_window.0.duration
There was a problem hiding this comment.
Oh yes - I see this was recently updated for the master_auth stuff (this explains why it didn't work when I tried to use those attributes without the 0 index recently!)
- Set ForceNew: true on the schema element daily_maintenance_window - Correct resource name in acceptance test - Correct documentation of resource attribute maintenance_policy.0.daily_maintenance_window.0.duration
|
Thanks, Dana! |
|
You're welcome, thanks for the quick turnaround! |
|
Hi, thanks for that! Just came across an issue though: Will open an issue. |
…corp#670) * Add support for maintenance window on google_container_cluster (hashicorp#526) * Address review comments - Set ForceNew: true on the schema element daily_maintenance_window - Correct resource name in acceptance test - Correct documentation of resource attribute maintenance_policy.0.daily_maintenance_window.0.duration
Signed-off-by: Modular Magician <[email protected]>
|
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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #526
This is my first venture into both the Go language and the internals of Terraform. Please be forthcoming with feedback but don't be too harsh if I've done something dumb! ;)
I have run my own standalone test creating a cluster with a maintenance window set, but I haven't yet run the acceptance tests, because the readme says it costs money but gives no indication of how much money that might be!