DOCD-1685 Set resource requests/limits from fiaas.yml on bootstrapping pod

.gitignore

@@ -106,3 +106,5 @@ ENV/
 ### JetBrains template
 .idea
 
+# Emacs directory specific variables
+.dir-locals.el

fiaas_skipper/deploy/bootstrap.py

@@ -6,7 +6,8 @@
 
 from k8s.client import NotFound
 from k8s.models.common import ObjectMeta
-from k8s.models.pod import Container, PodSpec, Pod
+from k8s.models.pod import Container, PodSpec, Pod, ResourceRequirements
+from k8s.models.resourcequota import ResourceQuota, NotBestEffort
 from prometheus_client import Counter
 
 BOOTSTRAP_POD_NAME = "fiaas-deploy-daemon-bootstrap"
@@ -17,38 +18,40 @@
 
 
 class BarePodBootstrapper(object):
-    def __init__(self, cmd_args=()):
-        self._cmd_args = cmd_args
+    def __init__(self, cmd_args=None):
+        self._cmd_args = [] if cmd_args is None else cmd_args
 
     def __call__(self, deployment_config, channel, spec_config=None):
+        namespace = deployment_config.namespace
         bootstrap_counter.inc()
-        LOG.info("Bootstrapping %s in %s", deployment_config.name, deployment_config.namespace)
+        LOG.info("Bootstrapping %s in %s", deployment_config.name, namespace)
         try:
-            Pod.delete(name=BOOTSTRAP_POD_NAME, namespace=deployment_config.namespace)
+            Pod.delete(name=BOOTSTRAP_POD_NAME, namespace=namespace)
         except NotFound:
             pass
-        pod_spec = _create_pod_spec(self._cmd_args, channel)
-        pod_metadata = _create_pod_metadata(deployment_config, spec_config)
+        pod_spec = _create_pod_spec(self._cmd_args, channel, namespace, spec_config)
+        pod_metadata = _create_pod_metadata(namespace, spec_config)
         pod = Pod(metadata=pod_metadata, spec=pod_spec)
         pod.save()
 
 
-def _create_pod_spec(args, channel):
+def _create_pod_spec(args, channel, namespace, spec_config):
     container = Container(
         name="fiaas-deploy-daemon-bootstrap",
         image=channel.metadata['image'],
-        command=["fiaas-deploy-daemon-bootstrap"] + args
+        command=["fiaas-deploy-daemon-bootstrap"] + args,
+        resources=_create_resource_requirements(namespace, spec_config)
     )
     pod_spec = PodSpec(containers=[container], serviceAccountName="default", restartPolicy="Never")
     return pod_spec
 
 
-def _create_pod_metadata(deployment_config, spec_config):
+def _create_pod_metadata(namespace, spec_config):
     pod_annotations = _get_pod_annotations(spec_config)
     pod_metadata = ObjectMeta(name=BOOTSTRAP_POD_NAME,
                               annotations=pod_annotations,
                               labels={"app": BOOTSTRAP_POD_NAME},
-                              namespace=deployment_config.namespace)
+                              namespace=namespace)
     return pod_metadata
 
 
@@ -59,3 +62,16 @@ def _get_pod_annotations(spec_config):
         except KeyError:
             pass
     return {}
+
+
+def _create_resource_requirements(namespace, spec_config):
+    if not spec_config or _only_besteffort_qos_is_allowed(namespace):
+        return ResourceRequirements()
+    else:
+        return ResourceRequirements(limits=spec_config.get('resources', {}).get('limits', None),
+                                    requests=spec_config.get('resources', {}).get('requests', None))
+
+
+def _only_besteffort_qos_is_allowed(namespace):
+    resourcequotas = ResourceQuota.list(namespace=namespace)
+    return any(rq.spec.hard.get("pods") == "0" and NotBestEffort in rq.spec.scopes for rq in resourcequotas)

tests/fiaas_skipper/conftest.py

@@ -1,11 +1,98 @@
 #!/usr/bin/env python
 # -*- coding: utf-8
 
+import itertools
 import mock
+
 import pytest
 
 
 @pytest.fixture(autouse=True)
 def get():
     with mock.patch('k8s.client.Client.get') as m:
         yield m
+
+
+@pytest.fixture()
+def post():
+    with mock.patch('k8s.client.Client.post') as mockk:
+        yield mockk
+
+
+@pytest.fixture()
+def delete():
+    with mock.patch('k8s.client.Client.delete') as mockk:
+        yield mockk
+
+
+@pytest.helpers.register
+def assert_any_call(mockk, first, *args):
+    __tracebackhide__ = True
+
+    def _assertion():
+        mockk.assert_any_call(first, *args)
+
+    _add_useful_error_message(_assertion, mockk, first, args)
+
+
+@pytest.helpers.register
+def assert_no_calls(mockk, uri=None):
+    __tracebackhide__ = True
+
+    def _assertion():
+        calls = [call[0] for call in mockk.call_args_list if (uri is None or call[0][0] == uri)]
+        assert len(calls) == 0
+
+    _add_useful_error_message(_assertion, mockk, None, None)
+
+
+def _add_useful_error_message(assertion, mockk, first, args):
+    """
+    If an AssertionError is raised in the assert, find any other calls on mock where the first parameter is uri and
+    append those calls to the AssertionErrors message to more easily find the cause of the test failure.
+    """
+    __tracebackhide__ = True
+    try:
+        assertion()
+    except AssertionError as ae:
+        other_calls = [call[0] for call in mockk.call_args_list if (first is None or call[0][0] == first)]
+        if other_calls:
+            extra_info = '\n\nURI {} got the following other calls:\n{}\n'.format(first, '\n'.join(
+                _format_call(call) for call in other_calls))
+            if len(other_calls) == 1 and len(other_calls[0]) == 2 and args is not None:
+                extra_info += _add_argument_diff(other_calls[0][1], args[0])
+            raise AssertionError(extra_info) from ae
+        else:
+            raise
+
+
+def _add_argument_diff(actual, expected, indent=0, acc=None):
+    first = False
+    if not acc:
+        acc = ["Actual vs Expected"]
+        first = True
+    if type(actual) != type(expected):
+        acc.append("{}{!r} {} {!r}".format(" " * indent * 2, actual, "==" if actual == expected else "!=", expected))
+    elif isinstance(actual, dict):
+        for k in set(actual.keys()) | set(expected.keys()):
+            acc.append("{}{}:".format(" " * indent * 2, k))
+            a = actual.get(k)
+            e = expected.get(k)
+            if a != e:
+                _add_argument_diff(a, e, indent + 1, acc)
+    elif isinstance(actual, list):
+        for a, e in itertools.zip_longest(actual, expected):
+            acc.append("{}-".format(" " * indent * 2))
+            if a != e:
+                _add_argument_diff(a, e, indent + 1, acc)
+    else:
+        acc.append("{}{!r} {} {!r}".format(" " * indent * 2, actual, "==" if actual == expected else "!=", expected))
+    if first:
+        return "\n".join(acc)
+
+
+def _format_call(call):
+    if len(call) > 1:
+        return 'call({}, {})'.format(call[0], call[1])
+    else:
+        return 'call({})'.format(call[0])

tests/fiaas_skipper/deploy/test_bootstrap.py

@@ -0,0 +1,117 @@
+from __future__ import absolute_import
+
+import copy
+import mock
+
+from k8s.models.resourcequota import ResourceQuota, ResourceQuotaSpec,  NotBestEffort, BestEffort
+from k8s.models.common import ObjectMeta
+import pytest
+
+from fiaas_skipper.deploy.deploy import default_spec_config
+from fiaas_skipper.deploy.cluster import DeploymentConfig
+from fiaas_skipper.deploy.channel import ReleaseChannel
+from fiaas_skipper.deploy.bootstrap import BarePodBootstrapper
+
+
+ONLY_BEST_EFFORT_ALLOWED = {
+    "hard": {
+        "pods": "0",
+    },
+    "scopes": [NotBestEffort],
+}
+
+BEST_EFFORT_NOT_ALLOWED = {
+    "hard": {
+        "pods": "0",
+    },
+    "scopes": [BestEffort],
+}
+
+OVERRIDE_ALL_RESOURCES = {
+    'requests': {
+        'memory': '128Mi',
+        'cpu': '500m',
+    },
+    'limits': {
+        'memory': '512Mi',
+        'cpu': '1',
+    },
+}
+
+
+def spec_config(resources=None):
+    config = copy.deepcopy(default_spec_config)
+    if resources:
+        config['resources'] = resources
+    return config
+
+
+class TestBarePodBootstrapper():
+
+    def pod_uri(self, namespace="default", name=""):
+        return f'/api/v1/namespaces/{namespace}/pods/{name}'
+
+    @pytest.fixture
+    def resourcequota_list(self):
+        with mock.patch("k8s.models.resourcequota.ResourceQuota.list") as mockk:
+            yield mockk
+
+    def create_resourcequota(self, namespace, resourcequota_spec):
+        metadata = ObjectMeta(name="foo", namespace=namespace)
+        spec = ResourceQuotaSpec.from_dict(resourcequota_spec)
+        return ResourceQuota(metadata=metadata, spec=spec)
+
+    @pytest.mark.parametrize("namespace,resourcequota_spec,resources,spec_config", [
+        ("default", None, spec_config()['resources'], spec_config()),
+        ("other-namespace", None, spec_config()['resources'], spec_config()),
+        ("default", None, OVERRIDE_ALL_RESOURCES, spec_config(resources=OVERRIDE_ALL_RESOURCES)),
+        ("other-namespace", None, OVERRIDE_ALL_RESOURCES, spec_config(resources=OVERRIDE_ALL_RESOURCES)),
+        ("other-namespace", None, spec_config()['resources'], spec_config()),
+        ("default", ONLY_BEST_EFFORT_ALLOWED, None, spec_config()),
+        ("other-namespace", ONLY_BEST_EFFORT_ALLOWED, None, spec_config()),
+        ("default", BEST_EFFORT_NOT_ALLOWED, spec_config()['resources'], spec_config()),
+        ("other-namespace", BEST_EFFORT_NOT_ALLOWED, spec_config()['resources'], spec_config()),
+        ("default", ONLY_BEST_EFFORT_ALLOWED, None, spec_config(resources=OVERRIDE_ALL_RESOURCES)),
+        ("other-namespace", ONLY_BEST_EFFORT_ALLOWED, None, spec_config(resources=OVERRIDE_ALL_RESOURCES)),
+        ("default", BEST_EFFORT_NOT_ALLOWED, OVERRIDE_ALL_RESOURCES, spec_config(OVERRIDE_ALL_RESOURCES)),
+        ("other-namespace", BEST_EFFORT_NOT_ALLOWED, OVERRIDE_ALL_RESOURCES, spec_config(OVERRIDE_ALL_RESOURCES)),
+    ])
+    def test_bootstrap(self, post, delete, resourcequota_list, namespace, resourcequota_spec, resources, spec_config):
+        resourcequota_list.return_value = \
+            [self.create_resourcequota(namespace, resourcequota_spec)] if resourcequota_spec else []
+        bootstrapper = BarePodBootstrapper()
+        channel = ReleaseChannel(None, None, {'image': 'example.com/image:tag'})
+        deployment_config = DeploymentConfig('foo', namespace, 'latest')
+        expected_pod = {
+            'metadata': {
+                'name': 'fiaas-deploy-daemon-bootstrap',
+                'namespace': namespace,
+                'labels': {'app': 'fiaas-deploy-daemon-bootstrap'},
+                'ownerReferences': []
+            },
+            'spec': {
+                'volumes': [],
+                'containers': [{
+                    'name': 'fiaas-deploy-daemon-bootstrap',
+                    'image': 'example.com/image:tag',
+                    'ports': [],
+                    'env': [],
+                    'envFrom': [],
+                    'volumeMounts': [],
+                    'imagePullPolicy': 'IfNotPresent',
+                    'command': ['fiaas-deploy-daemon-bootstrap'],
+                }],
+                'restartPolicy': 'Never',
+                'dnsPolicy': 'ClusterFirst',
+                'serviceAccountName': 'default',
+                'imagePullSecrets': [],
+                'initContainers': [],
+            }
+        }
+        if resources:
+            expected_pod['spec']['containers'][0]['resources'] = resources
+
+        bootstrapper(deployment_config, channel, spec_config=spec_config)
+
+        pytest.helpers.assert_any_call(delete, self.pod_uri(namespace=namespace, name='fiaas-deploy-daemon-bootstrap'))
+        pytest.helpers.assert_any_call(post, self.pod_uri(namespace=namespace), expected_pod)