Add --enable-admission-plugins API server flag for k8s 1.10#5221
Add --enable-admission-plugins API server flag for k8s 1.10#5221k8s-ci-robot merged 2 commits intokubernetes:masterfrom
Conversation
|
/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
6463dd7 to
d9252a1
Compare
|
@gambol99 - Thanks for the review. I added another commit to modify |
|
/ok-to-test |
| } | ||
| } | ||
|
|
||
| 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 |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.