Skip to content

KEP 1645: relax the ServiceExportConflict requirements #5436

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
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

KEP 1645: relax the ServiceExportConflict requirements #5436

wants to merge 1 commit into from
+6 −4

Conversation

tpantelis
Copy link
Contributor

The KEP states that "a ServiceExportConflict condition will be set on all ServiceExports for the conflicted service", however this assumes an implementation has a central controller that has access to all the constituent ServiceExports or that each cluster has access to the ServiceExports on every other cluster but this may not be the case.

This PR modifies the language to recommend the condition be set on all ServiceExports but not require it.

See the motivation and further discussion here:
kubernetes-sigs/mcs-api#111 (comment)

Issue link: Multi-Cluster Services API #1645

@tpantelis
KEP 1645: relax the ServiceExportConflict requirements
834ba33 
The KEP states that "a ServiceExportConflict condition will be set on
all ServiceExports for the conflicted service", however this assumes
an implementation has a central controller that has access to all the
constituent ServiceExports or that each cluster has access to the
ServiceExports on every other cluster but this may not be the case.

This PR modifies the language to recommend the condition be set on all
ServiceExports but not require it.

See the motivation and further discussion here:
kubernetes-sigs/mcs-api#111 (comment)

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 25, 2025
@k8s-ci-robot k8s-ci-robot requested review from JeremyOT and skitt June 25, 2025 14:07
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 25, 2025
@tpantelis
Copy link
Contributor Author

/cc @MrFreezeex

@k8s-ci-robot k8s-ci-robot requested a review from MrFreezeex June 25, 2025 14:07
@tpantelis tpantelis mentioned this pull request Jun 25, 2025
Open
MrFreezeex
MrFreezeex approved these changes Jun 25, 2025
View reviewed changes
Copy link
Member

@MrFreezeex MrFreezeex left a comment

Choose a reason for hiding this comment

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

Thanks!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 25, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MrFreezeex, tpantelis
Once this PR has been reviewed and has the lgtm label, please assign jeremyot for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mikemorris
Copy link
Member

mikemorris commented Jun 25, 2025 •
edited
Loading

/lgtm

Discussed in June 25th, 2025 SIG-Multicluster meeting, see notes at https://docs.google.com/document/d/18mk62nOXE_MCSSnb4yJD_8UadtzJrYyJxFwbrgabHe8/edit?tab=t.0#heading=h.gxm49ag2wja7

Thanks for updating this - definitely makes sense that the original language would have been much easier to implement in a centralized model.

@tpantelis
Copy link
Contributor Author

/cc @lauralorenz

@k8s-ci-robot k8s-ci-robot requested a review from lauralorenz June 26, 2025 13:59
@skitt
Copy link
Member

skitt commented Jun 26, 2025

This looks good to me too, but if I approve it it will merge immediately, I would like feedback from more participants in the discussion 😉.

@tpantelis tpantelis marked this pull request as draft July 2, 2025 15:37
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrFreezeex MrFreezeex MrFreezeex approved these changes

@JeremyOT JeremyOT Awaiting requested review from JeremyOT

@skitt skitt Awaiting requested review from skitt

@lauralorenz lauralorenz Awaiting requested review from lauralorenz

Assignees

@mikemorris mikemorris

@MrFreezeex MrFreezeex

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/multicluster Categorizes an issue or PR as relevant to SIG Multicluster. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tpantelis @k8s-ci-robot @mikemorris @skitt @MrFreezeex