Fixes#1702.
@chrisst I'm putting you as a reviewer, but no rush. Feel free to ask as many questions as you have! Also feel free to offer suggestions 😃 (or just say it's perfect as-is, that works too)
This PR also switched us to using the beta API in all cases, and that had a side effect which is worth noting, note included here for posterity.
=====
The problem is, we add a GPU, and as per the docs, GKE adds a taint to
the node pool saying "don't schedule here unless you tolerate GPUs",
which is pretty sensible.
Terraform doesn't know about that, because it didn't ask for the taint
to be added. So after apply, on refresh, it sees the state of the world
(1 taint) and the state of the config (0 taints) and wants to set the
world equal to the config. This introduces a diff, which makes the test
fail - tests fail if there's a diff after they run.
Taints are a beta feature, though. :) And since the config doesn't
contain any taints, terraform didn't see any beta features in that node
pool ... so it used to send the request to the v1 API. And since the v1
API didn't return anything about taints (since they're a beta feature),
terraform happily checked the state of the world (0 taints I know about)
vs the config (0 taints), and all was well.
This PR makes every node pool refresh request hit the beta API. So now
terraform finds out about the taints (which were always there) and the
test fails (which it always should have done).
The solution is probably to write a little bit of code which suppresses
the report of the diff of any taint with value 'nvidia.com/gpu', but
only if GPUs are enabled. I think that's something that can be done.
* add support for `google_container_node_pool` management (sans-tests)
* [review] add tests, docs, general cleanup
* add docs
* [review] amend test to check updates and terraform fmt
* test updates, make nested management fields non-computed
This reverts commit 8ab9d96d25 and revives
the original commit that adds t.Parallel to all acceptance tests. It
turns out test failures were unrelated to this change (rather, they were
related to quota issues).
This reverts commit 42de44592f. It appears
there might be thread-safety issues as panics have started occuring when
parallism is ramped up. Reverting for now while investigating.
* Add preemptible as an option to node config
* Check for preemptible in test matching functions
* Move flattenClusterNodeConfig to node_config
* Handle bools properly when comparing in cluster and node pool tests
* Use a supported image_type in cluster tests
* Add support node config for GKE node pool
* Review fixes:
- Set max items in node config schema
- Fill missing node config fields
- Put test helpers above than test vars
* Update checks in node pool tests
* Fix node pool check match
