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 terraform output for cluster names that begin with a digit #12202

Merged
merged 4 commits into from
Aug 27, 2021

Conversation

rifelpet
Copy link
Member

@rifelpet rifelpet commented Aug 24, 2021

This will fail until we address each resource type generating terraform resource names that are purely the cluster name

This now includes a proposed fix.

Release notes will need to mention something along the lines of:

terraform state mv aws_iam_openid_connect_provider.123-example-com aws_iam_openid_connect_provider.provider-123-example-com
terraform state mv aws_internet_gateway.123-example-com aws_internet_gateway.gateway-123-example-com
terraform state mv aws_route_table.123-example-com aws_route_table.table-123-example-com
terraform state mv aws_vpc.123-example-com aws_vpc.vpc-123-example-com
terraform state mv aws_vpc_dhcp_options.123-example-com aws_vpc_dhcp_options.options-123-example-com
terraform state mv aws_vpc_dhcp_options_association.123-example-com aws_vpc_dhcp_options_association.association-123-example-com

I left off the s3 managed files because they're still under feature flag, though terraform may require a second terraform apply to get them into a consistent state (race conditions with the same objects being deleted by the old name and created by the new name)

fixes: #12199

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 24, 2021
@rifelpet rifelpet changed the title Add integration test for cluster names beginning with a digit Fix terraform output for cluster names that begin with a digit Aug 24, 2021
@rifelpet rifelpet force-pushed the digit branch 2 times, most recently from 28f84fa to e21c767 Compare August 27, 2021 03:01
@johngmyers
Copy link
Member

Why not simply use a fixed prefix instead of something derived from the resource type?

In any case, this can conflict with clusters whose name matches, modulo one of the used prefixes. Perhaps we can use prefix_ instead of prefix-?

@rifelpet
Copy link
Member Author

I will update this to use the literal prefix_ instead of resource types

This will fail until we address each resource type generating terraform resource names that are purely the cluster name
docs/releases/1.22-NOTES.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 27, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johngmyers

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2021
@k8s-ci-robot k8s-ci-robot merged commit ea37cc2 into kubernetes:master Aug 27, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/documentation cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
3 participants