Skip to content

Rebase onto Kubernetes 6241a21#1136

Merged
openshift-bot merged 14 commits intoopenshift:masterfrom
smarterclayton:rebase_kube
Mar 4, 2015
Merged

Rebase onto Kubernetes 6241a21#1136
openshift-bot merged 14 commits intoopenshift:masterfrom
smarterclayton:rebase_kube

Conversation

@smarterclayton
Copy link
Contributor

Includes remote command execution and Docker caching

@smarterclayton
Copy link
Contributor Author

[test]

@smarterclayton
Copy link
Contributor Author

@pweil- @deads2k review please.

@smarterclayton
Copy link
Contributor Author

Needs #1135 fixed upstream (although patched here).

@smarterclayton
Copy link
Contributor Author

Might fix #1115

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1219/)

@smarterclayton
Copy link
Contributor Author

I cherry-picked Vish's change in as well.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct to be using the services strategy when validating pod name generation? In controllers/types.go they're technically the same strategy, just struck me as strange when reading it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard, the next file removes the Pods strategy....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno, ask upstream.

On Feb 25, 2015, at 6:16 AM, Paul notifications@github.com wrote:

In Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/create_test.go:

@@ -25,17 +25,17 @@ import (

func TestCheckGeneratedNameError(t *testing.T) {
expect := errors.NewNotFound("foo", "bar")

  • if err := CheckGeneratedNameError(Pods, expect, &api.Pod{}); err != expect {
  • if err := CheckGeneratedNameError(Services, expect, &api.Pod{}); err != expect {
    Is it correct to be using the services strategy when validating pod name generation? In controllers/types.go they're technically the same strategy, just struck me as strange when reading it.


Reply to this email directly or view it on GitHub.

@pweil-
Copy link

pweil- commented Feb 25, 2015

My eyes...they bleed! LGTM Some things that jumped out at me that may affect us:

  1. Changes to how ListWatchers are created
  2. Storage config in master has moved from fields to a map
  3. Change to endpoints from a string to a defined type

I can no longer read any secrets code without picturing @pmorie standing behind me whispering "shhh!"

@smarterclayton
Copy link
Contributor Author

The router is fixed in my last commit. Also, you don't have to review the merge commits :). Just my fixes.

On Feb 25, 2015, at 7:27 AM, Paul notifications@github.com wrote:

My eyes...they bleed! LGTM Some things that jumped out at me that may affect us:

Changes to how ListWatchers are created
Storage config in master has moved from fields to a map
Change to endpoints from a string to a defined type
I can no longer read any secrets code without picturing @pmorie standing behind me whispering "shhh!"


Reply to this email directly or view it on GitHub.

@smarterclayton smarterclayton force-pushed the rebase_kube branch 2 times, most recently from 4f08824 to 6e59b78 Compare February 27, 2015 03:07
@smarterclayton
Copy link
Contributor Author

[test]

@deads2k
Copy link
Contributor

deads2k commented Feb 27, 2015

I'll look at adding a proper error for #1135 today or Monday. I previously misunderstood and thought that self-linking was already tolerant upstream.

The number of required changes is much smaller than I anticipated. Is there going to be a card for someone to take advantage of the generic registry stuff?

@smarterclayton
Copy link
Contributor Author

@deads2k calling ResourceAccessReviews("") is disallowed now upstream (you can't invoke create on a namespace scoped resource with ""), which causes:

--- FAIL: TestResourceAccessReview (3.91s)
    authorization_test.go:188: expected forbidden error, but didn't get one
    authorization_test.go:194: unexpected error: request construction error: 'an empty namespace may not be set during creation'
    authorization_test.go:199: expected [system:cluster-admins], got []
    authorization_test.go:202: expected [system:kube-client system:openshift-client system:openshift-deployer], got []

My preference is to define GlobalResourceAccessReview() method that takes no namespace and does not set Namespace() on the Create method. Do you have an alternative suggestion?

Empty namespace is now disallowed as an action (you must not invoke
request.Namespace() if you don't want a namespace set).
@smarterclayton smarterclayton added this to the 0.5.0 milestone Mar 2, 2015
Fix router integration test
@deads2k
Copy link
Contributor

deads2k commented Mar 3, 2015

My preference is to define GlobalResourceAccessReview() method that takes no namespace and does not set Namespace() on the Create method. Do you have an alternative suggestion

I'd have both. A GlobalResourceAccessReview for cluster-admins who have rights to access resources across all namespaces and a ResourceAccessReview for project admins who only have rights to a single namespace. I had heard rumors that global objects were going to be eliminated...

@derekwaynecarr Will we still have globally objects in a month?

@liggitt
Copy link
Contributor

liggitt commented Mar 3, 2015

@smarterclayton fyi, I need the secrets stuff in this for #1208

@smarterclayton
Copy link
Contributor Author

Brenton is going to cut beta2 before this lands. Talk to him and make sure the other stuff for beta2 is ready to land.

@smarterclayton
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1102/) (Image: devenv-fedora_966)

@openshift-bot
Copy link
Contributor

Evaluated for origin up to 1c06be8

openshift-bot pushed a commit that referenced this pull request Mar 4, 2015
@openshift-bot openshift-bot merged commit c657948 into openshift:master Mar 4, 2015
sjenning pushed a commit to sjenning/origin that referenced this pull request Jul 30, 2018
…-3-8

UPSTREAM: 58433:  lstat with abs path of parent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants