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

Unbreak AWS cluster naming #30

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Unbreak AWS cluster naming #30

merged 1 commit into from
Dec 9, 2024

Conversation

moio
Copy link
Contributor

@moio moio commented Dec 6, 2024

The AWS backend got inadvertently broken by #19

go code expects specific cluster names: upstream, tester, and downstream + a suffix

Current AWS code returns names like upstream-0 or tester-1 which result in dartboard deploy failing.

This PR removes the ability to set a prefix and adds suffix numbers only to downstream to keep the contract with the go part of the code.

Tested manually on AWS.

Signed-off-by: Silvio Moioli <[email protected]>
@moio moio requested a review from git-ival December 6, 2024 10:15
Copy link
Contributor

@git-ival git-ival left a comment

Choose a reason for hiding this comment

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

I'm not sure this was necessarily needed since the defaults still made the module functional. I would have preferred if we didn't need to rely on explicitly naming clusters based on where they are in the "stream", but LGTM.

@moio
Copy link
Contributor Author

moio commented Dec 9, 2024

Well in my tests defaults were actually broken, I can imagine there were other differences between my setup and yours.

About ordering: if we have any reasons for distinguishing downstream clusters based on the template they were created from, I am not against either going back to downstream-$TEMPLATE_INDEX-$CLUSTER_INDEX or even downstream-$TEMPLATE_NAME-$CLUSTER_INDEX. It's just that I did not see a need right now.

Happy to complicate as soon as there is a need.

Thanks, merging for now, we can iterate further later.

@moio moio merged commit 2d87743 into rancher:main Dec 9, 2024
1 check passed
@moio moio deleted the unbreak_aws branch December 9, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants