Update GKE versions to something that will support NetApp + Quotas

[athornton]

We need at least 6.21 for Netapp Volume Quotas.

[athornton]

Corresponds to https://github.com/lsst/idf_deploy/actions/runs/14230558971/job/39880180904

The objects to be created look plausible to me.

[dspeck1]

Thecluster-science-platform-dev-7696 service account is for dev only and is unique per project. Please add the service accounts from demo, int, and stable, too. In the plans the service account membership in clusterAdmin is removed.

[athornton]

Turns out we're gonna have to edit the state directly to do this.

[dspeck1]

I would consider removing integration and production tfvars from this PR. Granted this is a new feature, but there is a module upgrade. Safer to validate dev first then deploy int and prod in a separate PR.

[dhirving]

In a future PR you might consider moving many of these definitions out of the variables file and into static module instantiatiations in the main terraform.

That would make it easier to get consistent deployments between environments without relying on copy/pasting correctly between the variables files.

[athornton]

It would, but they're still going to vary across environments (sizes and default quotas and user quota overrides, at least, will differ). I think it will be less confusing to have the whole volume definition in the same place.

[dhirving]

You could do variables like home_volume_size, scratch_volume_default_quota -- only having variables for the parts that vary between the environments.

[dhirving]

I think this depends_on is backwards -- the definition of the google_netapp_volume is referencing these backup policies, not the other way around.

If you make the change I recommended above, I think you can just remove this since Terraform will already be aware of the dependency.

[dhirving]

It would be better to specify this as google_netapp_backup_policy.instance[0].id -- less error-prone and it makes Terraform aware of the dependency, so it will set up infrastructure in the correct order.

[athornton]

I mean, maybe, but doing this and the next thing requires plumbing at least one more variable through and apparently I need some kind of explicit dependency because it hung on backup_vault_id . And since I know what that string looks like and I'm responsible for creating these objects, I don't think it's any more error-prone.

We don't actually care about the ordering at this stage: the worst thing that happens is that we somehow don't get our first scheduled backup, which, since it's a new volume, is not a big deal. By the time we get the initial data copy to the new volume done, the backup vault and policy will both long have existed.

[dhirving]

This one shouldn't require any extra variable plumbing -- you literally just replace the string with the module reference. The backup vault one does need a variable but that kind of dependency injection is a very standard terraform pattern.

Not sure what you mean by "hung" -- passing the variable through is how you declare the dependency.

The point of terraform is to allow a future developer to know what infrastructure was created and how all the pieces tie together. If you're using an implied string interpolation buried in a module, it's a lot more difficult for a future reader to trace back to where the resource is being created.

And you do care about the ordering -- the terraform deployment might fail if the volume is instantiated before the backup vault or backup policy, because the Google API will reject the reference to a non-existent resource. This is not going to be deterministic unless Terraform can see the dependency between the two resources.

[dhirving]

Similar to the above, it would be better to pass in the ID of this vault from the top level instead of making an assumption about its location here.

[dhirving]

In a future PR you might consider flattening out this object into separate variables here... as you discovered earlier in the week, Terraform's type-coercion rules can often lead to surprising errors with complex types. You'll get better error messages for typos if these are just module-level variables.

[dhirving]

Looks good to me other than what Dan already called out. And you checked on the billing account change, right?

[athornton]

Looks good to me other than what Dan already called out. And you checked on the billing account change, right?

Yeah, Dan said it was just the budget and was harmless.