Skip to content

Save default namespace in the client.#3503

Merged
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:default-ns
Aug 7, 2019
Merged

Save default namespace in the client.#3503
crosbymichael merged 1 commit intocontainerd:masterfrom
Random-Liu:default-ns

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Aug 7, 2019 •
edited
Loading

This is a regression introduced by #2285.

The CRI integration test stops passing after upgrading containerd:

        Error Trace:    containerd_image_test.go:47
        Error:		Received unexpected error failed precondition
        	github.com/containerd/cri/vendor/github.com/containerd/containerd/errdefs.init.ializers
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/errdefs/errors.go:47
        	runtime.main
        		/usr/local/go/src/runtime/proc.go:188
        	runtime.goexit
        		/usr/local/go/src/runtime/asm_amd64.s:1337
        	namespace is required
        	github.com/containerd/cri/vendor/github.com/containerd/containerd/namespaces.NamespaceRequired
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/namespaces/context.go:71
        	github.com/containerd/cri/vendor/github.com/containerd/containerd.(*Client).GetLabel
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/client.go:492
        	github.com/containerd/cri/vendor/github.com/containerd/containerd.(*Client).resolveSnapshotterName
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/client.go:678
        	github.com/containerd/cri/vendor/github.com/containerd/containerd.(*image).Unpack
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/image.go:152
        	github.com/containerd/cri/vendor/github.com/containerd/containerd.(*Client).Pull
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/pull.go:72
        	github.com/containerd/cri/integration.TestContainerdImage
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/integration/containerd_image_test.go:46
        	testing.tRunner
        		/usr/local/go/src/testing/testing.go:865
        	runtime.goexit
        		/usr/local/go/src/runtime/asm_amd64.s:1337
        	failed to unpack image on snapshotter 
        	github.com/containerd/cri/vendor/github.com/containerd/containerd.(*Client).Pull
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/vendor/github.com/containerd/containerd/pull.go:73
        	github.com/containerd/cri/integration.TestContainerdImage
        		/usr/local/google/home/lantaol/workspace/src/github.com/containerd/cri/integration/containerd_image_test.go:46
        	testing.tRunner
        		/usr/local/go/src/testing/testing.go:865
        	runtime.goexit
        		/usr/local/go/src/runtime/asm_amd64.s:1337

Previously, only the server side requires namespace, so we can simply add namespace to the grpc interceptor.

However, GetLabel now requires namespace on the client side, containerd.WithDefaultNamespace doesn't work anymore.

This PR stores the default namespace in the client, so that when the namespace is required on the client side, we can default to it.

Signed-off-by: Lantao Liu [email protected]

@Random-Liu Random-Liu added this to the 1.3 milestone Aug 7, 2019
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Aug 7, 2019

Build succeeded.

mxpv
mxpv approved these changes Aug 7, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@mxpv mxpv left a comment

Choose a reason for hiding this comment

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

LGTM

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Aug 7, 2019

LGTM

@crosbymichael crosbymichael merged commit 930da7c into containerd:master Aug 7, 2019
@Random-Liu Random-Liu deleted the default-ns branch August 7, 2019 20:38
@Random-Liu Random-Liu mentioned this pull request Aug 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mxpv mxpv mxpv approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

3 participants

@Random-Liu @crosbymichael @mxpv