[refactor] add method to check feature in manifest

The container creation code was starting to repeat the same logic for
testing that a container manifest was enabling a specific feature.

Instead we add a method to the image manifest to check for that.
Also adds tests for that feature.

nrm/aci.py

@@ -58,6 +58,7 @@ class Scheduler(SpecField):
 
     fields = {"policy": spec(unicode, True),
               "priority": spec(unicode, False),
+              "enabled": spec(unicode, False),
               }
 
     def __init__(self):
@@ -78,6 +79,10 @@ class Scheduler(SpecField):
             logger.warning("scheduler priority forced as 0 " +
                            "for non default policies")
             self.priority = "0"
+        if getattr(self, "enabled", "1") not in ["0", "False", "1", "True"]:
+            logger.error("Invalid value for scheduler enabled: %s",
+                         self.enabled)
+            return False
         return True
 
 
@@ -142,7 +147,7 @@ class PerfWrapper(SpecField):
         ret = super(PerfWrapper, self).load(data)
         if not ret:
             return ret
-        if self.enabled not in ["0", "False", "1", "True"]:
+        if getattr(self, "enabled", "1") not in ["0", "False", "1", "True"]:
             logger.error("Invalid value for perfwrapper enabled: %s",
                          self.enabled)
             return False
@@ -294,3 +299,23 @@ class ImageManifest(SpecField):
         with open(filename, 'r') as f:
             data = json.load(f)
         return super(ImageManifest, self).load(data)
+
+    def load_dict(self, data):
+        """Load a manifest in dictionary form."""
+        return super(ImageManifest, self).load(data)
+
+    def is_feature_enabled(self, feature, true_values=["1", "True"]):
+        """Check if a specific feature is enabled.
+
+        Since the enabled field itself is optional, we return true if an
+        isolator is present in a manifest or the enabled field is not true."""
+        typename = "argo/{}".format(feature)
+        assert typename in IsolatorList.types, \
+            "{} in not a valid feature".format(feature)
+        logger.debug(repr(self))
+        if hasattr(self.app.isolators, feature):
+            isolator = getattr(self.app.isolators, feature)
+            if hasattr(isolator, 'enabled'):
+                if isolator.enabled not in true_values:
+                    return False
+        return True

nrm/containers.py

@@ -62,7 +62,7 @@ class ContainerManager(object):
             logger.error("Manifest is invalid")
             return None
 
-        if hasattr(manifest.app.isolators, 'scheduler'):
+        if manifest.is_feature_enabled('scheduler'):
             sched = manifest.app.isolators.scheduler
             argv = self.chrt.getwrappedcmd(sched)
         else:
@@ -106,33 +106,25 @@ class ContainerManager(object):
             # argo-nodeos-config and not the final command -- that way it would
             # be running outside of the container.  However, because
             # argo-nodeos-config is suid root, perf can't monitor it.
-            if hasattr(manifest.app.isolators, 'perfwrapper'):
-                manifest_perfwrapper = manifest.app.isolators.perfwrapper
-                if hasattr(manifest_perfwrapper, 'enabled'):
-                    if manifest_perfwrapper.enabled in ["1", "True"]:
-                        argv.append(self.perfwrapper)
-
-            if hasattr(manifest.app.isolators, 'power'):
-                if hasattr(manifest.app.isolators.power, 'enabled'):
-                    pp = manifest.app.isolators.power
-                    if pp.enabled in ["1", "True"]:
-                        if pp.profile in ["1", "True"]:
-                            container_power['profile'] = dict()
-                            container_power['profile']['start'] = dict()
-                            container_power['profile']['end'] = dict()
-                        if pp.policy != "NONE":
-                            container_power['policy'] = pp.policy
-                            container_power['damper'] = pp.damper
-                            container_power['slowdown'] = pp.slowdown
+            if manifest.is_feature_enabled('perfwrapper'):
+                argv.append(self.perfwrapper)
+
+            if manifest.is_feature_enabled('power'):
+                pp = manifest.app.isolators.power
+                if pp.profile in ["1", "True"]:
+                    container_power['profile'] = dict()
+                    container_power['profile']['start'] = dict()
+                    container_power['profile']['end'] = dict()
+                if pp.policy != "NONE":
+                    container_power['policy'] = pp.policy
+                    container_power['damper'] = pp.damper
+                    container_power['slowdown'] = pp.slowdown
 
             # Compute hardware bindings
-            if hasattr(manifest.app.isolators, 'hwbind'):
-                manifest_hwbind = manifest.app.isolators.hwbind
-                if hasattr(manifest_hwbind, 'enabled'):
-                    if manifest_hwbind.enabled in ["1", "True"]:
-                        hwbindings['enabled'] = True
-                        hwbindings['distrib'] = sorted(self.hwloc.distrib(
-                                                ncpus, alloc), key=operator.
+            if manifest.is_feature_enabled('hwbind'):
+                hwbindings['enabled'] = True
+                hwbindings['distrib'] = sorted(self.hwloc.distrib(
+                                            ncpus, alloc), key=operator.
                                                 attrgetter('cpus'))
 
         # build context to execute

test/test_aci.py

+"""Tests for the ACI Manifest module."""
+import nrm
+import nrm.aci
+import pytest
+import json
+
+
+@pytest.fixture
+def manifest_base_data():
+    data = '''{
+    "acKind": "ImageManifest",
+    "acVersion": "0.6.0",
+    "name": "test",
+    "app":
+        {
+        "isolators": []
+        }
+}'''
+    return json.loads(data)
+
+
+def test_manifest_disabled_perfwrapper(manifest_base_data):
+    """Ensure we can check if a feature is disabled."""
+    manifest = nrm.aci.ImageManifest()
+    isolator_text = '''{
+    "name": "argo/perfwrapper",
+    "value": {
+        "enabled": "0"
+    }
+}'''
+    isolator = json.loads(isolator_text)
+    data = manifest_base_data
+    data["app"]["isolators"] = [isolator]
+    assert manifest.load_dict(data)
+    assert not manifest.is_feature_enabled("perfwrapper")
+
+
+def test_enabled_feature(manifest_base_data):
+    """Ensure we can check if a feature is enabled without enabled in it."""
+    manifest = nrm.aci.ImageManifest()
+    isolator_text = '''{
+    "name": "argo/perfwrapper",
+    "value": {}
+}'''
+    isolator = json.loads(isolator_text)
+    data = manifest_base_data
+    data["app"]["isolators"] = [isolator]
+    assert manifest.load_dict(data)
+    assert manifest.is_feature_enabled("perfwrapper")