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 disabling EKS Auto Mode #41155

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

flostadler
Copy link
Contributor

@flostadler flostadler commented Jan 30, 2025

Description

Disabling Auto Mode for an EKS Cluster is currently broken. There's three different problems involved:

  1. If you remove all the auto mode config blocks, Change not triggered for optional / computed string field when set to empty string terraform-plugin-sdk#1101 strikes and kubernetes_network_config resolves to the previous value with enabled = true => Error: compute_config.enabled, kubernetes_networking_config.elastic_load_balancing.enabled, and storage_config.block_storage.enabled must all be set to either true or false
  2. If you modify any of the compute_config options (e.g. removing node_role_arn) while setting enabled = false it incorrectly marks it as a replacement.
  3. If you try to work around the previous two issues by removing the compute_config and storage_config blocks while setting kubernetes_network_config.elastic_load_balancing.enabled = false you get an API error from AWS: InvalidParameterException: The type for cluster update was not provided. This seems to be caused by the request not including the properties for compute_config and storage_config.

This change tries to tackle issue number 3 mentioned above (InvalidParameterException: The type for cluster update was not provided). In detail it changes how the UpdateClusterConfig request is assembled. Previously it wrongly omitted the ComputeConfig & StorageConfig blocks when they were removed from the config. Instead this should trigger sending a request that disables auto mode if it was previously enabled.

This intentionally doesn't touch upon kubernetes_network_config because that attribute won't become nil due to hashicorp/terraform-plugin-sdk#1101. Instead it just falls back to the previous value.

Relations

Relates #40582

References

Output from Acceptance Testing

make testacc TESTS=TestAccEKSCluster_ PKG=eks
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/eks/... -v -count 1 -parallel 20 -run='TestAccEKSCluster_'  -timeout 360m -vet=off
2025/01/30 13:19:44 Initializing Terraform AWS Provider...
=== RUN   TestAccEKSCluster_basic
=== PAUSE TestAccEKSCluster_basic
=== RUN   TestAccEKSCluster_disappears
=== PAUSE TestAccEKSCluster_disappears
=== RUN   TestAccEKSCluster_AccessConfig_create
=== PAUSE TestAccEKSCluster_AccessConfig_create
=== RUN   TestAccEKSCluster_AccessConfig_update
=== PAUSE TestAccEKSCluster_AccessConfig_update
=== RUN   TestAccEKSCluster_BootstrapSelfManagedAddons_update
=== PAUSE TestAccEKSCluster_BootstrapSelfManagedAddons_update
=== RUN   TestAccEKSCluster_BootstrapSelfManagedAddons_migrate
=== PAUSE TestAccEKSCluster_BootstrapSelfManagedAddons_migrate
=== RUN   TestAccEKSCluster_ComputeConfig_OnCreate
=== PAUSE TestAccEKSCluster_ComputeConfig_OnCreate
=== RUN   TestAccEKSCluster_ComputeConfig_OnUpdate
=== PAUSE TestAccEKSCluster_ComputeConfig_OnUpdate
=== RUN   TestAccEKSCluster_ComputeConfig_ModifyARN
=== PAUSE TestAccEKSCluster_ComputeConfig_ModifyARN
=== RUN   TestAccEKSCluster_Encryption_create
=== PAUSE TestAccEKSCluster_Encryption_create
=== RUN   TestAccEKSCluster_Encryption_update
=== PAUSE TestAccEKSCluster_Encryption_update
=== RUN   TestAccEKSCluster_Encryption_versionUpdate
=== PAUSE TestAccEKSCluster_Encryption_versionUpdate
=== RUN   TestAccEKSCluster_version
=== PAUSE TestAccEKSCluster_version
=== RUN   TestAccEKSCluster_logging
=== PAUSE TestAccEKSCluster_logging
=== RUN   TestAccEKSCluster_tags
=== PAUSE TestAccEKSCluster_tags
=== RUN   TestAccEKSCluster_VPC_securityGroupIDs
=== PAUSE TestAccEKSCluster_VPC_securityGroupIDs
=== RUN   TestAccEKSCluster_VPC_securityGroupIDsAndSubnetIDs_update
=== PAUSE TestAccEKSCluster_VPC_securityGroupIDsAndSubnetIDs_update
=== RUN   TestAccEKSCluster_VPC_endpointPrivateAccess
=== PAUSE TestAccEKSCluster_VPC_endpointPrivateAccess
=== RUN   TestAccEKSCluster_VPC_endpointPublicAccess
=== PAUSE TestAccEKSCluster_VPC_endpointPublicAccess
=== RUN   TestAccEKSCluster_VPC_publicAccessCIDRs
=== PAUSE TestAccEKSCluster_VPC_publicAccessCIDRs
=== RUN   TestAccEKSCluster_Network_serviceIPv4CIDR
=== PAUSE TestAccEKSCluster_Network_serviceIPv4CIDR
=== RUN   TestAccEKSCluster_Network_ipFamily
=== PAUSE TestAccEKSCluster_Network_ipFamily
=== RUN   TestAccEKSCluster_Outpost_create
=== PAUSE TestAccEKSCluster_Outpost_create
=== RUN   TestAccEKSCluster_Outpost_placement
=== PAUSE TestAccEKSCluster_Outpost_placement
=== RUN   TestAccEKSCluster_RemoteNetwork_Node
=== PAUSE TestAccEKSCluster_RemoteNetwork_Node
=== RUN   TestAccEKSCluster_RemoteNetwork_Pod
=== PAUSE TestAccEKSCluster_RemoteNetwork_Pod
=== RUN   TestAccEKSCluster_upgradePolicy
=== PAUSE TestAccEKSCluster_upgradePolicy
=== RUN   TestAccEKSCluster_zonalShiftConfig
=== PAUSE TestAccEKSCluster_zonalShiftConfig
=== CONT  TestAccEKSCluster_basic
=== CONT  TestAccEKSCluster_tags
=== CONT  TestAccEKSCluster_ComputeConfig_OnUpdate
=== CONT  TestAccEKSCluster_logging
=== CONT  TestAccEKSCluster_version
=== CONT  TestAccEKSCluster_Encryption_versionUpdate
=== CONT  TestAccEKSCluster_Encryption_update
=== CONT  TestAccEKSCluster_Encryption_create
=== CONT  TestAccEKSCluster_ComputeConfig_ModifyARN
=== CONT  TestAccEKSCluster_AccessConfig_create
=== CONT  TestAccEKSCluster_AccessConfig_update
=== CONT  TestAccEKSCluster_ComputeConfig_OnCreate
=== CONT  TestAccEKSCluster_BootstrapSelfManagedAddons_migrate
=== CONT  TestAccEKSCluster_disappears
=== CONT  TestAccEKSCluster_Network_ipFamily
=== CONT  TestAccEKSCluster_zonalShiftConfig
=== CONT  TestAccEKSCluster_BootstrapSelfManagedAddons_update
=== CONT  TestAccEKSCluster_upgradePolicy
=== CONT  TestAccEKSCluster_RemoteNetwork_Node
=== CONT  TestAccEKSCluster_Outpost_placement
    cluster_test.go:1119: skipping since no Outposts found
--- SKIP: TestAccEKSCluster_Outpost_placement (2.37s)
=== CONT  TestAccEKSCluster_RemoteNetwork_Pod
--- PASS: TestAccEKSCluster_upgradePolicy (561.55s)
=== CONT  TestAccEKSCluster_Outpost_create
    cluster_test.go:1086: skipping since no Outposts found
--- SKIP: TestAccEKSCluster_Outpost_create (0.66s)
=== CONT  TestAccEKSCluster_VPC_endpointPublicAccess
--- PASS: TestAccEKSCluster_RemoteNetwork_Pod (573.71s)
=== CONT  TestAccEKSCluster_VPC_securityGroupIDsAndSubnetIDs_update
--- PASS: TestAccEKSCluster_AccessConfig_create (590.00s)
=== CONT  TestAccEKSCluster_Network_serviceIPv4CIDR
--- PASS: TestAccEKSCluster_zonalShiftConfig (606.45s)
=== CONT  TestAccEKSCluster_VPC_endpointPrivateAccess
--- PASS: TestAccEKSCluster_disappears (618.52s)
=== CONT  TestAccEKSCluster_VPC_publicAccessCIDRs
--- PASS: TestAccEKSCluster_basic (638.85s)
=== CONT  TestAccEKSCluster_VPC_securityGroupIDs
--- PASS: TestAccEKSCluster_Encryption_create (640.85s)
--- PASS: TestAccEKSCluster_tags (648.84s)
--- PASS: TestAccEKSCluster_RemoteNetwork_Node (660.05s)
--- PASS: TestAccEKSCluster_AccessConfig_update (708.06s)
--- PASS: TestAccEKSCluster_logging (857.91s)
--- PASS: TestAccEKSCluster_ComputeConfig_OnUpdate (942.64s)
--- PASS: TestAccEKSCluster_ComputeConfig_OnCreate (960.67s)
--- PASS: TestAccEKSCluster_BootstrapSelfManagedAddons_update (1124.76s)
--- PASS: TestAccEKSCluster_VPC_securityGroupIDs (591.09s)
--- PASS: TestAccEKSCluster_Encryption_versionUpdate (1242.22s)
--- PASS: TestAccEKSCluster_version (1276.76s)
--- PASS: TestAccEKSCluster_Network_ipFamily (1283.58s)
--- PASS: TestAccEKSCluster_ComputeConfig_ModifyARN (1403.20s)
--- PASS: TestAccEKSCluster_VPC_publicAccessCIDRs (873.23s)
--- PASS: TestAccEKSCluster_BootstrapSelfManagedAddons_migrate (1598.01s)
--- PASS: TestAccEKSCluster_Encryption_update (1611.12s)
--- PASS: TestAccEKSCluster_VPC_endpointPublicAccess (1117.26s)
--- PASS: TestAccEKSCluster_Network_serviceIPv4CIDR (1150.45s)
--- PASS: TestAccEKSCluster_VPC_endpointPrivateAccess (1383.69s)
--- PASS: TestAccEKSCluster_VPC_securityGroupIDsAndSubnetIDs_update (1490.51s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/eks        2074.795s

@flostadler flostadler requested a review from a team as a code owner January 30, 2025 13:25
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/eks Issues and PRs that pertain to the eks service. needs-triage Waiting for first response or review from a maintainer. labels Jan 30, 2025
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. service/eks Issues and PRs that pertain to the eks service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants