Allow for 'os' to be optional in deploy actions

Document that 'os' is approximate when it is used.
Document when it is still required.
Document that other uses are deprecated.
Allow the overlay to pick up common POSIX constants.

Change-Id: I1f7c8f27615cd33a8d2b505a3f0563373f8111f4

12 files changed, 219 insertions, 20 deletions

diff --git a/doc/v2/actions-deploy.rst b/doc/v2/actions-deploy.rst
index bb485e303..6cab931b6 100644
--- a/doc/v2/actions-deploy.rst
+++ b/doc/v2/actions-deploy.rst
@@ -73,21 +73,62 @@ Parameter List
 os *
 ****
 
-The operating system of the image **must** be specified so that the LAVA
-scripts can install packages and identify other defaults in the deployment
-data.
+The operating system of the image may be specified if the LAVA scripts
+need to use the LAVA install helpers to install packages or identify
+other defaults in the deployment data. However, this support is
+**deprecated** for most use cases.
+
+If ``os`` is used, the value does not have to be exact. A similar
+operating system can be specified, based on how the test job operates.
+If the test shell definition uses the deprecated LAVA install helpers
+(by defining ``install:`` steps), then any ``os`` value which provides
+the same installation tools will work. For example, operating systems
+which are derivatives of Debian can use ``os: debian`` without needing
+explicit support for each derivative because both will use ``apt`` and
+``dpkg``.
+
+.. seealso:: :ref:`less_reliance_on_install`
+
+Test jobs which execute operating system installers **will** require
+the deployment data for that installer, so ``os`` will need to be
+specified in those test jobs. When the Lava install helpers are
+removed, the elements of deployment data which are required for
+installers will be retained.
+
+Portable test definitions do not need to specify ``os`` at all, as long
+as the test definition is not expected to run on a DUT running Android.
+
+.. important:: Please read the notes on
+   :ref:`test_definition_portability` - all test writers are strongly
+   encouraged to drop all use of the LAVA install helpers as this
+   support is **deprecated** and is expected to be removed by moving to
+   support for an updated Lava-Test Test Definition.
 
 * **Not all deployment methods support all types of operating system.**
 * **Not all devices can support all operating systems.**
 
 .. topic:: Allowed values
 
- * ``android``
- * ``ubuntu``
+ * ``android`` : If your android test job executes a Lava Test Shell
+   **on the DUT** then ``os: android`` will be needed so that the
+   Android shell is used instead of ``/bin/sh``. Many AOSP images do
+   not include ``busybox`` or other support for a shell on the DUT, so
+   test jobs using those images drive the test from the LXC by using
+   ``adb``. The deployment to the LXC does not need to specify ``os``
+   as long as the test shell is **portable**.
+
+ * ``ubuntu`` : **deprecated** - compatible with ``debian``.
+
  * ``debian``
+
  * ``lede``
+
  * ``fedora``
- * ``centos``
+
+ * ``centos`` : **deprecated** - compatible with ``fedora``.
+
  * ``debian_installer``
+
  * ``centos_installer``
+
  * ``oe``
diff --git a/lava_dispatcher/action.py b/lava_dispatcher/action.py
index 2d1849ca9..eaa3b8f26 100644
--- a/lava_dispatcher/action.py
+++ b/lava_dispatcher/action.py
@@ -36,6 +36,7 @@ from lava_common.exceptions import (
 from lava_dispatcher.log import YAMLLogger
 from lava_dispatcher.utils.lxc import is_lxc_requested
 from lava_dispatcher.utils.strings import seconds_to_str
+from lava_dispatcher.deployment_data import deployment_data_dict
 
 
 class InternalObject(object):  # pylint: disable=too-few-public-methods
@@ -405,6 +406,13 @@ class Action(object):  # pylint: disable=too-many-instance-attributes,too-many-p
         except ValueError:
             raise LAVABug("Action results need to be a dictionary")
 
+    def get_constant(self, key, prefix):
+        # whilst deployment data is still supported, check if the key exists there.
+        # once deployment_data is removed, merge with device.get_constant
+        if self.parameters.get('deployment_data'):
+            return self.parameters['deployment_data'][key]
+        return self.job.device.get_constant(key, prefix=prefix)
+
     def validate(self):
         """
         This method needs to validate the parameters to the action. For each
@@ -622,7 +630,8 @@ class Action(object):  # pylint: disable=too-many-instance-attributes,too-many-p
                 data[attr] = getattr(self, attr)
         if 'deployment_data' in self.parameters:
             data['parameters'] = dict()
-            data['parameters']['deployment_data'] = self.parameters['deployment_data'].__data__
+            if isinstance(self.parameters['deployment_data'], deployment_data_dict):
+                data['parameters']['deployment_data'] = self.parameters['deployment_data'].__data__
         return data
 
     def get_namespace_keys(self, action, parameters=None):
diff --git a/lava_dispatcher/actions/deploy/environment.py b/lava_dispatcher/actions/deploy/environment.py
index ec44d4559..4175d223d 100644
--- a/lava_dispatcher/actions/deploy/environment.py
+++ b/lava_dispatcher/actions/deploy/environment.py
@@ -41,8 +41,8 @@ class DeployDeviceEnvironment(Action):
 
     def validate(self):
         super().validate()
-        if 'lava_test_shell_file' not in \
-           self.parameters['deployment_data'].keys():
+        shell_file = self.get_constant('lava_test_shell_file', 'posix')
+        if not shell_file:
             self.errors = "Invalid deployment data - missing lava_test_shell_file"
 
         if 'env_dut' in self.job.parameters and self.job.parameters['env_dut']:
diff --git a/lava_dispatcher/actions/deploy/overlay.py b/lava_dispatcher/actions/deploy/overlay.py
index 7d0fcb32f..11d428132 100644
--- a/lava_dispatcher/actions/deploy/overlay.py
+++ b/lava_dispatcher/actions/deploy/overlay.py
@@ -103,22 +103,23 @@ class OverlayAction(DeployAction):
         # Distro-specific scripts override the generic ones
         if not self.test_needs_overlay(self.parameters):
             return
-        lava_test_results_dir = self.parameters['deployment_data']['lava_test_results_dir']
+        lava_test_results_dir = self.get_constant('lava_test_results_dir', 'posix')
         lava_test_results_dir = lava_test_results_dir % self.job.job_id
         self.set_namespace_data(action='test', label='results', key='lava_test_results_dir',
                                 value=lava_test_results_dir)
-        lava_test_sh_cmd = self.parameters['deployment_data']['lava_test_sh_cmd']
+        lava_test_sh_cmd = self.get_constant('lava_test_sh_cmd', 'posix')
         self.set_namespace_data(action='test', label='shared', key='lava_test_sh_cmd',
                                 value=lava_test_sh_cmd)
 
-        # Add distro support scripts
-        distro = self.parameters['deployment_data']['distro']
-        distro_support_dir = '%s/distro/%s' % (self.lava_test_dir, distro)
-        self.scripts_to_copy += sorted(glob.glob(os.path.join(distro_support_dir,
-                                                              'lava-*')))
+        # Add distro support scripts - only if deployment_data is set
+        distro = self.parameters['deployment_data'].get('distro')
+        if distro:
+            distro_support_dir = '%s/distro/%s' % (self.lava_test_dir, distro)
+            self.scripts_to_copy += sorted(glob.glob(os.path.join(distro_support_dir,
+                                                                  'lava-*')))
 
         if not self.scripts_to_copy:
-            self.errors = "Unable to locate lava_test_shell support scripts."
+            self.logger.debug("Skipping lava_test_shell support scripts.")
         if 'parameters' in self.job.device:
             if 'interfaces' in self.job.device['parameters']:
                 if 'target' in self.job.device['parameters']['interfaces']:
diff --git a/lava_dispatcher/actions/deploy/testdef.py b/lava_dispatcher/actions/deploy/testdef.py
index efa91d7fd..ce89d2b9f 100644
--- a/lava_dispatcher/actions/deploy/testdef.py
+++ b/lava_dispatcher/actions/deploy/testdef.py
@@ -208,9 +208,10 @@ class RepoAction(Action):
             raise LAVABug("Overlay location does not exist")
 
         # runner_path is the path to read and execute from to run the tests after boot
+        lava_test_results_dir = self.get_constant('lava_test_results_dir', 'posix')
         args = self.parameters
         runner_path = os.path.join(
-            args['deployment_data']['lava_test_results_dir'] % self.job.job_id,
+            lava_test_results_dir % self.job.job_id,
             str(self.stage),
             'tests',
             args['test_name']
@@ -981,7 +982,8 @@ class TestInstallAction(TestOverlayAction):
                 deps = testdef['install'].get('deps', [])
 
                 # distro-specific dependencies
-                deps = deps + testdef['install'].get('deps-' + self.parameters['deployment_data']['distro'], [])
+                if 'distro' in self.parameters['deployment_data']:
+                    deps = deps + testdef['install'].get('deps-' + self.parameters['deployment_data']['distro'])
 
                 if deps:
                     install_file.write('lava-install-packages ')
diff --git a/lava_dispatcher/actions/test/multinode.py b/lava_dispatcher/actions/test/multinode.py
index 2b866f5f3..05dc1d284 100644
--- a/lava_dispatcher/actions/test/multinode.py
+++ b/lava_dispatcher/actions/test/multinode.py
@@ -57,6 +57,7 @@ class MultinodeTestShell(LavaTest):
 
     @classmethod
     def needs_deployment_data(cls):
+        """ Some, not all, deployments will want deployment_data """
         return True
 
     @classmethod
diff --git a/lava_dispatcher/actions/test/shell.py b/lava_dispatcher/actions/test/shell.py
index 0a787907b..4ecc74392 100644
--- a/lava_dispatcher/actions/test/shell.py
+++ b/lava_dispatcher/actions/test/shell.py
@@ -71,6 +71,7 @@ class TestShell(LavaTest):
 
     @classmethod
     def needs_deployment_data(cls):
+        """ Some, not all, deployments will want deployment_data """
         return True
 
     @classmethod
diff --git a/lava_dispatcher/deployment_data.py b/lava_dispatcher/deployment_data.py
index a982d634e..17ba6d7b8 100644
--- a/lava_dispatcher/deployment_data.py
+++ b/lava_dispatcher/deployment_data.py
@@ -26,7 +26,7 @@ def get_deployment_data(distro):
     Returns the deployment data by name, for the cases where we actually need that.
     """
     if distro == '':
-        raise JobError("Missing 'os' value for deployment - unable to identify operating system for deployment data.")
+        return {}
     this_module = sys.modules[__name__]
     try:
         return getattr(this_module, distro)
diff --git a/lava_dispatcher/test/pipeline_refs/kvm-noos.yaml b/lava_dispatcher/test/pipeline_refs/kvm-noos.yaml
new file mode 100644
index 000000000..062cdc5ce
--- /dev/null
+++ b/lava_dispatcher/test/pipeline_refs/kvm-noos.yaml
@@ -0,0 +1,50 @@
+- class: actions.deploy.image.DeployImagesAction
+  name: deployimages
+  pipeline:
+  - class: actions.deploy.download.DownloaderAction
+    name: download-retry
+    pipeline:
+    - {class: actions.deploy.download.HttpDownloadAction, name: http-download}
+  - class: actions.deploy.download.DownloaderAction
+    name: download-retry
+    pipeline:
+    - {class: actions.deploy.download.HttpDownloadAction, name: http-download}
+  - class: actions.deploy.overlay.OverlayAction
+    name: lava-overlay
+    pipeline:
+    - {class: actions.deploy.overlay.VlandOverlayAction, name: lava-vland-overlay}
+    - {class: actions.deploy.overlay.MultinodeOverlayAction, name: lava-multinode-overlay}
+    - class: actions.deploy.testdef.TestDefinitionAction
+      name: test-definition
+      pipeline:
+      - {class: actions.deploy.testdef.GitRepoAction, name: git-repo-action}
+      - {class: actions.deploy.testdef.TestOverlayAction, name: test-overlay}
+      - {class: actions.deploy.testdef.TestInstallAction, name: test-install-overlay}
+      - {class: actions.deploy.testdef.TestRunnerAction, name: test-runscript-overlay}
+      - {class: actions.deploy.testdef.GitRepoAction, name: git-repo-action}
+      - {class: actions.deploy.testdef.TestOverlayAction, name: test-overlay}
+      - {class: actions.deploy.testdef.TestInstallAction, name: test-install-overlay}
+      - {class: actions.deploy.testdef.TestRunnerAction, name: test-runscript-overlay}
+    - {class: actions.deploy.overlay.CompressOverlay, name: compress-overlay}
+    - {class: actions.deploy.overlay.PersistentNFSOverlay, name: persistent-nfs-overlay}
+  - {class: actions.deploy.apply_overlay.ApplyOverlayGuest, name: apply-overlay-guest}
+  - {class: actions.deploy.environment.DeployDeviceEnvironment, name: deploy-device-env}
+- class: actions.boot.qemu.BootQEMUImageAction
+  name: boot-image-retry
+  pipeline:
+  - class: actions.boot.qemu.BootQemuRetry
+    name: boot-qemu-image
+    pipeline:
+    - {class: actions.boot.qemu.CallQemuAction, name: execute-qemu}
+  - {class: actions.boot.AutoLoginAction, name: auto-login-action}
+  - {class: shell.ExpectShellSession, name: expect-shell-connection}
+  - {class: actions.boot.environment.ExportDeviceEnvironment, name: export-device-env}
+- class: actions.test.shell.TestShellRetry
+  name: lava-test-retry
+  pipeline:
+  - {class: actions.test.shell.TestShellAction, name: lava-test-shell}
+- class: power.FinalizeAction
+  name: finalize
+  pipeline:
+  - {class: power.PowerOff, name: power-off}
+  - {class: power.ReadFeedback, name: read-feedback}
diff --git a/lava_dispatcher/test/sample_jobs/kvm-noos.yaml b/lava_dispatcher/test/sample_jobs/kvm-noos.yaml
new file mode 100644
index 000000000..db138c3c0
--- /dev/null
+++ b/lava_dispatcher/test/sample_jobs/kvm-noos.yaml
@@ -0,0 +1,61 @@
+# Sample JOB definition for a KVM
+
+device_type: qemu
+
+job_name: kvm-pipeline
+timeouts:
+  job:
+    minutes: 5            # timeout for the whole job (default: ??h)
+  action:
+    minutes: 1         # default timeout applied for each action; can be overriden in the action itself (default: ?h)
+
+priority: medium
+visibility: public
+
+actions:
+
+    - deploy:
+        timeout:
+          minutes: 2
+        to: tmpfs
+        images:
+          disk1:
+            url: http://images.validation.linaro.org/kvm-debian-wheezy.img.gz
+            compression: gz
+            image_arg: -drive format=raw,file={disk1}
+          disk2:
+            url: http://images.validation.linaro.org/kvm-debian-wheezy.img.gz
+            compression: gz
+            image_arg: -drive format=raw,file={disk2}
+
+    - boot:
+        method: qemu
+        media: tmpfs
+        connection: serial
+        failure_retry: 2
+        auto_login: {login_prompt: 'login:', username: root}
+        prompts:
+          - 'linaro-test'
+          - 'root@debian:~#'
+
+    - test:
+        timeout:
+           minutes: 3
+        failure_retry: 3
+        timeout:
+          minutes: 10
+        name: kvm-basic-singlenode  # is not present, use "test $N"
+        definitions:
+            - repository: git://git.linaro.org/lava-team/lava-functional-tests.git
+              from: git
+              path: lava-test-shell/smoke-tests-basic.yaml
+              name: smoke-tests
+            - repository: http://git.linaro.org/lava-team/lava-functional-tests.git
+              from: git
+              path: lava-test-shell/single-node/singlenode03.yaml
+              name: singlenode-advanced
+              branch: stylesen
+              revision: 441b61
+
+context:
+  arch: amd64
diff --git a/lava_dispatcher/test/test_kvm.py b/lava_dispatcher/test/test_kvm.py
index 76d4763f1..b4dacb235 100644
--- a/lava_dispatcher/test/test_kvm.py
+++ b/lava_dispatcher/test/test_kvm.py
@@ -225,6 +225,32 @@ class TestKVMBasicDeploy(StdoutTestCase):  # pylint: disable=too-many-public-met
                 self.assertEqual(len(action.parameters['definitions']), 2)
 
 
+class TestKVMPortable(StdoutTestCase):  # pylint: disable=too-many-public-methods
+
+    def setUp(self):
+        super().setUp()
+        factory = Factory()
+        self.job = factory.create_kvm_job('sample_jobs/kvm-noos.yaml')
+
+    def test_deploy_job(self):
+        self.assertEqual(self.job.pipeline.job, self.job)
+        for action in self.job.pipeline.actions:
+            if isinstance(action, DeployAction):
+                self.assertEqual(action.job, self.job)
+
+    def test_pipeline(self):
+        description_ref = self.pipeline_reference('kvm-noos.yaml')
+        self.assertEqual(description_ref, self.job.pipeline.describe(False))
+
+    def test_validate(self):
+        try:
+            allow_missing_path(self.job.pipeline.validate_actions, self, 'qemu-system-x86_64')
+        except JobError as exc:
+            self.fail(exc)
+        for action in self.job.pipeline.actions:
+            self.assertEqual([], action.errors)
+
+
 class TestKVMQcow2Deploy(StdoutTestCase):  # pylint: disable=too-many-public-methods
 
     def setUp(self):
diff --git a/lava_scheduler_app/tests/device-types/base.jinja2 b/lava_scheduler_app/tests/device-types/base.jinja2
index 547f2fc3a..da26cc9a2 100644
--- a/lava_scheduler_app/tests/device-types/base.jinja2
+++ b/lava_scheduler_app/tests/device-types/base.jinja2
@@ -69,6 +69,13 @@ character_delays:
 
 {% block constants %}
 constants:
+
+  # POSIX os (not AOSP)
+  posix:
+    lava_test_sh_cmd: '/bin/sh'
+    lava_test_results_dir: '/lava-%s'
+    lava_test_shell_file: '~/.bashrc'
+
   # bootloader specific
   u-boot:
     interrupt-prompt: '{{ uboot_autoboot_prompt | default("Hit any key to stop autoboot") }}'