Basic packaging tests and restructure action (#1307)

Restructure action to remove custom handling around packaging of the
box and instead use more of the built-in provided actions instead.

Includes some packaging tests to cover both simple where the public
key is retained (can't modify the tinycore VM without more complex
provisioning steps), and a more complex one that utilizes a script
and supports triggering regenerating the hosts on subsequent boots.

The use of the upstream packaging helpers means that when the
default insecure ssh key has been replaced, the packaging process
will automatically include the generated key.

Fixes: #759
Fixes: #765
Fixes: #1013
Fixes: #994

lib/vagrant-libvirt/action.rb

@@ -4,8 +4,9 @@ require 'log4r'
 module VagrantPlugins
   module ProviderLibvirt
     module Action
-      # Include the built-in modules so we can use them as top-level things.
+      # Include the built-in & general modules so we can use them as top-level things.
       include Vagrant::Action::Builtin
+      include Vagrant::Action::General
       @logger = Log4r::Logger.new('vagrant_libvirt::action')
 
       # remove image from Libvirt storage pool
@@ -167,7 +168,18 @@ module VagrantPlugins
       def self.action_package
         Vagrant::Action::Builder.new.tap do |b|
           b.use ConfigValidate
-          b.use PackageDomain
+          b.use Call, IsCreated do |env, b2|
+            unless env[:result]
+              b2.use MessageNotCreated
+              next
+            end
+
+            b2.use PackageSetupFolders
+            b2.use PackageSetupFiles
+            b2.use action_halt
+            b2.use Package
+            b2.use PackageDomain
+          end
         end
       end
 
@@ -366,6 +378,9 @@ module VagrantPlugins
       autoload :WaitTillUp, action_root.join('wait_till_up')
       autoload :PrepareNFSValidIds, action_root.join('prepare_nfs_valid_ids')
 
+      autoload :Package, 'vagrant/action/general/package'
+      autoload :PackageSetupFiles, 'vagrant/action/general/package_setup_files'
+      autoload :PackageSetupFolders, 'vagrant/action/general/package_setup_folders'
       autoload :SSHRun, 'vagrant/action/builtin/ssh_run'
       autoload :HandleBox, 'vagrant/action/builtin/handle_box'
       autoload :SyncedFolders, 'vagrant/action/builtin/synced_folders'

lib/vagrant-libvirt/action/package_domain.rb

@@ -1,6 +1,12 @@
 require 'fileutils'
 require 'log4r'
 
+class String
+  def unindent
+    gsub(/^#{scan(/^\s*/).min_by{|l|l.length}}/, "")
+  end
+end
+
 module VagrantPlugins
   module ProviderLibvirt
     module Action
@@ -12,8 +18,9 @@ module VagrantPlugins
         def initialize(app, env)
           @logger = Log4r::Logger.new('vagrant_libvirt::action::package_domain')
           @app = app
-          env['package.files'] ||= {}
-          env['package.output'] ||= 'package.box'
+
+          @options = ENV.fetch('VAGRANT_LIBVIRT_VIRT_SYSPREP_OPTIONS', '')
+          @operations = ENV.fetch('VAGRANT_LIBVIRT_VIRT_SYSPREP_OPERATIONS', 'defaults,-ssh-userdir,-ssh-hostkeys,-customize')
         end
 
         def call(env)
@@ -26,13 +33,11 @@ module VagrantPlugins
             x.name == libvirt_domain.name + '.img'
           end.first
           raise Errors::NoDomainVolume if root_disk.nil?
-          boxname = env['package.output']
-          raise "#{boxname}: Already exists" if File.exist?(boxname)
-          @tmp_dir = Dir.pwd + '/_tmp_package'
-          @tmp_img = @tmp_dir + '/box.img'
-          FileUtils.mkdir_p(@tmp_dir)
-          env[:ui].info("Downloading #{root_disk.name} to #{@tmp_img}")
-          ret = download_image(@tmp_img, env[:machine].provider_config.storage_pool_name,
+
+          package_directory = env["package.directory"]
+          domain_img = package_directory + '/box.img'
+          env[:ui].info("Downloading #{root_disk.name} to #{domain_img}")
+          ret = download_image(domain_img, env[:machine].provider_config.storage_pool_name,
                                root_disk.name, env) do |progress,image_size|
             rewriting(env[:ui]) do |ui|
               ui.clear_line
@@ -42,70 +47,52 @@ module VagrantPlugins
           # Clear the line one last time since the progress meter doesn't
           # disappear immediately.
           rewriting(env[:ui]) {|ui| ui.clear_line}
-          backing = `qemu-img info "#{@tmp_img}" | grep 'backing file:' | cut -d ':' -f2`.chomp
+
+          # Prep domain disk
+          backing = `qemu-img info "#{domain_img}" | grep 'backing file:' | cut -d ':' -f2`.chomp
           if backing
             env[:ui].info('Image has backing image, copying image and rebasing ...')
-            `qemu-img rebase -p -b "" #{@tmp_img}`
+            `qemu-img rebase -p -b "" #{domain_img}`
           end
           # remove hw association with interface
           # working for centos with lvs default disks
-          options = ENV.fetch('VAGRANT_LIBVIRT_VIRT_SYSPREP_OPTIONS', '')
-          operations = ENV.fetch('VAGRANT_LIBVIRT_VIRT_SYSPREP_OPERATIONS', 'defaults,-ssh-userdir,-ssh-hostkeys,-customize')
-          `virt-sysprep --no-logfile --operations #{operations} -a #{@tmp_img} #{options}`
-          `virt-sparsify --in-place #{@tmp_img}`
-          # add any user provided file
-          extra = ''
-          @tmp_include = @tmp_dir + '/_include'
-          if env['package.include']
-            extra = './_include'
-            Dir.mkdir(@tmp_include)
-            env['package.include'].each do |f|
-              env[:ui].info("Including user file: #{f}")
-              FileUtils.cp(f, @tmp_include)
-            end
-          end
-          if env['package.vagrantfile']
-            extra = './_include'
-            Dir.mkdir(@tmp_include) unless File.directory?(@tmp_include)
-            env[:ui].info('Including user Vagrantfile')
-            FileUtils.cp(env['package.vagrantfile'], @tmp_include + '/Vagrantfile')
-          end
-          Dir.chdir(@tmp_dir)
-          info = JSON.parse(`qemu-img info --output=json #{@tmp_img}`)
+          `virt-sysprep --no-logfile --operations #{@operations} -a #{domain_img} #{@options}`
+          `virt-sparsify --in-place #{domain_img}`
+
+          # metadata / Vagrantfile
+          info = JSON.parse(`qemu-img info --output=json #{domain_img}`)
           img_size = (Float(info['virtual-size'])/(1024**3)).ceil
-          File.write(@tmp_dir + '/metadata.json', metadata_content(img_size))
-          File.write(@tmp_dir + '/Vagrantfile', vagrantfile_content)
-          assemble_box(boxname, extra)
-          FileUtils.mv(@tmp_dir + '/' + boxname, '../' + boxname)
-          FileUtils.rm_rf(@tmp_dir)
-          env[:ui].info('Box created')
-          env[:ui].info('You can now add the box:')
-          env[:ui].info("vagrant box add #{boxname} --name any_comfortable_name")
+          File.write(package_directory + '/metadata.json', metadata_content(img_size))
+          File.write(package_directory + '/Vagrantfile', vagrantfile_content(env))
+
           @app.call(env)
         end
 
-        def assemble_box(boxname, extra)
-          `tar cvzf "#{boxname}" --totals ./metadata.json ./Vagrantfile ./box.img #{extra}`
-        end
+        def vagrantfile_content(env)
+          include_vagrantfile = ""
 
-        def vagrantfile_content
-          <<-EOF
+          if env["package.vagrantfile"]
+            include_vagrantfile = <<-EOF
+
+              # Load include vagrant file if it exists after the auto-generated
+              # so it can override any of the settings
+              include_vagrantfile = File.expand_path("../include/_Vagrantfile", __FILE__)
+              load include_vagrantfile if File.exist?(include_vagrantfile)
+            EOF
+          end
+
+          <<-EOF.unindent
             Vagrant.configure("2") do |config|
               config.vm.provider :libvirt do |libvirt|
                 libvirt.driver = "kvm"
-                libvirt.host = ""
-                libvirt.connect_via_ssh = false
-                libvirt.storage_pool_name = "default"
               end
+            #{include_vagrantfile}
             end
-
-            user_vagrantfile = File.expand_path('../_include/Vagrantfile', __FILE__)
-            load user_vagrantfile if File.exists?(user_vagrantfile)
           EOF
         end
 
         def metadata_content(filesize)
-          <<-EOF
+          <<-EOF.unindent
             {
               "provider": "libvirt",
               "format": "qcow2",

spec/support/temporary_dir.rb

@@ -0,0 +1,10 @@
+shared_context 'temporary_dir' do
+  around do |example|
+    Dir.mktmpdir("rspec-") do |dir|
+      @temp_dir = dir
+      example.run
+    end
+  end
+
+  attr_reader :temp_dir
+end

spec/unit/action/package_domain_spec.rb

@@ -0,0 +1,102 @@
+require 'spec_helper'
+require 'support/sharedcontext'
+
+require 'vagrant-libvirt/action/clean_machine_folder'
+
+describe VagrantPlugins::ProviderLibvirt::Action::PackageDomain do
+  subject { described_class.new(app, env) }
+
+  include_context 'unit'
+  include_context 'libvirt'
+  include_context 'temporary_dir'
+
+  let(:libvirt_client) { double('libvirt_client') }
+  let(:libvirt_domain) { double('libvirt_domain') }
+  let(:servers) { double('servers') }
+  let(:volumes) { double('volumes') }
+
+  describe '#call' do
+    before do
+      allow_any_instance_of(VagrantPlugins::ProviderLibvirt::Driver)
+        .to receive(:connection).and_return(connection)
+      allow(connection).to receive(:client).and_return(libvirt_client)
+      allow(libvirt_client).to receive(:lookup_domain_by_uuid).and_return(libvirt_domain)
+
+      allow(connection).to receive(:servers).and_return(servers)
+      allow(servers).to receive(:get).and_return(domain)
+
+      allow(connection).to receive(:volumes).and_return(volumes)
+
+      allow(logger).to receive(:info)
+
+      env["package.directory"] = temp_dir
+    end
+
+    context 'with defaults' do
+      let(:root_disk) { double('libvirt_domain_disk') }
+      before do
+        allow(root_disk).to receive(:name).and_return('default_domain.img')
+        allow(domain).to receive(:volumes).and_return([root_disk])
+        allow(libvirt_domain).to receive(:name).and_return('default_domain')
+        allow(subject).to receive(:download_image).and_return(true)
+      end
+
+      it 'should succeed' do
+        expect(ui).to receive(:info).with('Packaging domain...')
+        expect(ui).to receive(:info).with(/Downloading default_domain.img to .*\/box.img/)
+        expect(ui).to receive(:info).with('Image has backing image, copying image and rebasing ...')
+        expect(subject).to receive(:`).with(/qemu-img info .*\/box.img | grep 'backing file:' | cut -d ':' -f2/).and_return("some image")
+        expect(subject).to receive(:`).with(/qemu-img rebase -p -b "" .*\/box.img/)
+        expect(subject).to receive(:`).with(/virt-sysprep --no-logfile --operations .* -a .*\/box.img .*/)
+        expect(subject).to receive(:`).with(/virt-sparsify --in-place .*\/box.img/)
+        expect(subject).to receive(:`).with(/qemu-img info --output=json .*\/box.img/).and_return(
+          { 'virtual-size': 5*1024*1024*1024 }.to_json
+        )
+
+        expect(subject.call(env)).to be_nil
+        expect(File.exist?(File.join(temp_dir, 'metadata.json'))).to eq(true)
+        expect(File.exist?(File.join(temp_dir, 'Vagrantfile'))).to eq(true)
+      end
+    end
+  end
+
+  describe '#vagrantfile_content' do
+    context 'with defaults' do
+      it 'should output expected content' do
+        expect(subject.vagrantfile_content(env)).to eq(
+          <<-EOF.unindent
+          Vagrant.configure("2") do |config|
+            config.vm.provider :libvirt do |libvirt|
+              libvirt.driver = "kvm"
+            end
+
+          end
+          EOF
+        )
+      end
+    end
+
+    context 'with custom user vagrantfile' do
+      before do
+        env["package.vagrantfile"] = "_Vagrantfile"
+      end
+      it 'should output Vagrantfile containing reference' do
+        expect(subject.vagrantfile_content(env)).to eq(
+          <<-EOF.unindent
+          Vagrant.configure("2") do |config|
+            config.vm.provider :libvirt do |libvirt|
+              libvirt.driver = "kvm"
+            end
+
+            # Load include vagrant file if it exists after the auto-generated
+            # so it can override any of the settings
+            include_vagrantfile = File.expand_path("../include/_Vagrantfile", __FILE__)
+            load include_vagrantfile if File.exist?(include_vagrantfile)
+
+          end
+          EOF
+        )
+      end
+    end
+  end
+end

tests/package_complex_example/Vagrantfile

@@ -0,0 +1,18 @@
+# -*- mode: ruby -*-
+# vi: set ft=ruby :
+
+Vagrant.configure("2") do |config|
+  config.vm.box = "generic/debian10"
+  config.vm.synced_folder ".", "/vagrant", disabled: true
+
+  config.vm.provider :libvirt do |libvirt|
+    libvirt.driver = "qemu"
+    libvirt.cpus = 2
+    libvirt.memory = 2048
+  end
+
+  # note by default packaging the resulting machine will bundle the generated
+  # ssh key with the resulting box, to disable this behaviour need to
+  # uncomment the following line.
+  #config.ssh.insert_key = false
+end

tests/package_complex_example/Vagrantfile.testbox

@@ -0,0 +1,13 @@
+# -*- mode: ruby -*-
+# vi: set ft=ruby :
+
+Vagrant.configure("2") do |config|
+  config.vm.box = "test-package-complex-example"
+  config.vm.synced_folder ".", "/vagrant", disabled: true
+
+  config.vm.provider :libvirt do |libvirt|
+    libvirt.driver = "qemu"
+    libvirt.cpus = 2
+    libvirt.memory = 2048
+  end
+end

tests/package_complex_example/scripts/sysprep.sh

@@ -0,0 +1,32 @@
+#!/bin/sh -eux
+
+# consider purging any packages you don't need here
+
+echo "autoremoving packages and cleaning apt data"
+apt-get -y autoremove;
+apt-get -y clean;
+
+# repeat what machine-ids does in sysprep as this script needs to run via customize
+# which has a bug resulting in the machine-ids being regenerated
+
+if [ -f /etc/machine-id ]
+then
+    truncate --size=0 /etc/machine-id
+fi
+
+if [ -f /var/lib/dbus/machine-id ]
+then
+    truncate --size=0 /run/machine-id
+fi
+
+echo "remove /var/cache"
+find /var/cache -type f -exec rm -rf {} \;
+
+echo "force a new random seed to be generated"
+rm -f /var/lib/systemd/random-seed
+
+# for debian based systems ensure host keys regenerated on boot
+if [ -e /usr/sbin/dpkg-reconfigure ]
+then
+    printf "@reboot root command bash -c 'export PATH=$PATH:/usr/sbin ; export DEBIAN_FRONTEND=noninteractive ; export DEBCONF_NONINTERACTIVE_SEEN=true ; /usr/sbin/dpkg-reconfigure openssh-server &>/dev/null ; /bin/systemctl restart ssh.service ; rm --force /etc/cron.d/keys'\n" > /etc/cron.d/keys
+fi

tests/package_simple/Vagrantfile

@@ -4,5 +4,6 @@
 Vagrant.configure("2") do |config|
   config.vm.box = "infernix/tinycore"
   config.ssh.shell = "/bin/sh"
+  config.ssh.insert_key = false
   config.vm.synced_folder ".", "/vagrant", disabled: true
 end

tests/package_simple/Vagrantfile.testbox

@@ -0,0 +1,9 @@
+# -*- mode: ruby -*-
+# vi: set ft=ruby :
+
+Vagrant.configure("2") do |config|
+  config.vm.box = "test-package-simple-domain"
+  config.ssh.shell = "/bin/sh"
+  config.ssh.insert_key = false
+  config.vm.synced_folder ".", "/vagrant", disabled: true
+end

tests/runtests.bats

@@ -142,19 +142,61 @@ cleanup() {
   echo "${output}"
   echo "status = ${status}"
   [ "$status" -eq 0 ]
-  run ${VAGRANT_CMD} halt
-  echo "${output}"
-  echo "status = ${status}"
-  [ "$status" -eq 0 ]
+  rm -f package.box
   run ${VAGRANT_CMD} package
   echo "${output}"
   echo "status = ${status}"
   [ "$status" -eq 0 ]
-  run ${VAGRANT_CMD} box add package.box --name test-package-simple-domain
+  run ${VAGRANT_CMD} destroy -f
   echo "${output}"
   echo "status = ${status}"
   [ "$status" -eq 0 ]
-  run ${VAGRANT_CMD} box remove test-package-simple-domain
+  run ${VAGRANT_CMD} box add --force package.box --name test-package-simple-domain
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  VAGRANT_VAGRANTFILE=Vagrantfile.testbox run ${VAGRANT_CMD} up ${VAGRANT_OPT}
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  run ${VAGRANT_CMD} box remove --force test-package-simple-domain
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  rm -f package.box
+
+  cleanup
+}
+
+@test "package complex example" {
+  export VAGRANT_CWD=tests/package_complex_example
+  # this will allow the host keys to be removed, and part of the sysprep script
+  # adds a step to trigger the regeneration.
+  export VAGRANT_LIBVIRT_VIRT_SYSPREP_OPERATIONS='defaults,-ssh-userdir,customize'
+  export VAGRANT_LIBVIRT_VIRT_SYSPREP_OPTIONS="--run $(pwd)/tests/package_complex_example/scripts/sysprep.sh"
+  cleanup
+  run ${VAGRANT_CMD} up ${VAGRANT_OPT}
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  rm -f package.box
+  run ${VAGRANT_CMD} package
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  run ${VAGRANT_CMD} destroy -f
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  run ${VAGRANT_CMD} box add --force package.box --name test-package-complex-example
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  VAGRANT_VAGRANTFILE=Vagrantfile.testbox run ${VAGRANT_CMD} up ${VAGRANT_OPT}
+  echo "${output}"
+  echo "status = ${status}"
+  [ "$status" -eq 0 ]
+  run ${VAGRANT_CMD} box remove --force test-package-complex-example
   echo "${output}"
   echo "status = ${status}"
   [ "$status" -eq 0 ]