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

Enable the Ceph admission controller by default #6242

Closed
travisn opened this issue Sep 11, 2020 · 8 comments
Closed

Enable the Ceph admission controller by default #6242

travisn opened this issue Sep 11, 2020 · 8 comments

Comments

@travisn
Copy link
Member

travisn commented Sep 11, 2020

Is this a bug report or feature request?

  • Feature Request

What should the feature do:
The admission controller currently requires extra steps to start, thus it is not enabled by default. We need the admission controller to start by default to get the extra level of checking in all clusters instead of just the few clusters that have enabled this feature.

See the Admission Controller topic for current requirements to start it.

What is use case behind this feature:
Improve error checking for all clusters.

@travisn travisn added this to the 1.5 milestone Sep 11, 2020
@subhamkrai subhamkrai self-assigned this Nov 12, 2020
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

@travisn travisn removed this from the 1.5 milestone Feb 10, 2021
@travisn travisn added keepalive and removed wontfix labels Feb 10, 2021
@leseb
Copy link
Member

leseb commented May 11, 2021

How far are we from doing this? @subhamkrai @travisn?

@subhamkrai
Copy link
Contributor

@leseb We are waiting to see how the OCS operator does this. Currently, if we want we can add an extra step to have cert-manager installed(I'll have to look again on this).

@travisn
Copy link
Member Author

travisn commented May 11, 2021

To enable the admission controller by default it seems we need to require that the cert-manager exists. Perhaps we could require this by default, but allow an option to disable it.

IMO the alternative of using a self-signed cert and running a script isn't an option for enabling by default.

@leseb
Copy link
Member

leseb commented May 17, 2021

To enable the admission controller by default it seems we need to require that the cert-manager exists. Perhaps we could require this by default, but allow an option to disable it.

IMO the alternative of using a self-signed cert and running a script isn't an option for enabling by default.

Why not? I remember we explored a lot of different options for this. Running a script or an operator as a dependency, isn't that the same thing? Also, cert-manager can also use self-signed certificates.

@travisn
Copy link
Member Author

travisn commented May 17, 2021

We currently have two methods for install:

  1. Helm chart + create yamls
  2. Create yamls

If we require something more than this, we need a really good reason. As much as I want to see the admission controller enabled by default, it doesn't seem like a critical requirement to justify adding a new dependency unless most clusters would already have that dependency enabled. For example, if the cert manager is already being used by majority of clusters, it seems acceptable to require it. But if not many clusters have it already, it will be a point of friction and cause fewer people to even try Rook.

@subhamkrai subhamkrai removed their assignment Jul 27, 2021
@subhamkrai
Copy link
Contributor

unassigned myself, in case someone else wants to try.

@travisn
Copy link
Member Author

travisn commented Oct 27, 2021

Closing for now, may come back to it another time...

@travisn travisn closed this as completed Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants