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

Use substring check for EKS cluster name provider #2227

Merged
merged 1 commit into from
May 30, 2024

Conversation

therealdwright
Copy link
Contributor

Summary of your changes

Karpenter managed clusters will use aws:eks:cluster-name for its
cluster name tag. This commit updates the logic to use a substring
check so both the existing tag of eks:cluster-name and karpenter
are supported.

Related Issues

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

@therealdwright therealdwright requested a review from a team as a code owner May 23, 2024 23:42
Copy link

mergify bot commented May 23, 2024

This pull request does not have a backport label. Could you fix it @therealdwright? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
    NOTE: backport-skip has been added to this pull request.

Copy link

github-actions bot commented May 24, 2024

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 359
⬜ Skipped 33

@moukoublen
Copy link
Member

Hello @therealdwright, and thanks for opening the issue and the PR,

One minor concern: Would it be safer to compare using ends with instead of contains?

I am not sure if it's actually needed. I just thought of cases where tags could perhaps contain eks:cluster-name in the name but refer to a different thing.

@therealdwright
Copy link
Contributor Author

I think this is a fair criticism and I am only personally interested in ends with but was trying to be robust but happy to boil this one down to YAGNI.

@therealdwright therealdwright force-pushed the improve-cluster-name-check branch from 57e7511 to 88cea46 Compare May 28, 2024 21:34
Karpenter managed clusters will use `aws:eks:cluster-name` for its
cluster name tag. This commit updates the logic to use a substring
check so both the existing tag of `eks:cluster-name` and karpenter
are supported.
@therealdwright therealdwright force-pushed the improve-cluster-name-check branch from 88cea46 to d790e72 Compare May 28, 2024 21:34
@moukoublen
Copy link
Member

LGTM

Thanks @therealdwright!

@therealdwright
Copy link
Contributor Author

Thanks @moukoublen - would you have any idea which elastic-agent release this will be in?

@moukoublen moukoublen merged commit 0fa2987 into elastic:main May 30, 2024
24 checks passed
@moukoublen
Copy link
Member

@therealdwright most probably 8.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloudbeat failed to get cluster name when using Karpenter
2 participants