Admission Controller Validation

[gambol99]

Since v1.10.0 the --admission-control is being deprecated in favour or --enable-admission-plugin, we should enforce the behaviour in the validation code

I did a fix for this a moment ago, but yes, enforcement is a better option than trying to support both

[gambol99]

@justinsb ... you were right .. blocking it off completely is a better option as i just hit a PR where I had to support both :-|

[gambol99]

/assign @justinsb

[justinsb]

So I think having both AdmissionControl and EnableAdmissionPlugins should be an error.

But it makes for difficult upgrades & rollbacks if we don't have a warning phase in between. So we can't have EnableAdmissionPlugins for 1.9, but with this PR we would require it for 1.10. So to upgrade to 1.10 we would force the user to edit their config, but then also they'd have to edit it back to downgrade if something went wrong with the update.

So we could do this PR, but only once we expect users have upgraded past the version which doesn't support EnableAdmissionPlugins. (That's why we have the 1 year deprecation policies on flags)

You said you hit an issue in another PR with supporting both - what was that?

[KashifSaadat]

Looks good :) Also following off Justin's comment, probably also best to include DisableAdmissionPlugins in that check also?

[gambol99]

So I think having both AdmissionControl and EnableAdmissionPlugins should be an error.
But it makes for difficult upgrades & rollbacks if we don't have a warning phase in between. So we can't have EnableAdmissionPlugins for 1.9, but with this PR we would require it for 1.10. So to upgrade to 1.10 we would force the user to edit their config, but then also they'd have to edit it back to downgrade if something went wrong with the update.

Yep .. good points! ... So i've changed the validation so that 'in the cluster spec' if both options are set it will fall. As for other condition where by the AdmissionControl is used but the model fills in the EnableAdmissionPlugin, will stay with the #5248 fix for now ..

[gambol99]

is this cool @justinsb?

[justinsb]

/approve
/lgtm

Thanks @gambol99

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gambol99, justinsb

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 [gambol99,justinsb]

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