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

[experiment] Switch from LocalID to Alias #8025

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

linki
Copy link
Member

@linki linki commented Sep 2, 2024

This is an attempt to overcome the sometimes cryptic use of Cluster.LocalID.

In the past we didn't parametrize our cluster-wide resources (such as IAM roles) because there was no need for it since the Kubernetes stack was only created once per account. The introduction of multiple e2e clusters in the very same e2e account changed that and let to the need to occasionally add the Cluster.LocalID parameter to the resource names. However, the list grew and with the outlook of having multiple clusters in many accounts will only intensify.

Unfamiliar users, such as new team members or other platform teams, are even more surprised by the need of this parameter as they haven't been exposed to multiple clusters in the accounts up until now.

In order to make this concept more clear and understandable, this PR changes it to use the Cluster.Alias. The alias is globally unique and hence will also be unique in the same account, like Cluster.LocalID. In addition, the more familiar alias value is easier to understand and unforeseen future name conflicts are avoided proactively by making the alias part of each AWS resource name.

@linki linki added experiment do-not-merge major Major feature changes or updates, e.g. feature rollout to a new country, new API calls. labels Sep 2, 2024
@linki linki changed the title Switch from LocalID to Alias [experiment] Switch from LocalID to Alias Sep 11, 2024
@@ -556,7 +556,7 @@ Resources:
Resource: '*'
Version: 2012-10-17
PolicyName: root
RoleName: "{{.Cluster.LocalID}}-app-autoscaler"
RoleName: "cluster-autoscaler-{{ .Cluster.Alias }}"
Copy link
Member Author

@linki linki Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should think about keeping the app- prefix as a kind of namespace to avoid clashing with other resources. Let's say there's another role related to the autoscaler for a different purpose, it could still lead to clashes although less likely because it's in the same project.

@linki
Copy link
Member Author

linki commented Sep 20, 2024

Alright, roll out is actually not that difficult: Instead of modifying the IAM roles in place, we can just make duplicate roles. Then change the annotations on the Kubernetes resources in a separate PR. Once everything is rolled out, remove the old IAM roles from the Stack and AWS will clean them up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge experiment major Major feature changes or updates, e.g. feature rollout to a new country, new API calls.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant