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

feat(ke): set ControlPlaneRefValid condition to false #1414

Merged
merged 2 commits into from
Mar 31, 2025
Merged

feat(ke): set ControlPlaneRefValid condition to false #1414

merged 2 commits into from
Mar 31, 2025
+136 −47

Conversation

mlavacca
Copy link
Member

@mlavacca mlavacca commented Mar 28, 2025 •
edited
Loading

What this PR does / why we need it:

If the KonnectExtension references the KE by NamespacedRef, but the CP does not exist, the KonnectExtension ControlPlaneRefValid condition is not properly set. This PR fixes that.

Which issue this PR fixes

Fixes #

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 force-pushed the ke-cp-ref-false-condition branch from 22daf7e to b0abd7e Compare March 28, 2025 16:04
@mlavacca mlavacca marked this pull request as ready for review March 28, 2025 16:18
@mlavacca mlavacca requested a review from a team as a code owner March 28, 2025 16:18
@mlavacca mlavacca added this to the KGO v1.5.x milestone Mar 28, 2025
pmalek
pmalek reviewed Mar 28, 2025
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.

This looks good. One thing I think is worth pointing out is that we do not have an envtest based test to check this functionality which I think we should add (similarly as it's being done in #1404 for DPs). I think it could be done in a separate PR.

What I think should be done in this PR is to add code for the TestKonnectExtension integration test to test the changes here. WDYT?

@mlavacca mlavacca force-pushed the ke-cp-ref-false-condition branch 3 times, most recently from 171a4ca to 7fcd363 Compare March 30, 2025 20:39
@mlavacca mlavacca force-pushed the ke-cp-ref-false-condition branch from 7fcd363 to a30b8ef Compare March 30, 2025 20:57
@mlavacca
Copy link
Member Author

This looks good. One thing I think is worth pointing out is that we do not have an envtest based test to check this functionality which I think we should add (similarly as it's being done in #1404 for DPs). I think it could be done in a separate PR.

What I think should be done in this PR is to add code for the TestKonnectExtension integration test to test the changes here. WDYT?

I've added an integration test. Let me know if that works for you

@mlavacca mlavacca requested a review from pmalek March 31, 2025 07:55
pmalek
pmalek approved these changes Mar 31, 2025
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.

👍

@pmalek pmalek modified the milestones: KGO v1.5.x, KGO v1.6.x Mar 31, 2025
@pmalek
Copy link
Member

pmalek commented Mar 31, 2025

ℹ️ I've changed the milestone for this to 1.6 as this is where this will land when merged. We'll want to backport it to 1.5 to get it there.

@czeslavo czeslavo mentioned this pull request Mar 31, 2025
Open
21 tasks
@mlavacca mlavacca added this pull request to the merge queue Mar 31, 2025
Merged via the queue into main with commit 5aca26c Mar 31, 2025
24 checks passed
@mlavacca mlavacca deleted the ke-cp-ref-false-condition branch March 31, 2025 09:41
czeslavo pushed a commit that referenced this pull request Mar 31, 2025
@mlavacca @czeslavo
feat(ke): set ControlPlaneRefValid condition to false (#1414)
5d720f5 
* feat(ke): set ControlPlaneRefValid cond to false

Signed-off-by: Mattia Lavacca <[email protected]>

* test(integration): KonnectExtension CP not found

Signed-off-by: Mattia Lavacca <[email protected]>

---------

Signed-off-by: Mattia Lavacca <[email protected]>
czeslavo pushed a commit that referenced this pull request Mar 31, 2025
@mlavacca @czeslavo
feat(ke): set ControlPlaneRefValid condition to false (#1414)
ad77fbe 
* feat(ke): set ControlPlaneRefValid cond to false

Signed-off-by: Mattia Lavacca <[email protected]>

* test(integration): KonnectExtension CP not found

Signed-off-by: Mattia Lavacca <[email protected]>

---------

Signed-off-by: Mattia Lavacca <[email protected]>
(cherry picked from commit 5aca26c)
czeslavo pushed a commit that referenced this pull request Mar 31, 2025
@mlavacca @czeslavo
feat(ke): set ControlPlaneRefValid condition to false (#1414)
e691378 
* feat(ke): set ControlPlaneRefValid cond to false

Signed-off-by: Mattia Lavacca <[email protected]>

* test(integration): KonnectExtension CP not found

Signed-off-by: Mattia Lavacca <[email protected]>

---------

Signed-off-by: Mattia Lavacca <[email protected]>
(cherry picked from commit 5aca26c)
czeslavo pushed a commit that referenced this pull request Mar 31, 2025
@mlavacca @czeslavo
feat(ke): set ControlPlaneRefValid condition to false (#1414)
ba4fe67 
* feat(ke): set ControlPlaneRefValid cond to false

Signed-off-by: Mattia Lavacca <[email protected]>

* test(integration): KonnectExtension CP not found

Signed-off-by: Mattia Lavacca <[email protected]>

---------

Signed-off-by: Mattia Lavacca <[email protected]>
(cherry picked from commit 5aca26c)
czeslavo added a commit that referenced this pull request Mar 31, 2025
@czeslavo @mlavacca
feat(ke): set ControlPlaneRefValid condition to false (#1414) (#1421)
dd9449d 
* feat(ke): set ControlPlaneRefValid cond to false

Signed-off-by: Mattia Lavacca <[email protected]>

* test(integration): KonnectExtension CP not found

Signed-off-by: Mattia Lavacca <[email protected]>

---------

Signed-off-by: Mattia Lavacca <[email protected]>
(cherry picked from commit 5aca26c)

Co-authored-by: Mattia Lavacca <[email protected]>
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

Assignees

@mlavacca mlavacca

Labels
None yet
Projects
None yet
Milestone
KGO v1.6.x
Development

Successfully merging this pull request may close these issues.
2 participants
@mlavacca @pmalek