UPSTREAM: 46236: Support sandbox images from private registries #15880
Conversation
openshift-merge-robot
added
the
size/L
|
/unassign @bparees @stevekuznetsov |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, sjenning The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
/retest |
|
extended_conformance_gce failed to provision cluster |
|
I'm not seeing how these errors could be caused by my change. Yet I don't see them happening in other PRs. One more time for good measure. |
|
I'm left to conclude this change is breaking some e2e tests. Need to look into it. |
|
Nope, other 3.6.x PRs are failing the same tests. Flake #15941 |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
@sjenning: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Automatic merge from submit-queue |
| @@ -340,15 +343,44 @@ func getSecurityOptSeparator(v *semver.Version) rune { | |||
|
|
|||
| // ensureSandboxImageExists pulls the sandbox image when it's not present. | |||
| func ensureSandboxImageExists(client dockertools.DockerInterface, image string) error { | |||
| dockerCfgSearchPath := []string{"/.docker", filepath.Join(os.Getenv("HOME"), ".docker")} | |||
There was a problem hiding this comment.
Should this be "./.docker" ?
Or is looking in the top of the file system in /.docker intentional here?
There was a problem hiding this comment.
I didn't really notice that before. Might ask on the upstream PR why this is.
What trouble are you having with the containerized node? I figured it would Just Work (tm) since /var/lib/origin is bind mounted from the host into the containerized node.
There was a problem hiding this comment.
kubernetes/kubernetes#46236
xref https://bugzilla.redhat.com/show_bug.cgi?id=1481251
This is a regression vs origin 1.5. When using the dockershim+CRI, the new default in kube 1.6, the client no longer uses
.docker/config.jsonfor pull credentials for infra pods.This upstream commit went into kube 1.7 but was not cherry-picked to kube 1.6.
This PR is the pick for origin 1.6 and should try to go into 1.6.1
Reviewer note:
Upstream refactored
dockertoolstolibdockerso the patch doesn't match exactly.@sdodson @michaelgugino @derekwaynecarr @eparis