Add --enable-admission-plugins API server flag for k8s 1.10 #5221
Conversation
k8s-ci-robot
added
needs-ok-to-test
|
/assign @geojaz I'm not sure the proper way to deprecate these flags are, so comments are appreciated. |
|
/assign @gambol99 |
…on-control in v1.10
…ion controller is enabled or not
ripta
force-pushed
the
enable-admission-plugins
branch
from
6463dd7 to
d9252a1
Compare
k8s-ci-robot
added
size/L
|
@gambol99 - Thanks for the review. I added another commit to modify |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
| @@ -461,5 +465,16 @@ func (c *KubeAPIServerConfig) HasAdmissionController(name string) bool { | |||
| } | |||
| } | |||
|
|
|||
| for _, x := range c.DisableAdmissionPlugins { | |||
There was a problem hiding this comment.
perhaps ...
for _, x := range x.DisableAdmissionPlugins {
if x == name {
return false
}
}
for _, x := range append(c.AdmissionControl, c.EnableAdmissionPlugins...)
if x == name {
return true // and save a few lines
}
}but up to you .. it's just code preference, so no biggie :-)
There was a problem hiding this comment.
Unless you and another reviewer feel strongly about combining them, I'd rather keep them separate. Subjectively, range append(c.AdmissionControl, c.EnableAdmissionPlugins...) is harder to read, only to save a couple of lines. Presumably we'll be cleaning up c.AdmissionControl in the future anyway, when it has been removed from k/k and kops stops supporting older versions.
|
Looks good to me ... cc'ing @justinsb for a second opinion :-) |
|
/approve /lgtm Thanks @ripta |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, ripta 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 |
k8s-ci-robot
added
the
approved
A previous PR kubernetes#5221 introduced the --enable-admission-plugins for >= 1.10.0 as recommended, it does however cause an issue if you already have AdmissionControl is specified in the Spec as both flags get rendered
A previous PR kubernetes#5221 introduced the --enable-admission-plugins for >= 1.10.0 as recommended, it does however cause an issue if you already have AdmissionControl is specified in the Spec as both flags get rendered
A previous PR kubernetes#5221 introduced the --enable-admission-plugins for >= 1.10.0 as recommended, it does however cause an issue if you already have AdmissionControl is specified in the Spec as both flags get rendered
A previous PR kubernetes#5221 introduced the --enable-admission-plugins for >= 1.10.0 as recommended, it does however cause an issue if you already have AdmissionControl is specified in the Spec as both flags get rendered
A previous PR kubernetes#5221 introduced the --enable-admission-plugins for >= 1.10.0 as recommended, it does however cause an issue if you already have AdmissionControl is specified in the Spec as both flags get rendered
According to the section on recommended admission controllers:
--admission-controlhas been deprecated in 1.10, replaced with a new flag.AllOrderedPlugins.As such, this PR:
--enable-admission-pluginsand also the additional flag,--disable-admission-plugins.--admission-controldeprecated.