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.

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

📖 Add CAPI Provider Operator CAEP #3833

Merged

Conversation

wfernandes
Copy link
Contributor

What this PR does / why we need it:
This PR adds the management cluster operator CAEP proposal to Cluster API for review.

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 #3427

/kind proposal

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/proposal Issues or PRs related to proposals. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 20, 2020
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 20, 2020
@wfernandes wfernandes force-pushed the mgmt-cluster-operator-caep branch 2 times, most recently from 44d3306 to 469e342 Compare October 23, 2020 20:58
@wfernandes
Copy link
Contributor Author

@fabriziopandini Feel free to take another look and make changes or add comments where ever you see fit.
Let me know if you'd to update the sequence diagrams to state diagrams if that visualization makes more sense.

Since I'm out next week, if you think we are in a good place to get feedback, feel free to remove the WIP label. Thanks.

docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 3, 2020
@wfernandes wfernandes changed the title WIP: 📖 Add Management Cluster Operator CAEP 📖 Add Management Cluster Operator CAEP Nov 6, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2020
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
@wfernandes wfernandes force-pushed the mgmt-cluster-operator-caep branch 2 times, most recently from c2a1261 to 0d09e7c Compare December 8, 2020 18:27
Copy link
Contributor Author

@wfernandes wfernandes left a comment

Choose a reason for hiding this comment

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

As per the feedback from the review, I've updated the CAEP with the suggestions.
I'm still thinking about the need or options instead of using SecretName for the variables.

docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
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.

Apologies if I duplicated comments from the google doc, I lost access and wasn't able to review it again

docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
docs/proposals/20201020-management-cluster-operator.md Outdated Show resolved Hide resolved
@wfernandes wfernandes force-pushed the mgmt-cluster-operator-caep branch 3 times, most recently from 54e2a3a to a4d573a Compare January 8, 2021 23:13
@vincepri
Copy link
Member

@wfernandes When you're ready for final review, and approvals, please close out all the open discussions and squash commits

@CecileRobertMichon
Copy link
Contributor

lgtm overall

@wfernandes
Copy link
Contributor Author

uh...fatal error: concurrent map read and map write in TestPatchTargets.
/retest

@fabriziopandini
Copy link
Member

fabriziopandini commented Jan 15, 2021

/lgtm
Thanks for the amazing work. Let gets this started!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 15, 2021
@fabriziopandini
Copy link
Member

/milestone v0.4.0

@k8s-ci-robot k8s-ci-robot added this to the v0.4.0 milestone Jan 19, 2021
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 20, 2021
@wfernandes
Copy link
Contributor Author

@fabriziopandini Please lgtm it again. I had to fix a markdown linking issue for [secret-name-discussion] and I also updated the lastUpdated field for the proposal to today. (talk about a multi-year proposal! 😄 )

/cc @CecileRobertMichon @vincepri @detiber also looking for a final approval (if y'all approve of course 😄 )

@wfernandes
Copy link
Contributor Author

/test pull-cluster-api-test-main

@fabriziopandini
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 21, 2021
@vincepri
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2021
@k8s-ci-robot k8s-ci-robot merged commit d42cc80 into kubernetes-sigs:master Jan 21, 2021
@wfernandes wfernandes deleted the mgmt-cluster-operator-caep branch January 21, 2021 19:46
@wfernandes
Copy link
Contributor Author

For future reference, I've created this hackmd doc that has a some of the story break down to accomplish this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/proposal Issues or PRs related to proposals. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFE: Management cluster operator
8 participants