Merge ~raharper/curtin:feature/add-grub-config-options into curtin:master

[Ryan Harper]

grub: add grub config to control os_prober,terminal settings in target

Curtin by default writes out some overrides to the default grub
behavior that is desirable for a MAAS installed machine. Curtin
now has two other use-cases where these defaults are not the
best choice. When installing a pet-system, it is far more likely
that the user has additional operating systems which they would
prefer to have grub discover and they may have different terminal
settings which make more sense than serial console.

- Add 'probe_additional_os' boolean, defaulting to False, which
  retains the default behavior of curtin for MAAS.
- Add 'terminal' key which will allows users to specify what
  terminal value for grub to use.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:9ca9725ba6abf283d144b0a01c0f8dfaca455623
https://jenkins.ubuntu.com/server/job/curtin-ci/1226/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1226
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1226
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1226
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1226

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/1226/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:09c385ba66d8ea01a20e8ceb39ef9045c15c1027
https://jenkins.ubuntu.com/server/job/curtin-ci/1230/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1230
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1230
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1230
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1230

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/1230/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

https://jenkins.ubuntu.com/server/job/curtin-vmtest-devel-debug/163/console

[Ryan Harper (raharper)]

Rebuild, had the wrong branch name.

[Chad Smith (chad.smith)]

Looks good Ryan, I had a couple of inline comments for your review

[Ryan Harper (raharper)]

Thanks, applied changes

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:ca2ee5c1a39966b27c013957a20e5c6d14ebedfd
https://jenkins.ubuntu.com/server/job/curtin-ci/1248/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1248
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1248
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1248
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1248

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/1248/rebuild

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Looks good to me. +1!

review: Approve

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:60cd290c0dc368cb7316cab20ea66a1414927885
https://jenkins.ubuntu.com/server/job/curtin-ci/1251/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1251
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1251
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1251
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1251

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/1251/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

All but three vmtests passed:

----------------------------------------------------------------------
Ran 4130 tests in 30115.216s

FAILED (SKIP=627, errors=3)
Fri, 10 May 2019 05:11:41 +0000: vmtest end [1] in 30119s

I've re-run those three which pass:

Fri, 10 May 2019 10:15:27 -0500: vmtest start: nosetests3 --process-timeout=86400 --processes=4 -vv --nologcapture tests/vmtests/test_bcache_ceph.py:DiscoTestBcacheCeph tests/vmtests/test_lvm_raid.py:DiscoTestLvmOverRaid tests/vmtests/test_mdadm_bcache.py:DiscoTestAllindata
...
----------------------------------------------------------------------
Ran 43 tests in 567.253s

OK
Fri, 10 May 2019 10:24:55 -0500: vmtest end [0] in 570s

[Server Team CI bot (server-team-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
https://jenkins.ubuntu.com/server/job/curtin-autoland-test/176/
Executed test runs:
    None: https://jenkins.ubuntu.com/server/job/admin-lp-git-autoland/431/console

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

Needed to rebase. Done

review: Approve

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:3608a15c93a94ff79ba743d8546b18063ec0d5c0
https://jenkins.ubuntu.com/server/job/curtin-ci/1258/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1258
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1258
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1258
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1258

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/1258/rebuild

review: Approve (continuous-integration)

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index b6326fb..c4eed5b 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -484,6 +484,20 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
     else:
         env['REPLACE_GRUB_LINUX_DEFAULT'] = "1"
 
+    probe_os = grubcfg.get('probe_additional_os', False)
+    if probe_os not in (False, True):
+        raise ValueError("Unexpected value %s for 'probe_additional_os'. "
+                         "Value must be boolean" % probe_os)
+    env['DISABLE_OS_PROBER'] = "0" if probe_os else "1"
+
+    # if terminal is present in config, but unset, then don't
+    grub_terminal = grubcfg.get('terminal', 'console')
+    if not isinstance(grub_terminal, str):
+        raise ValueError("Unexpected value %s for 'terminal'. "
+                         "Value must be a string" % grub_terminal)
+    if not grub_terminal.lower() == "unmodified":
+        env['GRUB_TERMINAL'] = grub_terminal
+
     if instdevs:
         instdevs = [block.get_dev_name_entry(i)[1] for i in instdevs]
     else:
diff --git a/doc/topics/config.rst b/doc/topics/config.rst
index bad8fc2..b7cca43 100644
--- a/doc/topics/config.rst
+++ b/doc/topics/config.rst
@@ -207,6 +207,27 @@ update the NVRAM settings to preserve the system configuration.
 Users may want to force NVRAM to be updated such that the next boot
 of the system will boot from the installed device.
 
+**probe_additional_os**: *<boolean: default False>*
+
+This setting controls grub's os-prober functionality and Curtin will
+disable this feature by default to prevent grub from searching for other
+operating systems and adding them to the grub menu.
+
+When False, curtin writes "GRUB_DISABLE_OS_PROBER=true" to target system in
+/etc/default/grub.d/50-curtin-settings.cfg.  If True, curtin won't modify the
+grub configuration value in the target system.
+
+**terminal**: *<['unmodified', 'console', ...]>*
+
+Configure target system grub option GRUB_TERMINAL ``terminal`` value
+which is written to /etc/default/grub.d/50-curtin-settings.cfg.  Curtin
+does not attempt to validate this string, grub2 has many values that
+it accepts and the list is platform dependent.  If ``terminal`` is
+not provided, Curtin will set the value to 'console'.  If the ``terminal``
+value is 'unmodified' then Curtin will not set any value at all and will
+use Grub defaults.
+
+
 **Example**::
 
   grub:
@@ -214,6 +235,35 @@ of the system will boot from the installed device.
        - /dev/sda1
      replace_linux_default: False
      update_nvram: True
+     terminal: serial
+
+**Default terminal value, GRUB_TERMINAL=console**::
+
+  grub:
+     install_devices:
+       - /dev/sda1
+
+**Don't set GRUB_TERMINAL in target**::
+
+  grub:
+     install_devices:
+       - /dev/sda1
+     terminal: unmodified
+
+**Allow grub to probe for additional OSes**::
+
+  grub:
+    install_devices:
+      - /dev/sda1
+     probe_additional_os: True
+
+**Avoid writting any settings to etc/default/grub.d/50-curtin-settings.cfg**::
+
+  grub:
+    install_devices:
+      - /dev/sda1
+     probe_additional_os: True
+     terminal: unmodified
 
 
 http_proxy
diff --git a/examples/tests/no-grub-file.yaml b/examples/tests/no-grub-file.yaml
 new file mode 100644
index 0000000..d5ba698
--- /dev/null
+++ b/examples/tests/no-grub-file.yaml
@@ -0,0 +1,9 @@
+# This pushes curtin through a automatic installation
+# where no storage configuration is necessary.
+placeholder_simple_install: unused
+
+# configure curtin so it does not emit a grub config file
+# in etc/default/grub/grub.d
+grub:
+   probe_additional_os: true
+   terminal: unmodified
diff --git a/helpers/common b/helpers/common
index 34f0870..5928150 100644
--- a/helpers/common
+++ b/helpers/common
@@ -779,13 +779,6 @@ install_grub() {
     esac
     debug 1 "carryover command line params '$newargs'"
 
-    case $os_family in
-       debian)
-        : > "$mp/$mygrub_cfg" ||
-            { error "Failed to write '$mygrub_cfg'"; return 1; }
-        ;;
-    esac
-
     if [ "${REPLACE_GRUB_LINUX_DEFAULT:-1}" != "0" ]; then
         apply_grub_cmdline_linux_default "$mp" "$newargs" || {
             error "Failed to apply grub cmdline."
@@ -793,11 +786,19 @@ install_grub() {
         }
     fi
 
-    {
-        echo "# Curtin disable grub os prober that might find other OS installs."
-        echo "GRUB_DISABLE_OS_PROBER=true"
-        echo "GRUB_TERMINAL=console"
-    } >> "$mp/$mygrub_cfg"
+    if [ "${DISABLE_OS_PROBER:-1}" == "1" ]; then
+        {
+            echo "# Curtin disable grub os prober that might find other OS installs."
+            echo "GRUB_DISABLE_OS_PROBER=true"
+        } >> "$mp/$mygrub_cfg"
+    fi
+
+    if [ -n "${GRUB_TERMINAL}" ]; then
+        {
+            echo "# Curtin configured GRUB_TERMINAL value"
+            echo "GRUB_TERMINAL=${GRUB_TERMINAL}"
+        } >> "$mp/$mygrub_cfg"
+    fi
 
     local short="" bd="" grubdev grubdevs_new=""
     grubdevs_new=()
diff --git a/tests/vmtests/test_simple.py b/tests/vmtests/test_simple.py
index dca75cc..bfac4e9 100644
--- a/tests/vmtests/test_simple.py
+++ b/tests/vmtests/test_simple.py
@@ -4,6 +4,7 @@ from . import VMBaseClass
 from .releases import base_vm_classes as relbase
 from .releases import centos_base_vm_classes as centos_relbase
 
+import os
 import textwrap
 
 
@@ -118,4 +119,30 @@ class EoanTestSimpleStorage(relbase.eoan, TestSimpleStorage):
     def test_output_files_exist(self):
         self.output_files_exist(["netplan.yaml"])
 
+
+class TestGrubNoDefaults(VMBaseClass):
+    """ Test that curtin does not emit any grub configuration files. """
+    conf_file = "examples/tests/no-grub-file.yaml"
+    extra_disks = []
+    extra_nics = []
+    extra_collect_scripts = [textwrap.dedent("""
+        cd OUTPUT_COLLECT_D
+        cp /etc/netplan/50-cloud-init.yaml netplan.yaml
+
+        exit 0
+        """)]
+
+    def test_no_grub_file_created(self):
+        """ Curtin did not write a grub configuration file. """
+        grub_d_path = self.collect_path('etc_default_grub_d')
+        grub_d_files = os.listdir(grub_d_path)
+        self.assertNotIn('50-curtin-settings.cfg', grub_d_files)
+
+
+class DiscoTestGrubNoDefaults(relbase.disco, TestGrubNoDefaults):
+    __test__ = True
+
+    def test_output_files_exist(self):
+        self.output_files_exist(["netplan.yaml"])
+
 # vi: ts=4 expandtab syntax=python