-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Translate v2 requests into v3 ClusterMemberAttrSetRequest and ClusterVersionSetRequest #17008
Conversation
Skipping CI for Draft Pull Request. |
c8b2c5b
to
6474163
Compare
/retest |
You are working on #12913 PR by PR.
|
Plan was established long time ago in the top comment of #12913 On high level the plan is to clean up apply code until we can easily switch to membership information to v3. The main way to achieve that is by no longer applying any entries older than consistent index. |
We need a detailed plan. No fancy documentation required, just provide a list of PRs you are going to do, and the purpose and simple description for each PR. The task has existed for more than 2 years, and multiple people worked on it. Please don't expect everyone to keep all the context/details in mind at all times. |
Plan:
|
Will revisit this PR after #17015 is merged, and this PR is rebased. |
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
Please rebase this PR and wait for workflow green before merging
6474163
to
53e258e
Compare
Done |
53e258e
to
efae7e7
Compare
/retest |
Will want to add some additional e2e test to avoid breaking things. |
/retest |
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
efae7e7
to
c25e849
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, fuweid, serathius The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…VersionSetRequest Signed-off-by: Marek Siarkowicz <[email protected]>
c25e849
to
b781251
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
... and 23 files with indirect coverage changes @@ Coverage Diff @@
## main #17008 +/- ##
==========================================
+ Coverage 68.69% 68.79% +0.10%
==========================================
Files 420 420
Lines 35532 35544 +12
==========================================
+ Hits 24409 24453 +44
+ Misses 9686 9661 -25
+ Partials 1437 1430 -7 Continue to review full report in Codecov by Sentry.
|
@serathius: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
/retest |
From membership.Cluster perspective they are the same request, just using different path.
I remembered that v3 code does through authorization verification, so I have double checked that
ClusterMemberAttrSet
andClusterVersionSet
request don't require admin permissions as they are internal requests (not available via API). This means that this change will work even if authorization is enabled.