Merge pull request #106 from Cryptophobia/ess-fix-autoscale-take-2

Fix for autoscale on k8s-1.9+ without breaking manual scaling

charts/controller/templates/controller-clusterrole.yaml

@@ -44,7 +44,7 @@ rules:
 - apiGroups: ["extensions", "apps"]
   resources: ["deployments"]
   verbs: ["get", "list", "create", "update", "delete"]
-- apiGroups: ["extensions"]
+- apiGroups: ["extensions", "apps"]
   resources: ["deployments/scale", "replicasets/scale"]
   verbs: ["get", "update"]
 - apiGroups: ["extensions", "autoscaling"]

rootfs/api/models/app.py

@@ -968,7 +968,9 @@ def autoscale(self, proc_type, autoscale):
         """
         name = '{}-{}'.format(self.id, proc_type)
         # basically fake out a Deployment object (only thing we use) to assign to the HPA
-        target = {'kind': 'Deployment', 'metadata': {'name': name}}
+        target = {'apiVersion': 'extensions/v1beta1',
+                  'kind': 'Deployment',
+                  'metadata': {'name': name}}
 
         try:
             # get the target for autoscaler, in this case Deployment

rootfs/scheduler/__init__.py

@@ -1,11 +1,12 @@
 from collections import OrderedDict
 from datetime import datetime
 import logging
-from packaging.version import Version
+from packaging.version import Version, parse
 import requests
 import requests.exceptions
 from requests_toolbelt import user_agent
 import time
+import re
 from urllib.parse import urljoin
 
 from api import __version__ as deis_version
@@ -84,7 +85,9 @@ def version(self):
             raise KubeHTTPException(response, 'fetching Kubernetes version')
 
         data = response.json()
-        return Version('{}.{}'.format(data['major'], data['minor']))
+        parsed_version = parse(
+            re.sub("[^0-9\.]", '', str('{}.{}'.format(data['major'], data['minor']))))
+        return Version('{}'.format(parsed_version))
 
     @staticmethod
     def parse_date(date):

rootfs/scheduler/resources/deployment.py

@@ -1,13 +1,22 @@
 from datetime import datetime, timedelta
 import json
 import time
+
+from packaging.version import parse
+
 from scheduler.resources import Resource
 from scheduler.exceptions import KubeException, KubeHTTPException
 
 
 class Deployment(Resource):
     api_prefix = 'apis'
-    api_version = 'extensions/v1beta1'
+
+    @property
+    def api_version(self):
+        if self.version() >= parse("1.9.0"):
+            return 'extensions/v1beta1'
+
+        return 'extensions/v1beta1'
 
     def get(self, namespace, name=None, **kwargs):
         """
@@ -43,7 +52,7 @@ def manifest(self, namespace, name, image, entrypoint, command, spec_annotations
 
         manifest = {
             'kind': 'Deployment',
-            'apiVersion': 'extensions/v1beta1',
+            'apiVersion': self.api_version,
             'metadata': {
                 'name': name,
                 'labels': labels,

rootfs/scheduler/resources/horizontalpodautoscaler.py

@@ -75,6 +75,7 @@ def manifest(self, namespace, name, app_type, target, **kwargs):
             manifest['spec']['targetCPUUtilizationPercentage'] = cpu_percent
 
             manifest['spec']['scaleTargetRef'] = {
+                'apiVersion': target['apiVersion'],
                 # only works with Deployments, RS and RC
                 'kind': target['kind'],
                 'name': target['metadata']['name'],

rootfs/scheduler/tests/test_deployments.py

@@ -3,6 +3,9 @@
 
 Run the tests with './manage.py test scheduler'
 """
+from unittest import mock
+import copy
+from packaging.version import parse, Version, InvalidVersion
 from scheduler import KubeHTTPException, KubeException
 from scheduler.tests import TestCase
 from scheduler.utils import generate_random_name
@@ -73,6 +76,57 @@ def scale(self, namespace=None, name=generate_random_name(), **kwargs):
         self.scheduler.scale(namespace, name, **kwargs)
         return name
 
+    def test_good_init_api_version(self):
+        try:
+            data = "1.13"
+            Version('{}'.format(data))
+        except InvalidVersion:
+            self.fail("Version {} raised InvalidVersion exception!".format(data))
+
+    def test_bad_init_api_version(self):
+        data = "1.13+"
+        with self.assertRaises(
+            InvalidVersion,
+            msg='packaging.version.InvalidVersion: Invalid version: {}'.format(data)  # noqa
+        ):
+            Version('{}'.format(data))
+
+    def test_deployment_api_version_1_9_and_up(self):
+        cases = ['1.12', '1.11', '1.10', '1.9']
+
+        deployment = copy.copy(self.scheduler.deployment)
+
+        expected = 'extensions/v1beta1'
+
+        for canonical in cases:
+            deployment.version = mock.MagicMock(return_value=parse(canonical))
+            actual = deployment.api_version
+            self.assertEqual(
+                    expected,
+                    actual,
+                    "{} breaks - expected {}, got {}".format(
+                        canonical,
+                        expected,
+                        actual))
+
+    def test_deployment_api_version_1_8_and_lower(self):
+        cases = ['1.8', '1.7', '1.6', '1.5', '1.4', '1.3', '1.2']
+
+        deployment = copy.copy(self.scheduler.deployment)
+
+        expected = 'extensions/v1beta1'
+
+        for canonical in cases:
+            deployment.version = mock.MagicMock(return_value=parse(canonical))
+            actual = deployment.api_version
+            self.assertEqual(
+                    expected,
+                    actual,
+                    "{} breaks - expected {}, got {}".format(
+                        canonical,
+                        expected,
+                        actual))
+
     def test_create_failure(self):
         with self.assertRaises(
             KubeHTTPException,

rootfs/scheduler/tests/test_kubehttpclient_version.py

@@ -0,0 +1,98 @@
+"""
+Unit tests for the Deis scheduler module.
+
+Run the tests with "./manage.py test scheduler"
+"""
+import requests
+import requests_mock
+from unittest import mock
+from packaging.version import parse
+
+from django.test import TestCase
+
+import scheduler
+
+
+def mock_session_for_version(blah=None):
+    return requests.Session()
+
+
+def connection_refused_matcher(request):
+    raise requests.ConnectionError("connection refused")
+
+
+@mock.patch('scheduler.get_session', mock_session_for_version)
+class KubeHTTPClientTest(TestCase):
+    """Tests kubernetes HTTP client version calls"""
+
+    def setUp(self):
+        self.adapter = requests_mock.Adapter()
+        self.url = 'http://versiontest.example.com'
+        self.path = '/version'
+
+        # use the real scheduler client.
+        self.scheduler = scheduler.KubeHTTPClient(self.url)
+        self.scheduler.session.mount(self.url, self.adapter)
+
+    def test_version_for_gke(self):
+        """
+        Ensure that version() sanitizes info from GKE clusters
+        """
+
+        cases = {
+                "1.12": {"major": "1", "minor": "12-gke"},
+                "1.10": {"major": "1", "minor": "10-gke"},
+                "1.9": {"major": "1", "minor": "9-gke"},
+                "1.8": {"major": "1", "minor": "8-gke"},
+                }
+
+        for canonical in cases:
+            resp = cases[canonical]
+            self.adapter.register_uri('GET', self.url + self.path, json=resp)
+
+            expected = parse(canonical)
+            actual = self.scheduler.version()
+
+            self.assertEqual(expected, actual, "{} breaks".format(resp))
+
+    def test_version_for_eks(self):
+        """
+        Ensure that version() sanitizes info from EKS clusters
+        """
+
+        cases = {
+                "1.12": {"major": "1", "minor": "12+"},
+                "1.10": {"major": "1", "minor": "10+"},
+                "1.9": {"major": "1", "minor": "9+"},
+                "1.8": {"major": "1", "minor": "8+"},
+                }
+
+        for canonical in cases:
+            resp = cases[canonical]
+            self.adapter.register_uri('GET', self.url + self.path, json=resp)
+
+            expected = parse(canonical)
+            actual = self.scheduler.version()
+
+            self.assertEqual(expected, actual, "{} breaks".format(resp))
+
+    def test_version_vanilla(self):
+        """
+        Ensure that version() sanitizes info from vanilla k8s clusters
+        """
+
+        cases = {
+                "1.12": {"major": "1", "minor": "12"},
+                "1.10": {"major": "1", "minor": "10"},
+                "1.9": {"major": "1", "minor": "9"},
+                "1.8": {"major": "1", "minor": "8"},
+                }
+
+        for canonical in cases:
+            resp = cases[canonical]
+            self.adapter.register_uri('GET', self.url + self.path, json=resp)
+
+            expected = parse(canonical)
+            actual = self.scheduler.version()
+
+            self.assertEqual(expected, actual, "{} breaks".format(resp))