Enable PodDistruptionBudget

[soltysh]

Fixes #10938.

@smarterclayton is there something else needed you were thinking of?

@deads2k I have a question regarding ServiceAccounts and SharedIndexInformer. If a controller (here DisruptionController does not have explicit access rights to Pods, but can access them through that SharedIndexInformer is ok? If the same controller would like to access pods directly, it would need such an access right, why then using this cache, it's not required anymore?

[soltysh]

[test]

[deads2k]

If the same controller would like to access pods directly, it would need such an access right, why then using this cache, it's not required anymore?

We never built one that checks permissions on an individual client being submitted. Informers are non-mutating and at least one client in the process had rights, so we let it through.

[soltysh]

@deads2k can I be the picky one complaining that this is not cool about it? Is there a plan (sometime in the future) to fix it, or we're cool with the current state of world?

[deads2k]

@deads2k can I be the picky one complaining that this is not cool about it? Is there a plan (sometime in the future) to fix it, or we're cool with the current state of world?

I think we're ok with the state of the world. For now.

[soltysh]

@smarterclayton any objections for merge?

[smarterclayton]

Lgtm

[soltysh]

Flake #11016

[soltysh]

It looks like I'm constantly hitting #11016, I need to investigate this more before actually merging.

[soltysh]

@mfojtik @Kargakis so I am able to reproduce consistently the timeouts, with this PR. I'll be digging deeper what might be causing that.

[soltysh]

@smarterclayton this required 2 additional changes upstream and an extension to policy, so that regular users should be able to use it. Mind taking a look once again?

[smarterclayton]

@soltysh what's the security impact of enabling PDB? Can you bring down / DoS cluster? @deads2k

[soltysh]

Updated missing generated completions.

[soltysh]

After discussion with @smarterclayton about security implications PDB will be enabled only for cluster admins. I've dropped the commit updating policy. Upon green tests I'll merge this in.

[0xmichalis]

Have you also enabled access to the eviction subresource?

[soltysh]

Rebased and waiting for green tests...

[openshift-bot]

Evaluated for origin test up to 2b5dd3d

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10529/) (Base Commit: 84cddbc)

[mfojtik]

[merge]

[openshift-bot]

Evaluated for origin merge up to 2b5dd3d

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10529/) (Image: devenv-rhel7_5235)

[smarterclayton]

Lgtm [merge]

On Oct 21, 2016, at 5:52 PM, OpenShift Bot [email protected] wrote:

continuous-integration/openshift-jenkins/test SUCCESS (
https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10440/) (Base
Commit: fa658ac
fa658ac
)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11187 (comment),
or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABG_p5Ix9R2oLXENhG6_JImpNUqOwIuTks5q2TQtgaJpZM4KMcuX
.