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

📖 Topology Mutation Hook proposal #6349

Merged
merged 1 commit into from
May 22, 2022
Merged

📖 Topology Mutation Hook proposal #6349

merged 1 commit into from
May 22, 2022

Conversation

sbueringer
Copy link
Member

@sbueringer sbueringer commented Mar 30, 2022 •
edited
Loading

Signed-off-by: Stefan Büringer buringerst@vmware.com

What this PR does / why we need it:
Proposal to address #6319

Tasks:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #6319

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 30, 2022
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 30, 2022
@sbueringer sbueringer mentioned this pull request Mar 30, 2022
Closed
@sbueringer
Copy link
Member Author

sbueringer commented Mar 30, 2022 •
edited
Loading

/cc @imikushin fyi that's the proposal we mentioned yesterday

@sbueringer sbueringer force-pushed the pr-proposal-topology-mutation-hook branch 2 times, most recently from aacc8fd to 8724de1 Compare April 4, 2022 17:37
@fabriziopandini
Copy link
Member

@sbueringer In order to gather some feedback let's ping the appointed reviewers (If someone else is interested, we should make sure to add them to this list as well).

@sbueringer
Copy link
Member Author

@CecileRobertMichon @enxebre @vincepri @killianmuldoon @ykakarap
If you have some time to review this proposal (if not already done), I would appreciate it :)

ykakarap
ykakarap reviewed Apr 27, 2022
View reviewed changes
Copy link
Contributor

@ykakarap ykakarap left a comment

Choose a reason for hiding this comment

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

The proposal is LGTM pending the naming comment.

Exciting improvements to ClusterClass 🚀

docs/proposals/20220330-topology-mutation-hook.md Outdated Show resolved Hide resolved
killianmuldoon
killianmuldoon approved these changes May 5, 2022
View reviewed changes
Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

Awesome stuff!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 5, 2022
@killianmuldoon
Copy link
Contributor

/area runtime-sdk

@k8s-ci-robot k8s-ci-robot added the area/runtime-sdk Issues or PRs related to Runtime SDK label May 10, 2022
@sbueringer
Add Topology Mutation Hook proposal
1c39879 
Signed-off-by: Stefan Büringer buringerst@vmware.com
@sbueringer sbueringer force-pushed the pr-proposal-topology-mutation-hook branch from 8724de1 to 1c39879 Compare May 11, 2022 16:14
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2022
vincepri
vincepri approved these changes May 11, 2022
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 11, 2022
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 11, 2022
@elmiko
Copy link
Contributor

elmiko commented May 11, 2022

agreed at the 11 May 2022 meeting, we are starting a 1 week lazy consensus today. if there are no objections it will merge on the 18th.

sbueringer
sbueringer commented May 11, 2022
View reviewed changes
docs/proposals/20220330-topology-mutation-hook.md
patch: <JSON-patch>
```

The full OpenAPI specification (draft) of the GeneratePatches hook can be seen via the [Swagger Editor](https://editor.swagger.io/?url=https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/main/docs/proposals/images/topology-mutation-hook/runtime-sdk-openapi.yaml).
Copy link
Member Author

Choose a reason for hiding this comment

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

This link and the one below only works after merge. For review this one points to the file on the PR: https://editor.swagger.io/?url=https://raw.githubusercontent.com/kubernetes-sigs/cluster-api/1c39879fa72a7dd77b8827f6acec2e4cca8e492a/docs/proposals/images/topology-mutation-hook/runtime-sdk-openapi.yaml

CecileRobertMichon
CecileRobertMichon reviewed May 12, 2022
View reviewed changes
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@sbueringer sbueringer mentioned this pull request May 12, 2022
Closed
30 tasks
@sbueringer sbueringer mentioned this pull request May 13, 2022
Merged
@fabriziopandini
Copy link
Member

fabriziopandini commented May 22, 2022 •
edited
Loading

/hold cancel
/lgtm
/approve
Lazy consensus deadline expired, thanks everyone for helping in this effort

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 22, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, vincepri

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:
  • OWNERS [fabriziopandini,vincepri]

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

@k8s-ci-robot k8s-ci-robot merged commit e944a43 into kubernetes-sigs:main May 22, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.2 milestone May 22, 2022
@sbueringer sbueringer deleted the pr-proposal-topology-mutation-hook branch May 24, 2022 11:11
@sbueringer sbueringer mentioned this pull request May 25, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ykakarap ykakarap ykakarap left review comments

@CecileRobertMichon CecileRobertMichon CecileRobertMichon left review comments

@fabriziopandini fabriziopandini fabriziopandini left review comments

@johannesfrey johannesfrey johannesfrey left review comments

@killianmuldoon killianmuldoon killianmuldoon approved these changes

@vincepri vincepri vincepri approved these changes

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

@vincepri vincepri

@fabriziopandini fabriziopandini

@CecileRobertMichon CecileRobertMichon

@killianmuldoon killianmuldoon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/runtime-sdk Issues or PRs related to Runtime SDK cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

RFE: ClusterClass external patches
9 participants
@sbueringer @fabriziopandini @killianmuldoon @elmiko @k8s-ci-robot @vincepri @johannesfrey @CecileRobertMichon @ykakarap