Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

Clone support #235

Closed
wants to merge 2 commits into from
Closed

Clone support #235

wants to merge 2 commits into from

Conversation

ccope
Copy link
Contributor

@ccope ccope commented Mar 5, 2014

No description provided.

@ccope
Copy link
Contributor Author

ccope commented Mar 5, 2014

Reopening #233 . Also covers #232

@fgrehm
Copy link
Owner

fgrehm commented Mar 6, 2014

@ccope thanks a lot for kicking this off! here's some initial feedback based on a 10 minute review of the changes:

  • Would we still have a need to have a base box around for "machines" that are created using lxc-clone? I'm wondering if we should provide some sort of "dummy" base box like it is done with vagrant-digitalocean and vagrant-aws
  • Is lxc-create -B supported on older versions of lxc? Yes, looks like it is present on 0.7.5
  • Does that work with lxc 1.0? I noticed that @mowings had to work around some lxc 0.9 issues with https://github.com/turbosquid/lxc-clone-vagrant

@fgrehm fgrehm added this to the Nice to have milestone Mar 6, 2014
@mowings
Copy link

mowings commented Mar 6, 2014

It should work with 1.0 -- lxc-clone was rewritten completely. Our provisioning automatically installs lxc-clone-vagrant; but I can try this out with the stock 1.0 lxc-clone and let you know.

-B still works with 0.7.5

Requiring a dummy base box seems unnecessary. For our usage in particular we only clone if a base container is specified in an environment variable; else we actually use the configured box to build the VM as normal (although using btrfs as the backing store so we can do a fast clone of the container later). So for example, in my Vagrantfile:

 inst.vm.provider :lxc do |lxc|
    if system("which btrfs && sudo btrfs filesystem df  /var/lib/lxc 2>1 > /dev/null")
      lxc.backingstore = 'btrfs'
      lxc.existing_container_name = ENV['clone_source']
    end
 end

def finalize!
@sudo_wrapper = nil if @sudo_wrapper == UNSET_VALUE
@use_machine_name = false if @use_machine_name == UNSET_VALUE
@container_name = nil if @container_name == UNSET_VALUE
@backingstore = "none" if @backingstore == UNSET_VALUE
Copy link

Choose a reason for hiding this comment

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

Despite the man entry, it looks like -B 'none' does not work with lxc-create 1.0.0.alpha1. You'll get an error as follows:

lxc-create -B 'none'  -t ubuntu -n foo
lxc-create: Failed to create backing store type none
lxc-create: Error creating backing store type none for foo
lxc-create: Error creating container foo

Probably better to use 'dir' here, or don't use the -B option at all in lxc-create if @backingstore is undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mowings Ah darn, I didn't go back and actually test "none" after upgrading to 1.0. If "dir" works for both 0.7.5 and 1.0 I'll change it to that, otherwise this might need a bit more refactoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "best" is a better default?

@ccope
Copy link
Contributor Author

ccope commented Mar 7, 2014

@fgrehm The way I'm currently using cloning, I can optionally provision from a base box or clone it from another container. A base box is definitely not needed when cloning, just the name for the original lxc container. I'm not sure what functionality adding a "dummy" box would add. If vagrant could actually keep track of which box I'm cloning though, there could be some potentially better error checking.
I'm currently using this code with lxc 1.0 and the lvm backingstore. As mowings discovered though, apparently the "none" default backingstore is no longer accepted by lxc-create.

EDIT: The boxes I am using were built back in version 0.6.x of vagrant-lxc, but I'm not sure if that matters.

@mowings
Copy link

mowings commented Mar 10, 2014

My use case is exactly the same as ccopes (although we use btrfs). Devs will generally build as usual from the base box; but our deployment, demo, stagng, qa, etc instances clone from existing images if present.

@fgrehm
Copy link
Owner

fgrehm commented Mar 10, 2014

Thanks for all the information and efforts guys, I'm really looking forward to having this in place for the first 1.0.0 beta / release candidate but I need to take care of other things (like releasing new base boxes, fixing 1.5 compatibility and private networking) before looking into this as it will require more "testing efforts" from me.

This will probably be the last feature that will come in so I'd like to apologize in advance for not giving this PR enough attention 😞

@tailhook
Copy link
Contributor

tailhook commented Apr 4, 2014

If someone (e.g. me) split out backing store configuration from this patch as a separate pull request, will it help? As backing store is more useful feature than clone support.

@fgrehm
Copy link
Owner

fgrehm commented Apr 4, 2014

Well, I have been giving this some thought lately and I think there's no need to have an specific config for backing store, maybe @mowings's idea from #232 to have a lxc.create_extra_opts = [...] or just lxc.create_opts is easier to implement and more "future proof"

What do you guys think?

To answer @tailhook question, yes, a separate PR for the extra lxc-create options is likely to get merged quicker than the clone support as it will be easier to test =]

@tailhook
Copy link
Contributor

tailhook commented Apr 4, 2014

I think that separate backing store parameter is better than create_opts, because I believe that default must be -B best. But if you think otherwise, I'm ok with that.

@ccope
Copy link
Contributor Author

ccope commented Apr 4, 2014

If vagrant-lxc is going to do any sanity checking on the backing store options or do backing store-specific logic it needs to know which particular backing store is in use. Otherwise, the create_extra_opts route would work fine.

@fgrehm
Copy link
Owner

fgrehm commented Apr 6, 2014

If vagrant-lxc is going to do any sanity checking on the backing store options or do backing store-specific logic it needs to know which particular backing store is in use

I'm not sure it is worth to do any sanity check on the backing store options but I'm wondering if we'd need to do any kind of backing store specific logic that is not handled by lxc itself.

What do you guys think?

@tailhook
Copy link
Contributor

tailhook commented Apr 6, 2014

Just noticed that separate backing store options is useful, to override backing store in ~/.vagrant.d/Vagrantfile (keeping other create options intact if any will be introduced in future)

@fgrehm
Copy link
Owner

fgrehm commented Apr 6, 2014

👍

@ccope
Copy link
Contributor Author

ccope commented Apr 7, 2014

I believe shared folders needs to behave differently when the backing store is only mounted within the container (lvm, probably btrfs and zfs).

@fgrehm
Copy link
Owner

fgrehm commented Apr 7, 2014

@ccope would you mind being a bit more specific? :-)

What exactly do we need to do differently based on backing store?

@ccope
Copy link
Contributor Author

ccope commented Apr 10, 2014

@fgrehm sorry I thought I had mentioned this somewhere else in this thread. Shared folders does some checking for whether the guest_path exists, and if not, it attempts to mkdir it. This doesn't work for containers that have mounted filesystems in their own namespace, as those mounts aren't directly visible to the host. There's a method described on this page which can access the container's filesystem through it's PID in /proc: https://www.stgraber.org/2013/12/21/lxc-1-0-advanced-container-usage/ (note: I think the lxc-info syntax varies across different versions of LXC, but the command is supposed to return just the PID of the container). I believe that will work for all containers regardless of backing store, but I haven't had time to confirm. If different shared folder methods are required, the separate backing store option would then be necessary.

@fgrehm
Copy link
Owner

fgrehm commented Apr 16, 2014

Ok then, let's make the backingstore a separate option! 👍

@fgrehm
Copy link
Owner

fgrehm commented Sep 23, 2014

Sorry this PR has been open for so long. Unfortunately this is a feature that I won't be using on a daily basis and I'm not sure I'll have enough cycles to support it. Not to say that I'm not 100% sure if this is the best / only way we can leverage lxc-clone and we should probably do some house cleaning before incorporating that. I can expand on my thoughts if any of you is interested.

I'm cutting a 1.0.0 final release today and unfortunately this is not coming in now. Some cloning functionality is likely to be in place for a future release though and I'll definitely look into this PR for ideas and inspiration, please do not delete that branch if possible :-)

@fgrehm fgrehm removed this from the v1.0.0 milestone Sep 23, 2014
@ccope ccope mentioned this pull request Apr 5, 2015
@jperelli
Copy link

jperelli commented May 4, 2015

+1 for this feature. I'm just a user. I don't know how to help. Just giving appreciation for this work.

@ccope
Copy link
Contributor Author

ccope commented May 18, 2015

I'm actually going to be working on this at my job in the next couple weeks. @fgrehm let me know if you have more ideas on how you want to see this implemented.

@fgrehm
Copy link
Owner

fgrehm commented May 18, 2015

I remember having some thoughts around making the existing_container_name option not mandatory and getting the plugin to do some magic around automatically cloning containers based on base boxes for us.

Can't remember exactly if it would be possible / doable but I thought I should share those thoughts 😄

Another thing to keep in mind is that recent Vagrant versions no longer have a requirement for base boxes to be set. The Docker provider supports that IIRC so you might want to have that around here too!

HTH!

@ccope
Copy link
Contributor Author

ccope commented May 20, 2015

Yeah, I was already thinking I should look at how vagrant works with docker/boot2docker since I'll need something for Mac users here. Calling lxc-clone without a base container to clone would be ambiguous though. If you have 3 boxes configured, what should it do? I guess you could designate one container as being the default clone source?

@fgrehm
Copy link
Owner

fgrehm commented May 27, 2015

IIRC the idea I had in mind was to get vagrant box add to add the base box AND create the base container which will be used on subsequent lxc-clones. I'm not sure would be possible to implement, might need some digging into Vagrant's sources and / or some additional functionality on Vagrant core.

If that doesn't work, we could then create that base container on the first vagrant up that makes use of the base box.

What do you think about that?

@ccope
Copy link
Contributor Author

ccope commented May 27, 2015

Hm, silently turning 'up' into clone may be surprising and confusing to users. I suppose it is logically consistent in a way... but for users using the default tarball/directory copying mode, cloning would use extra space (which could be problematic with large images and small disks).

I think I'd still like to see it configured as an option per machine, but we could make each box have a default 'base box name'. So any machine using the 'precise64-lxc' box could have the option lxc.clone = true set, and then clone 'precise64-lxc_base' if it exists already (or create+clone it if not).

Maybe if we set the default backingstore to 'dir' and had users just configure that, we could use better default behavior for vagrant up doing clones vs plain creates?

@fgrehm
Copy link
Owner

fgrehm commented May 28, 2015

Hm, silently turning 'up' into clone may be surprising and confusing to users.

Yup, I think this should be a machine / container specific setting to be set from a project specific Vagrantfile (or globally from ~/.vagrant.d/Vagrantfile)

@fgrehm
Copy link
Owner

fgrehm commented Jun 3, 2015

FYI linked clone support is on the works for the VBox provider on core and you might want to grab some ideas from there ;-)

@ccope
Copy link
Contributor Author

ccope commented Jun 4, 2015

Woot, subscribed!

@fgrehm
Copy link
Owner

fgrehm commented Oct 8, 2015

FYI - latest changes on Vagrant's core seems to be adding some abstractions that could help handling this (on master already) 🍻

@fgrehm fgrehm added the ignored label Nov 17, 2022
@fgrehm
Copy link
Owner

fgrehm commented Nov 17, 2022

Hey, sorry for the silence here but this project is looking for maintainers 😅

As per #499, I've added the ignored label and will close this issue. Thanks for the interest in the project and LMK if you want to step up and take ownership of this project on that other issue 👋

@fgrehm fgrehm closed this Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants