-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix: Add MembersReferenceResolved condition for cp groups #824
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please cover this with a test? Perhaps we can extend the test cases in https://github.com/Kong/gateway-operator/blob/5d0b4b78c138f606b1eb5116e2649f35cf780bdb/test/envtest/konnect_entities_gatewaycontrolplane_test.go
Added unit tests. |
7fdd5fd
to
b7c6345
Compare
b7c6345
to
c74eabe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doing some testing on this one and I do not see the condition set consistently. Let's put this on pause until that's resolved.
c74eabe
to
d575d40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is missing setting the condition when we encounter a conflict against the API. Specifically when we hit
id, err = getControlPlaneForUID(ctx, sdk.GetControlPlaneSDK(), ent) |
d575d40
to
ed01d88
Compare
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve conflicts that arose since last review.
Overall this looks good. Just 1 nit about adding a test case for this.
ed01d88
to
fe39896
Compare
8eac34e
to
7c524b0
Compare
What this PR does / why we need it:
Add a
MembersReferenceResolved
condition in status of control plane groups.Which issue this PR fixes
Fixes #797
Special notes for your reviewer:
PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect significant changes