fix the namespaceScoped of cachers

[caesarxuchao]

Fix #19939
It's strange that so many of them are misconfigured and haven't caused any problem so far.

@janetkuo

[k8s-github-robot]

Labelling this PR as size/S

[lavalamp]

It has caused a problem-- someone else has this same exact change out for review, but I can't find it now.

In that review, I asked for it to use e.g. "pods.NamespaceScoped()" instead of "true", so that future copy-paste inheritence will be more likely to change it. But I can't find the PR now...

[k8s-github-robot]

Labelling this PR as size/M

[caesarxuchao]

@lavalamp, I've addressed your comment. PTAL. Thanks.

[k8s-bot]

GCE e2e test build/test passed for commit 587409d20bae136be00a7f093b94408ed8bfc835.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-bot]

GCE e2e test build/test passed for commit f76a7ca6e8f7e36aabd87699e2f2e80c56730edb.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[bprashanth]

Thanks for doing this, was on call so didn't get a chance to address the review comments (#19663) but you can go ahead and get yours in.

[liggitt]

any chance you could include a test that would have caught the error?

[smarterclayton]

If you can add the test I'll add LGTM

[lavalamp]

It's tricky to test this, since to be useful the test has to check resources that will be added in the future and that it therefore doesn't know about-- I suggested adding a check where REST objects are used that their storage & strategy scopes agree.

[caesarxuchao]

I suggested adding a check where REST objects are used that their storage & strategy scopes agree.

@lavalamp, by actually doing this I found it perhaps is a bad idea. I pushed a second commit for the ease of discussion. See my comments in that commit for details.

[smarterclayton]

Instead of passing in the boolean to storage decorator, why not pass in the create strategy itself? Then someone has to provide a valid strategy, and you can ensure you test that NamespaceScoped is true. That would satisfy the test aspect for me.

[smarterclayton]

Or if not the create strategy, the NamespaceScoped() bool method as a sliced interface

[lavalamp]

Or if not the create strategy, the NamespaceScoped() bool method as a sliced interface

Not sure what you mean by "sliced" but this sounds good to me.

[k8s-bot]

GCE e2e build/test failed for commit 46df6fb439843abc05516a267b2731077c1a1c20.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[smarterclayton]

Sliced = a partial interface like:

type NamespaceScopeCheck interface {
  NamespaceScoped() bool
}

that create strategy satisfies

On Fri, Jan 22, 2016 at 5:05 PM, Kubernetes Bot notifications@github.com
wrote:

GCE e2e build/test failed for commit 46df6fb
46df6fb
.

• Build Log
  https://storage.cloud.google.com/kubernetes-jenkins/pr-logs/46df6fb439843abc05516a267b2731077c1a1c20/kubernetes-pull-build-test-e2e-gce/24790/build-log.txt
• Test Artifacts
  https://console.developers.google.com/storage/browser/kubernetes-jenkins/pr-logs/46df6fb439843abc05516a267b2731077c1a1c20/kubernetes-pull-build-test-e2e-gce/24790/artifacts/
• Internal Jenkins Results
  http://goto.google.com/prkubekins/job/kubernetes-pull-build-test-e2e-gce//24790

—
Reply to this email directly or view it on GitHub
#19970 (comment)
.

[caesarxuchao]

@smarterclayton yeah, I figured that out somehow. Could you take another look? Thanks.

[smarterclayton]

I feel more comfortable with this (you would have to provide a fake method, knowing that you have the strategy handy). No further comments.

[k8s-bot]

GCE e2e test build/test passed for commit fb9ac049752479332effb49f2382382446f0b495.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

[k8s-bot]

GCE e2e test build/test passed for commit ca3434d6350c497588eda19d10cc1eb729f92bb3.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

[k8s-bot]

GCE e2e test build/test passed for commit ca3434d6350c497588eda19d10cc1eb729f92bb3.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-bot]

GCE e2e test build/test passed for commit ca3434d6350c497588eda19d10cc1eb729f92bb3.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

PR needs rebase

[bprashanth]

Apparently you need a rebase

[k8s-bot]

GCE e2e build/test failed for commit ebcff4b.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[bprashanth]

@k8s-bot test this please #20306

[k8s-bot]

GCE e2e test build/test passed for commit ebcff4b.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

[k8s-bot]

GCE e2e test build/test passed for commit ebcff4b.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[k8s-github-robot]

Automatic merge from submit-queue

[wojtek-t]

@caesarxuchao - thanks a lot for fixing it!