-
Notifications
You must be signed in to change notification settings - Fork 49
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
Introduced member leases to work-around limitations in Server-Side-Apply #207
Introduced member leases to work-around limitations in Server-Side-Apply #207
Conversation
0065a16
to
5912322
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.
Looks good, thanks for writing the latest decisions down. I only have a few questions/suggestions.
5912322
to
442a557
Compare
@timuthy Thanks for the review! I have addressed your comments. Can you please check if this fine or if more changes are required? |
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.
/lgtm
Thanks for addressing the feedback
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.
@amshuman-kr Thanks for updating the proposal with the changes from our discussion. I just had a doubt regarding the utility of id
field over role
field. Please address this. Thanks.
442a557
to
cfe150f
Compare
@shreyas-s-rao Thanks for the review. I have changed the proposal as discussed. Now both |
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.
@amshuman-kr Thanks for addressing my concern. LGTM
How to categorize this PR?
/area control-plane
/kind enhancement
What this PR does / why we need it:
Introduced member leases to work-around limitations in Server-Side-Apply.
Which issue(s) this PR fixes:
Alternative design to #206.
Special notes for your reviewer:
Limitations in Server-Side Apply are documented in #179. I have updated the proposal with alternative design that was discussed out-of-band of using
leases
per member instead of introducing a newEtcdMember
resource (as described in #206).cc @shreyas-s-rao @timuthy @abdasgupta
Release note: