Use lxc clone by default

Use lxc clone by default

All providers now support configuring lxc
clone support, not just local provider. The
config attributes which previously were just
for local provider now are available on all
providers:
lxc-clone
lxc-clone-aufs

lxc-clone, if unspecified, will be set to
true if the underlying platform supports it.
The test is for Ubuntu trusty or greater. The
final check is done when the container manager
is created.

1.19.2 used a setting lxc-use-clone. This is now
removed in place of the universal lxc-clone setting.

https://code.launchpad.net/~wallyworld/juju-core/consolidate-lxc-clone-config/+merge/219362

(do not edit description out of merge proposal)

[wallyworld, 2014-05-13 13:03:50]

Please take a look.

[menn0, 2014-05-15 00:41:03]

SGTM. Although I've gone through the changes in detail, you probably want a more
experienced team member to also check.

I've added a bunch of comments but they're mainly around documentation.

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode71
cmd/juju/deploy.go:71: created with the name
Given that we're moving towards supporting non-Ubuntu workloads, is it a good
idea to be so tied to Ubuntu concepts in the help output?

What about something like:

If the destination is an LXC container the default is to use lxc-clone to create
the container where possible. For Ubuntu deployments, lxc-clone is supported for
the trusty OS series and later. A 'template' container is created with the
name...

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode75
cmd/juju/deploy.go:75: You can override the use of clone by changing the
provider configuration:
s/clone/lxc-clone/

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode80
cmd/juju/deploy.go:80: This means that the clones use up much less disk space. 
If you do not have btrfs,
s/that the clones/that clones will/

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode81
cmd/juju/deploy.go:81: lxc will attempt to use aufs (which is an overlay type
filesystem). You can
s/which is an overlay type filesystem/an overlay filesystem/

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode91
cmd/juju/deploy.go:91: juju deploy mysql -n 5 --constraints mem=8G (deploy 5
instances of mysql with at least 8 GB of RAM each)
Capitalisation is inconsistent here: Deploy vs deploy

https://codereview.appspot.com/94410043/diff/1/cmd/juju/help_topics.go
File cmd/juju/help_topics.go (left):

https://codereview.appspot.com/94410043/diff/1/cmd/juju/help_topics.go#oldcode82
cmd/juju/help_topics.go:82: As of trusty, the local provider will prefer to use
lxc-clone to create
Remember to mirror any help updates from deploy.go to here as well.

https://codereview.appspot.com/94410043/diff/1/environs/config/config.go
File environs/config/config.go (right):

https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newc...
environs/config/config.go:705: return v, ok
More of a curiousity question than anything: what's the upgrade story for people
who may have had lxc-use-clone in their config?

[wallyworld, 2014-05-15 01:42:23]

>
https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newc...
> environs/config/config.go:705: return v, ok
> More of a curiousity question than anything: what's the upgrade story for
people
> who may have had lxc-use-clone in their config?

If this were a release version, we'd handle the upgrade in code. But the config
attribute in question was only introduced in the last dev point release and
there's just the MAAS guys using it AFAIK so the release notes for 1.19.3 will
be used to communicate that the option has changed. Also, it was off by default
before; now it will be on by default so even if they don't update their config,
it will still work as the only reason they would have used the config attr would
have been to turn it on.

[menn0, 2014-05-15 02:22:14]

On 2014/05/15 01:42:23, wallyworld wrote:
> >
>
https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newc...
> > environs/config/config.go:705: return v, ok
> > More of a curiousity question than anything: what's the upgrade story for
> people
> > who may have had lxc-use-clone in their config?
> 
> If this were a release version, we'd handle the upgrade in code. But the
config
> attribute in question was only introduced in the last dev point release and
> there's just the MAAS guys using it AFAIK so the release notes for 1.19.3 will
> be used to communicate that the option has changed. Also, it was off by
default
> before; now it will be on by default so even if they don't update their
config,
> it will still work as the only reason they would have used the config attr
would
> have been to turn it on.

Cool. Thanks for clarifying.

[wallyworld, 2014-05-15 02:30:04]

On 2014/05/15 02:22:14, menn0 wrote:
> On 2014/05/15 01:42:23, wallyworld wrote:
> > >
> >
>
https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newc...
> > > environs/config/config.go:705: return v, ok
> > > More of a curiousity question than anything: what's the upgrade story for
> > people
> > > who may have had lxc-use-clone in their config?
> > 
> > If this were a release version, we'd handle the upgrade in code. But the
> config
> > attribute in question was only introduced in the last dev point release and
> > there's just the MAAS guys using it AFAIK so the release notes for 1.19.3
will
> > be used to communicate that the option has changed. Also, it was off by
> default
> > before; now it will be on by default so even if they don't update their
> config,
> > it will still work as the only reason they would have used the config attr
> would
> > have been to turn it on.
> 
> Cool. Thanks for clarifying.

Except that I forgot the setting did make it into 1.18.3 also.
So I'll need to add processing for that prior to landing.

[wallyworld, 2014-05-15 05:56:42]

Please take a look.

[fwereade, 2014-05-15 09:44:17]

LGTM, I now see my comment refers to a move rather than to new code.

https://codereview.appspot.com/94410043/diff/20001/container/lxc/lxc.go
File container/lxc/lxc.go (right):

https://codereview.appspot.com/94410043/diff/20001/container/lxc/lxc.go#newco...
container/lxc/lxc.go:138: return value >= 14.04
Yeah, I'm also a bit worried that this is ever-so ubuntu-specific.

[wallyworld, 2014-05-15 10:12:57]

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go
File cmd/juju/deploy.go (right):

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode71
cmd/juju/deploy.go:71: created with the name
On 2014/05/15 00:41:03, menn0 wrote:
> Given that we're moving towards supporting non-Ubuntu workloads, is it a good
> idea to be so tied to Ubuntu concepts in the help output?
> 
> What about something like:
> 
> If the destination is an LXC container the default is to use lxc-clone to
create
> the container where possible. For Ubuntu deployments, lxc-clone is supported
for
> the trusty OS series and later. A 'template' container is created with the
> name...

Done.

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode75
cmd/juju/deploy.go:75: You can override the use of clone by changing the
provider configuration:
On 2014/05/15 00:41:03, menn0 wrote:
> s/clone/lxc-clone/

"clone" here is correct as it is referring to the clone concept itself

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode80
cmd/juju/deploy.go:80: This means that the clones use up much less disk space. 
If you do not have btrfs,
On 2014/05/15 00:41:03, menn0 wrote:
> s/that the clones/that clones will/

Done.

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode81
cmd/juju/deploy.go:81: lxc will attempt to use aufs (which is an overlay type
filesystem). You can
On 2014/05/15 00:41:03, menn0 wrote:
> s/which is an overlay type filesystem/an overlay filesystem/

Done.

https://codereview.appspot.com/94410043/diff/1/cmd/juju/deploy.go#newcode91
cmd/juju/deploy.go:91: juju deploy mysql -n 5 --constraints mem=8G (deploy 5
instances of mysql with at least 8 GB of RAM each)
On 2014/05/15 00:41:03, menn0 wrote:
> Capitalisation is inconsistent here: Deploy vs deploy

Done.