When using instance templates, if you use one of our image shorthands at
the moment, you'll get a perma-diff. This is because the config gets
resolved to another format before it is set in state, and so we need to
set that other format in state.
Unfortunately, resolving images requires network access, so we have to
do this with CustomizeDiff. CustomizeDiff was having trouble (I think?
More on this below) on setting a field as not ForceNew once the field
was already set, so I moved the decision for whether a field was
ForceNew or not into CustomizeDiff. I also resolved the old and new
images, and if they were the same, cleared the diff.
Unfortunately, you can't actually clear a field on a sub-block right
now. You have to clear top-level fields only. So this will currently
throw an error. I opened hashicorp/terraform#18795 to fix that. Once
that's merged, and we vendor it here, this patch fixes the problem.
If hashicorp/terraform#18795 doesn't get merged, the next best
workaround is to keep track of _all_ the fields under `disk` with a diff
in our CustomizeDiff, check whether they've all changed or not, and if
they've all changed, clear the changes on `disk`, which I _think_ will
resolve the issue. That's just a massive pain, unfortunately.
Hypothetically fixes#1643.
@thomasriley, are you able to patch this change into your provider to see if it fixed the problem? I haven't been able to get a working repo so I haven't verified the fix yet.
Commit 8f31fec introduced a bug for the 'service_account_key' resource
where it required a project be set either in the provider or in the
resource for 'service_account_key', but a project isn't required if the
service account is a service account fully qualified name or a service
account email.
This PR relaxes the requirement that a project needs to be set for the
'service_account_key' resource, 'service_account' datasource and
'service_account_key' datasource, but will error if we try to build a
fully qualified name from a service account id when no project can be
found.
This also cleans up 'serviceAccountFQN' so it is slightly easier to
follow and return an error if there is no project but we need one to
build the service account fully qualified name.
Fixes: #1655
When reading a project, both App Engine and Billing would fail if
_neither_ the default project the provider was configured with nor the
project being targeted had the App Engine Admin or Billing APIs
(respectively) enabled. We didn't catch this because our source project
obviously has both enabled.
This fixes the matter by checking the error returned from each of those,
and if it looks like an API not enabled error, logging it at warning
level instead of returning it as an error, which will let the read
proceed as usual.
* Revert "Merge pull request #1434 from terraform-providers/paddy_revert_beta"
This reverts commit 118cd71201, reversing
changes made to d59fcbbc59.
* add ConvertSelfLinkToV1 calls to places where beta links are stored
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 util for handling wrapped/raw google api errors
* add 404 error handling to shared iam files
* fix errwrap calls in iam files
* fix import
* remove newlines, clear ID for removed state in iam binding
* Add Set method to TerraformResourceData and ResourceDataMock
* Add Id() and SetId() to ResourceDataMock and TerraformResourceData
* Keep only name when reading region or zone field to handle api that returns self_link
- Fetch Zone attribute any place where it *was* being fetched from the schema by
combination schema / provider-level attribute.
- Allow region to be unspecified if zone is specified.
- Switch one example to using provider-level zone as an example.
- Make provider-level zone optional. (Individual resources will fail if they can't find a zone.)
- Add tests for getZone and getRegion.
* Move AliasIpRange helpers into utils
To reflect the fact they'll be used by multiple resources.
* Pass Config to build helpers, not meta
It's the only thing meta is used for.
* Refactor getNetwork util methods to return early for the happy path.
* Update compute APIs
compute.Instance.MinCpuPlatform is now GA.
* Fix panic in TestComputeInstanceMigrateState
This seemed to be a pre-existing issue, i.e. I could repro it in master.
--- FAIL: TestComputeInstanceMigrateState (0.00s)
panic: interface conversion: interface {} is nil, not *google.Config [recovered]
panic: interface conversion: interface {} is nil, not *google.Config
goroutine 85 [running]:
testing.tRunner.func1(0xc4205d60f0)
/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:711 +0x2d2
panic(0x203acc0, 0xc4205d2080)
/usr/local/Cellar/go/1.9.1/libexec/src/runtime/panic.go:491 +0x283
github.com/terraform-providers/terraform-provider-google/google.migrateStateV3toV4(0xc4205f2000, 0x0, 0x0, 0x0, 0x48, 0xc4205f2000)
/Users/negz/control/go/src/github.com/terraform-providers/terraform-provider-google/google/resource_compute_instance_migrate.go:182 +0x2405
github.com/terraform-providers/terraform-provider-google/google.resourceComputeInstanceMigrateState(0x2, 0xc4205f2000, 0x0, 0x0, 0x0, 0x0, 0xe0000000000)
/Users/negz/control/go/src/github.com/terraform-providers/terraform-provider-google/google/resource_compute_instance_migrate.go:48 +0x21a
github.com/terraform-providers/terraform-provider-google/google.runInstanceMigrateTest(0xc4205d60f0, 0x2260816, 0x8, 0x227d23a, 0x20, 0x2, 0xc4205ec0f0, 0xc4205ec120, 0x0,
0x0)
/Users/negz/control/go/src/github.com/terraform-providers/terraform-provider-google/google/resource_compute_instance_migrate_test.go:803 +0xc1
github.com/terraform-providers/terraform-provider-google/google.TestComputeInstanceMigrateState(0xc4205d60f0)
/Users/negz/control/go/src/github.com/terraform-providers/terraform-provider-google/google/resource_compute_instance_migrate_test.go:71 +0xc84
testing.tRunner(0xc4205d60f0, 0x22d81c0)
/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:789 +0x2de
FAIL github.com/terraform-providers/terraform-provider-google/google 0.035s
* Use only the v1 API for resource_compute_instance
Alias IP ranges, Accelerators, and min CPU platform are now GA.
* Move common instance code into utils.go
Methods used by both resource_compute_instance and
resource_compute_instance_template are currently spread between their respective
files, and utils.go.
This commit moves them all into utils.go for the sake of consistency. It may be
worth considering an instance_common.go file or similar.
* Unify compute_instance and compute_instance_template network_interface and service_account code
This has the side effect of enabling Alias IP range support for
compute_instance_templates.
* Add tests for compute instance template Alias IP ranges
* Mark instance template region as computed
We compute it from the subnet its network interfaces are in. Note this
is not new behaviour - I believe it was erroneously missing the computed
flag.
* Support guest accelerators for instance templates
Since most of the code is already there.
* Add a test for using 'address' rather than 'network_ip' for instance templates
* Don't mark assigned_nat_ip as deprecated
* Remove network_interface schema fields that don't make sense for a compute instance template
* Add newline after count in instance template docs
* Don't try to dedupe guest accelerator expansion code
The API calls to Google to create guest accelerators take different values
for instances and instance templates. Instance templates don't have a zone
and can thus *only* be passed a guest accelerator name.
* Use ParseNetworkFieldValue instead of getNetworkLink
* Add support for parsing regional fields, and subnetworks specifically
Currently unused because subnetworks may have a separate project from that
of the instance using them, which complicates looking up the project field.
* Fall back to provider region when parsing regional field values
Also slightly refactors getXFromSchema field helper functions for readability.
* Revert to assigned_nat_ip in compute instance docs
* Add beta scaffolding to compute instance and compute instance template
Note these resources don't currently use beta features - this is futureproofing.
* Fix indentation in comment about instance template alias IP ranges
* Consolidate metadata helper functions in metadata.go
* Move compute instance (and template) related helpers into their own file
* Relax diff on maintenance_policy.daily_maintenance_window.start_time
If the maintenance window has been set outside of Terraform to a time with a
single-digit hour (such as 1:00), and the terraform definition is set to the
same hour but with a leading zero as per validation (i.e. 01:00), do not
consider the time to be changed (as we currently don't support update on this
property).
Fixes#719
* Generalise rfc3339TimeDiffSuppress and add more test cases
