Classic snap #6936

Merged
merged 6 commits into from Feb 9, 2017

Conversation

Projects
None yet
5 participants
Owner

nskaggs commented Feb 7, 2017

Description of change

This change enables the easy building of classic snaps for juju, including bash completion support.

QA steps

Verify the change works by creating your own local juju snap. Verify bash completion works, and juju sees your current environments, etc.

Documentation changes

The current user workflow for juju isn't changed. For developers, building a snap against there local source tree now works out of the box nicely :-)

Bug reference

Owner

nskaggs commented Feb 7, 2017

I have built beta5 and 2.0.2 using this method. They are in the store now

snap install juju --classic
gets you 2.0.2
snap install juju --classic --beta
get you 2.1-beta5

Contributor

battlemidget commented Feb 8, 2017

tested this and bash completion works along with the other functionalities of juju

I like the general changes that I see. Can you also look into the .gitignore? I put some ignores in there the last time I created a snap, but now that things are in a subdir, I'd imagine that what directories are created are different. (used to be parts/, stage/, and maybe build/ maybe prime? I don't quite rememeber)

I'd also like to understand the story around LXD and whether we can support classic lxd or not. Especially people who are running Xenial will already have an 'lxd' installed. I'm hesitant to change their working environment.

snap/wrappers/juju
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+export LXD_DIR=/var/snap/lxd/common/lxd
@jameinel

jameinel Feb 8, 2017

Owner

Does this mean we only work with snapped lxd, what is the story between debian lxd and snap lxd? Do they see the same containers?

@nskaggs

nskaggs Feb 8, 2017

Owner

Yes, as it stands this uses the LXD snap, which has a different socket. The two LXD's do NOT see the same containers.

That said, @wallyworld and I were able to produce a condition on our machines that resulted in the 2 LXD's not being happy co-installed (at least, for juju's sake, as it reported it couldn't bind). Having juju and LXD installed as both a snap and a deb works fine for @battlemidget, and mirrors conjure-up, so it's odd to see the issue.

@battlemidget

battlemidget Feb 8, 2017

Contributor

Yea and no matter what I do I can't seem to reproduce their behavior

Owner

nskaggs commented Feb 8, 2017

The .gitignore file seemed to work fine for me after building; I was pleasantly surprised. Your currently ignored files seem to get everything.

As far as supporting a snap juju with a debian LXD install, I think it's technically possible, but I'm not sure we can easily "look for either one" and use it. I don't think we can make that decision on an on-going basis, and snaps only give us the ability to tweak things at install. Tying it to the LXD snap is a better story, but I understand not everyone may want that. I think some feedback from @tych0 or @stgraber might help.

Owner

nskaggs commented Feb 8, 2017

@jameinel Is it possible for us to drop the LXD snap install altogether and have juju simply support looking for either socket (so it can support a LXD snap with the deb installed version of juju). You can prefer the deb version of course, and I think that will unblock this.

Owner

nskaggs commented Feb 9, 2017

@battlemidget could we drop installing lxd as a snap, and instead check for and LXD installation and only install it if LXD doesn't exist? Then modify our wrapper to check for the existence of either LXD socket, and prefer the debian one if found?

@nskaggs nskaggs closed this Feb 9, 2017

@nskaggs nskaggs reopened this Feb 9, 2017

snap/hooks/configure
+#!/bin/bash
+
+# Make sure we have lxd installed to use
+snap install lxd || true
@battlemidget

battlemidget Feb 9, 2017

Contributor

I actually decided to take another approach to this: https://github.com/conjure-up/conjure-up/blob/master/snap/hooks/configure#L3-L40

This allows us to utilize the deb installed LXD on Xenial servers and above and for desktops and Trusty will utilize the snap LXD. It's not ideal but at least you won't run into that issue of 2 LXD's trying to complete.

Owner

nskaggs commented Feb 9, 2017

$$merge$$

Contributor

jujubot commented Feb 9, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit f49b5be into juju:2.1 Feb 9, 2017

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