Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4825)

Issue 94410043: Use lxc clone by default (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by wallyworld
Modified:
5 years, 9 months ago
Reviewers:
mp+219362, menn0, fwereade
Visibility:
Public.

Description

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)

Patch Set 1 #

Total comments: 12

Patch Set 2 : Use lxc clone by default #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+286 lines, -243 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/deploy.go View 2 chunks +25 lines, -9 lines 0 comments Download
M cmd/juju/help_topics.go View 2 chunks +4 lines, -4 lines 0 comments Download
M container/lxc/export_test.go View 1 chunk +8 lines, -0 lines 0 comments Download
M container/lxc/lxc.go View 2 chunks +30 lines, -4 lines 1 comment Download
M container/lxc/lxc_test.go View 1 chunk +62 lines, -0 lines 0 comments Download
M environs/config.go View 1 1 chunk +8 lines, -0 lines 0 comments Download
M environs/config/config.go View 1 6 chunks +33 lines, -11 lines 0 comments Download
M environs/config/config_test.go View 1 6 chunks +62 lines, -8 lines 0 comments Download
M environs/config_test.go View 1 3 chunks +8 lines, -1 line 0 comments Download
M provider/local/config.go View 3 chunks +0 lines, -14 lines 0 comments Download
M provider/local/environ.go View 2 chunks +6 lines, -3 lines 0 comments Download
M provider/local/environprovider.go View 1 chunk +0 lines, -7 lines 0 comments Download
M provider/local/environprovider_test.go View 2 chunks +0 lines, -83 lines 0 comments Download
M provider/local/export_test.go View 1 chunk +0 lines, -2 lines 0 comments Download
D provider/local/lxc.go View 1 chunk +0 lines, -30 lines 0 comments Download
D provider/local/lxc_test.go View 1 chunk +0 lines, -53 lines 0 comments Download
M state/api/provisioner/provisioner_test.go View 1 chunk +31 lines, -13 lines 0 comments Download
M state/apiserver/provisioner/provisioner.go View 1 chunk +6 lines, -1 line 0 comments Download
M upgrades/deprecatedenvsettings.go View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 8
wallyworld
Please take a look.
5 years, 9 months ago (2014-05-13 13:03:50 UTC) #1
menn0
SGTM. Although I've gone through the changes in detail, you probably want a more experienced ...
5 years, 9 months ago (2014-05-15 00:41:03 UTC) #2
wallyworld
> https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newcode705 > environs/config/config.go:705: return v, ok > More of a curiousity question than anything: ...
5 years, 9 months ago (2014-05-15 01:42:23 UTC) #3
menn0
On 2014/05/15 01:42:23, wallyworld wrote: > > > https://codereview.appspot.com/94410043/diff/1/environs/config/config.go#newcode705 > > environs/config/config.go:705: return v, ok ...
5 years, 9 months ago (2014-05-15 02:22:14 UTC) #4
wallyworld
On 2014/05/15 02:22:14, menn0 wrote: > On 2014/05/15 01:42:23, wallyworld wrote: > > > > ...
5 years, 9 months ago (2014-05-15 02:30:04 UTC) #5
wallyworld
Please take a look.
5 years, 9 months ago (2014-05-15 05:56:42 UTC) #6
fwereade
LGTM, I now see my comment refers to a move rather than to new code. ...
5 years, 9 months ago (2014-05-15 09:44:17 UTC) #7
wallyworld
5 years, 9 months ago (2014-05-15 10:12:57 UTC) #8
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b