Don't add config.vm.base_mac to libvirt vagrant boxes

- Refactor VagrantConfigTemplate to no longer require a base_mac by default
- Move _random_mac() to Virtualbox subclass
- Include config.vm.base_mac in Virtualbox boxes via
  get_additional_vagrant_config_settings()

This fixes #1119

kiwi/storage/subformat/template/vagrant_config.py

@@ -18,7 +18,6 @@
 import os
 
 from textwrap import dedent
-from string import Template
 
 
 class VagrantConfigTemplate(object):
@@ -29,9 +28,9 @@ class VagrantConfigTemplate(object):
     inside a vagrant box.
 
     The included Vagrantfile carries additional information for vagrant: by
-    default that is just the boxes' MAC address, but depending on the provider
-    additional information need to be present. These can be passed via the
-    parameter ``custom_settings`` to the method :meth:`get_template`.
+    default that is nothing, but depending on the provider additional
+    information need to be present. These can be passed via the parameter
+    ``custom_settings`` to the method :meth:`get_template`.
 
     Example usage:
 
@@ -43,10 +42,8 @@ class VagrantConfigTemplate(object):
         >>> vagrant_config = VagrantConfigTemplate()
         >>> print(
         ...     vagrant_config.get_template()
-        ...     .substitute({'mac_address': 'deadbeef'})
         ... )
         Vagrant.configure("2") do |config|
-          config.vm.base_mac = "deadbeef"
         end
 
     If your provider/box requires additional settings, provide them as follows:
@@ -61,10 +58,8 @@ class VagrantConfigTemplate(object):
         ... ''').strip()
         >>> print(
         ...     vagrant_config.get_template(extra_settings)
-        ...     .substitute({'mac_address': 'DEADBEEF'})
         ... )
         Vagrant.configure("2") do |config|
-          config.vm.base_mac = "DEADBEEF"
           config.vm.hostname = "no-dead-beef"
           config.vm.provider :special do |special|
             special.secret_settings = "please_work"
@@ -79,10 +74,6 @@ class VagrantConfigTemplate(object):
             Vagrant.configure("2") do |config|
         ''').strip() + os.linesep
 
-        self.mac = self.indent + dedent('''
-            config.vm.base_mac = "${mac_address}"
-        ''').strip() + os.linesep
-
         self.end = dedent('''
             end
         ''').strip()
@@ -96,18 +87,17 @@ class VagrantConfigTemplate(object):
             pasted into the Vagrantfile template. The string is put at the
             correct indentation level for you, but the internal indentation has
             to be provided by the caller.
-        :return: A template with ``custom_settings`` inserted at the
+        :return: A string with ``custom_settings`` inserted at the
             appropriate position. The template has one the variable
             ``mac_address`` that must be substituted.
-        :rtype: string.Template
+        :rtype: str
         """
-        template_data = self.header
-        template_data += self.mac
+        template = self.header
         if custom_settings:
-            template_data += self.indent
-            template_data += self.indent.join(
+            template += self.indent
+            template += self.indent.join(
                 custom_settings.splitlines(True)
             )
-            template_data += os.linesep
-        template_data += self.end
-        return Template(template_data)
+            template += os.linesep
+        template += self.end
+        return template

kiwi/storage/subformat/vagrant_base.py

@@ -17,7 +17,6 @@
 #
 import json
 import os.path
-import random
 
 from tempfile import mkdtemp
 
@@ -234,18 +233,6 @@ class DiskFormatVagrantBase(DiskFormatBase):
 
     def _create_box_vagrantconfig(self):
         template = VagrantConfigTemplate()
-        vagrant_config = template.get_template(
+        return template.get_template(
             custom_settings=self.get_additional_vagrant_config_settings()
         )
-        return vagrant_config.substitute(
-            {'mac_address': self._random_mac()}
-        )
-
-    @staticmethod
-    def _random_mac():
-        return '%02x%02x%02x%02x%02x%02x'.upper() % (
-            0x00, 0x16, 0x3e,
-            random.randrange(0, 0x7e),
-            random.randrange(0, 0xff),
-            random.randrange(0, 0xff)
-        )

kiwi/storage/subformat/vagrant_virtualbox.py

@@ -17,6 +17,9 @@
 #
 
 import os
+import random
+
+from textwrap import dedent
 
 # project
 from kiwi.storage.subformat.template.virtualbox_ovf import (
@@ -41,8 +44,16 @@ class DiskFormatVagrantVirtualBox(DiskFormatVagrantBase):
         Configure the default shared folder to use rsync when guest additions
         are not present inside the box.
         """
+        extra_settings = dedent('''
+            config.vm.base_mac = "{mac_address}"
+        ''').strip().format(mac_address=self._random_mac())
+
         if not self.xml_state.get_vagrant_config_virtualbox_guest_additions():
-            return 'config.vm.synced_folder ".", "/vagrant", type: "rsync"'
+            extra_settings += os.linesep + dedent('''
+            config.vm.synced_folder ".", "/vagrant", type: "rsync"
+            ''').strip()
+
+        return extra_settings
 
     def create_box_img(self, temp_image_dir):
         """
@@ -75,3 +86,12 @@ class DiskFormatVagrantVirtualBox(DiskFormatVagrantBase):
                 })
             )
         return [box_img, box_ovf]
+
+    @staticmethod
+    def _random_mac():
+        return '%02x%02x%02x%02x%02x%02x'.upper() % (
+            0x00, 0x16, 0x3e,
+            random.randrange(0, 0x7e),
+            random.randrange(0, 0xff),
+            random.randrange(0, 0xff)
+        )

test/unit/storage_subformat_template_vagrant_config_test.py

@@ -13,17 +13,13 @@ class TestVagrantConfigTemplate(object):
     def test_default_Vagrantfile(self):
         Vagrantfile = dedent('''
         Vagrant.configure("2") do |config|
-          config.vm.base_mac = "deadbeef"
         end
         ''').strip()
-        assert self.vagrant_config.get_template()\
-                                  .substitute({'mac_address': 'deadbeef'}) \
-            == Vagrantfile
+        assert self.vagrant_config.get_template() == Vagrantfile
 
     def test_customized_Vagrantfile(self):
         Vagrantfile = dedent('''
         Vagrant.configure("2") do |config|
-          config.vm.base_mac = "DEADBEEF"
           config.vm.hostname = "no-dead-beef"
           config.vm.provider :special do |special|
             special.secret_settings = "please_work"
@@ -36,6 +32,4 @@ class TestVagrantConfigTemplate(object):
           special.secret_settings = "please_work"
         end
         ''').strip()
-        assert self.vagrant_config.get_template(extra_settings)\
-                                  .substitute({'mac_address': 'DEADBEEF'}) \
-            == Vagrantfile
+        assert self.vagrant_config.get_template(extra_settings) == Vagrantfile

test/unit/storage_subformat_vagrant_base_test.py

@@ -61,12 +61,10 @@ class TestDiskFormatVagrantBase(object):
 
     @patch('kiwi.storage.subformat.vagrant_base.Command.run')
     @patch('kiwi.storage.subformat.vagrant_base.mkdtemp')
-    @patch('kiwi.storage.subformat.vagrant_base.random.randrange')
     @patch.object(DiskFormatVagrantBase, 'create_box_img')
     @patch_open
     def test_create_image_format(
-        self, mocked_open, mock_create_box_img, mock_rand,
-        mock_mkdtemp, mock_command
+        self, mocked_open, mock_create_box_img, mock_mkdtemp, mock_command
     ):
         # select an example provider
         self.disk_format.image_format = 'vagrant.libvirt.box'
@@ -80,11 +78,9 @@ class TestDiskFormatVagrantBase(object):
 
         expected_vagrantfile = dedent('''
             Vagrant.configure("2") do |config|
-              config.vm.base_mac = "00163E0A0A0A"
             end
         ''').strip()
 
-        mock_rand.return_value = 0xa
         mock_mkdtemp.return_value = 'tmpdir'
 
         mocked_open.return_value = MagicMock(spec=io.IOBase)

test/unit/storage_subformat_vagrant_libvirt_test.py

@@ -69,12 +69,10 @@ class TestDiskFormatVagrantLibVirt(object):
 
     @patch('kiwi.storage.subformat.vagrant_base.Command.run')
     @patch('kiwi.storage.subformat.vagrant_base.mkdtemp')
-    @patch('kiwi.storage.subformat.vagrant_base.random.randrange')
     @patch.object(DiskFormatVagrantLibVirt, 'create_box_img')
     @patch_open
     def test_create_image_format(
-        self, mock_open, mock_create_box_img, mock_rand,
-        mock_mkdtemp, mock_command
+        self, mock_open, mock_create_box_img, mock_mkdtemp, mock_command
     ):
         mock_mkdtemp.return_value = 'tmpdir'
         mock_create_box_img.return_value = ['arbitrary']
@@ -84,7 +82,6 @@ class TestDiskFormatVagrantLibVirt(object):
         assert file_handle.write.call_args_list[1] == call(
             dedent('''
                 Vagrant.configure("2") do |config|
-                  config.vm.base_mac = "00163E010101"
                   config.vm.synced_folder ".", "/vagrant", type: "rsync"
                   config.vm.provider :libvirt do |libvirt|
                     libvirt.driver = "kvm"

test/unit/storage_subformat_vagrant_virtualbox_test.py

@@ -78,15 +78,21 @@ class TestDiskFormatVagrantVirtualBox(object):
 
         assert file_handle.write.call_args_list[0] == call(self.Leap_15_ovf)
 
-    def test_get_additional_vagrant_config_settings(self):
+    @patch('kiwi.storage.subformat.vagrant_virtualbox.random.randrange')
+    def test_get_additional_vagrant_config_settings(self, mock_rand):
         self.xml_state.get_vagrant_config_virtualbox_guest_additions \
             .return_value = None
+
+        expected_res = dedent('''
+            config.vm.base_mac = "00163E010101"
+            config.vm.synced_folder ".", "/vagrant", type: "rsync"
+        ''').strip()
         assert self.disk_format.get_additional_vagrant_config_settings() == \
-            'config.vm.synced_folder ".", "/vagrant", type: "rsync"'
+            expected_res
 
     @patch('kiwi.storage.subformat.vagrant_base.Command.run')
     @patch('kiwi.storage.subformat.vagrant_base.mkdtemp')
-    @patch('kiwi.storage.subformat.vagrant_base.random.randrange')
+    @patch('kiwi.storage.subformat.vagrant_virtualbox.random.randrange')
     @patch.object(DiskFormatVagrantVirtualBox, 'create_box_img')
     @patch_open
     def test_create_image_format_with_and_without_guest_additions(