Skip to content
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

fix: Supports setting cluster_compute_config = null or cluster_compute_config.enabled = false #3308

Conversation

lorengordon
Copy link
Contributor

@lorengordon lorengordon commented Feb 22, 2025

Description

Couple logic issues around cluster_compute_config that this is addressing:

  1. The logic around enabling Auto Mode is based on the value never being null, otherwise the length() function fails, so we use nullable = false in the variable definition so that a null value will be interpreted as an empty map (the default value).
  2. The API will return an error if compute_config.enabled is set to false, when every other required option for auto mode is also not provided. So we just condition the block on local.auto_mode_enabled so the entire block is absent.

Motivation and Context

Fixes #3307
Fixes #3273

Breaking Changes

None

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

The logic around enabling Auto Mode is based on the value never being
null, so use `nullable = false` in the variable definition so that a
null value will be interpreted as nn empty map (the default value).

Fixes terraform-aws-modules#3307
The API will return an error if compute_config.enabled is set to false,
when every other required option for auto mode is also not provided.

```
│ Error: creating EKS Cluster (test-minimum-inputs-kuneic): operation error EKS: CreateCluster, https response error StatusCode: 400, RequestID: 573c664d-68c6-463c-b117-ba12722ebe96, InvalidParameterException: For EKS Auto Mode, please ensure that all required configs, including computeConfig, kubernetesNetworkConfig, and blockStorage are all either fully enabled or fully disabled.
```

Seems like an odd API choice, but that's how it works.

Fixes terraform-aws-modules#3307
@bryantbiggs
Copy link
Member

This doesn't solve it, has to be fixed upstream first #3273

@lorengordon
Copy link
Contributor Author

The issue you linked to is about an existing cluster. These errors occur in a brand new cluster. Please reopen and reconsider a little further.

@lorengordon
Copy link
Contributor Author

lorengordon commented Feb 22, 2025

Alright, @bryantbiggs, I've figured out the issue with disabling Auto Mode. It's not really an upstream issue, as much as a difficult API. But this module can support enabling and disabling Auto Mode, with some limitations. I'm pushing the fix to this branch, the commit has the full explanation. If you'll please reopen and review, it would be much appreciated!

@bryantbiggs
Copy link
Member

we are not making any further changes to enable/disable of Auto Mode until hashicorp/terraform-provider-aws#41155 lands upstream

@lorengordon
Copy link
Contributor Author

I don't think that upstream patch is actually a solution to the issue. It can't fix the CreateCluster API, and that is what is imposing the limitations. It's not a provider issue.

@lorengordon
Copy link
Contributor Author

Well I guess the PR doesn't update once closed, but the branch has the fix. You can review the changeset here, instead, master...lorengordon:terraform-aws-eks:fix/compute-config-false-or-null

I'll leave the branch, so It'll be waiting for you whenever you get around to reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants