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

Modify "Eventbridge Rule" resource patches to include spec.prameters.id as prefix #33

Closed
wants to merge 1 commit into from

Conversation

moonorb
Copy link

@moonorb moonorb commented Jul 19, 2024

Description of your changes

Fixes #485-676

I have:

  • Read and followed Upbound's contribution process.
  • [] Run make reviewable to ensure this PR is ready for review.
  • [] Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

…ernal-names to allow multiple Karpenter instances to create unique resources on the same AWS account and same management cluster
@ytsarev
Copy link
Member

ytsarev commented Aug 1, 2024

Hi @moonorb . Thanks a lot for your contribution. Can you please elaborate on Description Fixes #485-676 doesn't say much :)

@moonorb
Copy link
Author

moonorb commented Aug 14, 2024

Sure @ytsarev,

Some of the resources related to Karpenter are created outside the EKS cluster on AWS account level. For example "kind: Rule" at #485. As a result the external names that are created must be unique if there are more than one Karpenter instance running in a single AWS account. The code works without any problem if there is one instance of Karpenter.

If you look at the patch for "ruleHealthEvent" resource at #482 it creates the EventBridge Rule using regexp with a static name which is called "healthevent". As a result a second Karpenter will not be able to provision this resource due to the name conflict. I fixed this by appending a prefix(id) to the external names by modifying the patch for this resource and others below this one. This enables the Eventbridge rules to be craeted with unique names. I hope I was able to clarify :)

@haarchri
Copy link
Member

/test-examples

@haarchri
Copy link
Member

thanks for fixing this - we added this change in #34 - and we bump karpenter to v1

@haarchri haarchri closed this Aug 16, 2024
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.

3 participants