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.

Sign up for GitHub

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

Jump to bottom

fix: cluster-wide resources do not use ownerRefs #259

Merged
merged 4 commits into from
Jun 13, 2024
Merged

fix: cluster-wide resources do not use ownerRefs #259

merged 4 commits into from
Jun 13, 2024

Conversation

mlavacca
Copy link
Member

@mlavacca mlavacca commented May 14, 2024 •
edited

What this PR does / why we need it:

Which issue this PR fixes

Fixes #72

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

@mlavacca mlavacca changed the title fix: cluster-wide resources do not use ]ownerRefs fix: cluster-wide resources do not use ownerRefs May 14, 2024
@mlavacca mlavacca marked this pull request as ready for review May 15, 2024 12:19
@mlavacca mlavacca requested a review from a team as a code owner May 15, 2024 12:19
@mlavacca mlavacca force-pushed the mlavacca/issue-72 branch 7 times, most recently from a223253 to fa3a93d Compare May 31, 2024 10:00
@mlavacca mlavacca requested review from pmalek and czeslavo May 31, 2024 10:04
pmalek
pmalek reviewed Jun 11, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

Couple of possible places where we can make things simpler + a proposal to add some tests.

👍

pkg/utils/kubernetes/ownerrefs.go Outdated Show resolved Hide resolved
pkg/utils/kubernetes/ownerrefs.go Outdated Show resolved Hide resolved
pkg/utils/kubernetes/lists.go Outdated Show resolved Hide resolved
pkg/utils/kubernetes/ownerrefs.go Outdated Show resolved Hide resolved
@mlavacca mlavacca force-pushed the mlavacca/issue-72 branch 3 times, most recently from 86487c9 to 6401d2d Compare June 13, 2024 12:11
@mlavacca mlavacca requested a review from pmalek June 13, 2024 12:12
pmalek
pmalek reviewed Jun 13, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

I'd add a Changelog entry describing this change. WDYT?

test/e2e/test_helm_install_upgrade.go Outdated Show resolved Hide resolved
test/e2e/test_helm_install_upgrade.go Show resolved Hide resolved
pkg/utils/kubernetes/owners.go Show resolved Hide resolved
pkg/utils/kubernetes/owners.go Outdated Show resolved Hide resolved
@mlavacca
Copy link
Member Author

I'd add a Changelog entry describing this change. WDYT?

I don't see any new commit 🤔 I've created a CHANGELOG entry though

@mlavacca
fix: cluster-wide res do not use ownerRefs
c663f24 
Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
@mlavacca
address review comments
56d4a77 
Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
@mlavacca
address comments
3ad3ea3 
Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
@mlavacca
reviews
16fcfec 
Signed-off-by: Mattia Lavacca <lavacca.mattia@gmail.com>
@mlavacca mlavacca enabled auto-merge (squash) June 13, 2024 13:00
@mlavacca mlavacca requested a review from pmalek June 13, 2024 13:00
@pmalek pmalek added this to the KGO v1.3.x milestone Jun 13, 2024
pmalek
pmalek approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@pmalek pmalek left a comment

Choose a reason for hiding this comment

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

I believe this is good to go.

Thanks for working on this @mlavacca 🙇

@mlavacca mlavacca merged commit 043b192 into main Jun 13, 2024
16 checks passed
@mlavacca mlavacca deleted the mlavacca/issue-72 branch June 13, 2024 13:33
@pmalek pmalek mentioned this pull request Jun 24, 2024
Closed
2 tasks
This was referenced Jul 5, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmalek pmalek pmalek approved these changes

@czeslavo czeslavo Awaiting requested review from czeslavo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
KGO v1.3.x
3 participants
@mlavacca @pmalek @czeslavo