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

Introduce rosaControlPlane for managed kubernetes #4344

Closed
wants to merge 1 commit into from

Conversation

enxebre
Copy link
Member

@enxebre enxebre commented Jun 16, 2023

What type of PR is this?

What this PR does / why we need it:

This introduces support ROSA analogous to the EKS implementation.

This branch introduces a new ROSAControlPlane CRD and controller which enables functional ROSA Cluster creation with the proper yaml definition. We plan to iterate and follow up to flesh out design and implementation for the API, cluster deletion, day 2 changes, machine management via rosaMachinePools, etc.
This first iteration is meant to be little more than a starting common ground for collaboration.

Let's discuss on next community meeting next steps.

cc @vincepri @richardcase

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 #

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Release note:


@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 16, 2023
@k8s-ci-robot k8s-ci-robot added needs-priority size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 16, 2023
@enxebre
Copy link
Member Author

enxebre commented Jun 28, 2023

Planning to bring this up in next community call Mon 3 Jul

@enxebre
Copy link
Member Author

enxebre commented Jul 3, 2023

Planning to bring this up in next community call Mon 3 Jul

There's no meeting today, so 10Jul.

@vincepri
Copy link
Member

vincepri commented Jul 5, 2023

@enxebre Can you rebase?

@richardcase
Copy link
Member

/milestone v2.3.0

@k8s-ci-robot k8s-ci-robot added this to the v2.3.0 milestone Jul 10, 2023
Copy link
Member

@Ankitasw Ankitasw left a comment

Choose a reason for hiding this comment

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

Shall this be an experimental feature first and then be graduated?
If yes, then it would require a bit of refactoring, as we follow for exp APIs and controllers.

@richardcase
Copy link
Member

Shall this be an experimental feature first and then be graduated? If yes, then it would require a bit of refactoring, as we follow for exp APIs and controllers.

Good point @Ankitasw . Yes i think we do need to sit this behind a feature flag initially like we did when we added other significant new features.

@enxebre
Copy link
Member Author

enxebre commented Jul 13, 2023

Makes sense, I'll account for it while rebasing

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from enxebre. For more information see the Kubernetes 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

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2023
@enxebre enxebre force-pushed the rosa-poc branch 2 times, most recently from a05ded4 to aecf29b Compare July 31, 2023 08:11
@enxebre
Copy link
Member Author

enxebre commented Jul 31, 2023

Rebased, put behind feature gate and started to gather follow ups here #4431

@muraee
Copy link
Contributor

muraee commented Aug 2, 2023

it would be good if we can add an OWNER file or something to allow us to approve PRs related to the ROSA provider without needing to bother the maintainers every time.
cc @richardcase @Ankitasw thoughts?

Comment on lines +52 to +53
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ROSAclusters,verbs=get;list;watch;update;patch;delete
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=ROSAclusters/status,verbs=get;update;patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be these resources in lowercase?

@vincepri
Copy link
Member

vincepri commented Aug 7, 2023

it would be good if we can add an OWNER file or something to allow us to approve PRs related to the ROSA provider without needing to bother the maintainers every time. cc @richardcase @Ankitasw thoughts?

Adding a sub-owner file sounds good to me, although I'd generally prefer a steady stream of contribution/reviews before we add approver rights.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 11, 2023
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

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/test-infra repository.

@Ankitasw
Copy link
Member

In favor of #4453

@Ankitasw Ankitasw closed this Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-priority needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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.

7 participants