Skip to content

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Nov 12, 2024

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 12, 2024
@marseel marseel force-pushed the pr/marseel/cleanup_leaked_clusters branch from d3c0d4c to a4092ea Compare November 12, 2024 11:34
Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel added the release-note/ci This PR makes changes to the CI. label Nov 12, 2024
@marseel marseel force-pushed the pr/marseel/cleanup_leaked_clusters branch from a4092ea to 07d7664 Compare November 12, 2024 11:37
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 12, 2024
@marseel
Copy link
Contributor Author

marseel commented Nov 12, 2024

/test

@marseel marseel marked this pull request as ready for review November 12, 2024 11:39
@marseel marseel requested review from a team as code owners November 12, 2024 11:39
@marseel marseel requested review from thorn3r, aanm and brlbil November 12, 2024 11:39
@marseel marseel changed the title Pr/marseel/cleanup leaked clusters Cleanup leaked GCE kops clusters Nov 12, 2024
@brlbil brlbil requested review from viktor-kurchenko and removed request for brlbil November 12, 2024 11:50
@viktor-kurchenko
Copy link
Contributor

Hey @marseel, overall the PR LGTM!
But what do you think about delegating the cleanup to already existed Cloud Custodian pipeline?

@marseel
Copy link
Contributor Author

marseel commented Nov 12, 2024

But what do you think about delegating the cleanup to already existed Cloud Custodian pipeline?

I don't mind that, but Cloud Custodian will still probably not clean up kops state?
I went with this approach as I know it will work :)

@viktor-kurchenko
Copy link
Contributor

But what do you think about delegating the cleanup to already existed Cloud Custodian pipeline?

I don't mind that, but Cloud Custodian will still probably not clean up kops state? I went with this approach as I know it will work :)

Absolutely, makes sense!
Do you want to test together if Custodian can do a cleanup?
If yes, can send me an KOPS cluster example and I'll test C7N.

Copy link
Contributor

@viktor-kurchenko viktor-kurchenko left a comment

Choose a reason for hiding this comment

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

LGTM.

@aanm
Copy link
Member

aanm commented Nov 13, 2024

/ci-gateway-api

@aanm aanm added this pull request to the merge queue Nov 13, 2024
Merged via the queue into main with commit 97bd34a Nov 13, 2024
78 checks passed
@aanm aanm deleted the pr/marseel/cleanup_leaked_clusters branch November 13, 2024 09:01
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Nov 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 13, 2024
marseel added a commit to marseel/cilium that referenced this pull request Nov 14, 2024
Select should not be performed on list but boolean value.

Fixes: cilium#35915

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request Nov 15, 2024
Select should not be performed on list but boolean value.

Fixes: #35915

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants