cmd,interfaces/mount: run snap-update-ns and snap-discard-ns from core if possible #3326

Merged
merged 8 commits into from May 16, 2017

Conversation

Projects
None yet
3 participants
Contributor

zyga commented May 16, 2017

This patch fixes "re-exec" of the two aforementioned tools by adding a new helper cmd.InternalToolPath and using it in the right spot. I will follow up with a change to use the same
helper for snap-confine

zyga added some commits May 16, 2017

cmd: re-factor the code and add InternalToolPath
This patch re-factors some of the re-exec code to allow us to compute
the path of an internal tool (like snap-confine or snap-update-ns) using
the same rules we use for re-exec.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/mount: use update-ns/discard-ns from core snap if possible
This patch leverages the InternalToolPath helper to run snap-discard-ns
and snap-update-ns from the core snap if we can.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

I'd be happier with unit tests, but OK.

zyga added some commits May 16, 2017

cmd,tests: restore logging that tests expected but tweak wording
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Contributor

zyga commented May 16, 2017

@chipaca yes, I want to fix the immediate issue but I'm already writing some tests for that (we had none before)

zyga added some commits May 16, 2017

cmd: add stub tests
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

Looks good, some suggestions inline.

+//
+// Ensure we do not use older version of snapd, look for info file and ignore
+// version of core that do not yet have it.
+func coreSupportsReExec(corePath string) bool {
@mvo5

mvo5 May 16, 2017

Collaborator

I would prefer a different name for this helper, most cores "support" re-exec, this code is about if the version of snapd in core is higher than the version currently running. Maybe newerSnapdIn("/path/to/core") ?

+ corePath := newCore
+ coreTool := filepath.Join(newCore, "/usr/lib/snapd", tool)
+ if !osutil.FileExists(coreTool) {
+ corePath = oldCore
@mvo5

mvo5 May 16, 2017

Collaborator

Not sure we still need to support oldCore at this point. This code will only be build in the core snap. We have frozen ubuntu-core at version 2.25.

@zyga

zyga May 16, 2017

Contributor

I wanted to keep it as is but I can remove it. If you agree I'll do it in a separate PR (I've started working on tests already)

@@ -38,7 +39,7 @@ func mountNsPath(snapName string) string {
func runNamespaceTool(toolName, snapName string) ([]byte, error) {
mntFile := mountNsPath(snapName)
if osutil.FileExists(mntFile) {
- toolPath := filepath.Join(dirs.DistroLibExecDir, toolName)
+ toolPath := cmd.InternalToolPath(toolName)
@mvo5

mvo5 May 16, 2017

Collaborator

Feels like a slightly odd place cmd.InternalToolPath but then it also fits - it is about commands afterall.

@zyga

zyga May 16, 2017

Contributor

Yes, I was wondering where this all ought to live. Ideas welcome, I'll happily move this elsewhere

zyga added some commits May 16, 2017

cmd: add tests for DistroSupportsReExec
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
interfaces/mount: mock release info for tests to pass
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit f0d083a into snapcore:master May 16, 2017

2 of 7 checks passed

artful-amd64 autopkgtest running
Details
xenial-amd64 autopkgtest running
Details
xenial-i386 autopkgtest running
Details
yakkety-amd64 autopkgtest running
Details
zesty-amd64 autopkgtest running
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-ppc64el autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:feature/tools-from-core branch May 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment