fix(app): create image pull secrets outside of the async deploy loop (#1032)

The issue is that multiple process types can cause 409 issues when ran in the async process. It's fine to run this ahead of any deploy as this secret is not tied to the release version of an application

Moved pull image secret and registry config generation into the App model as it makes more sense given where it gets used.
Also reduces `django.settings` in the `scheduler`

Fixes #1031

Author: helgi

rootfs/api/models/app.py

@@ -1,7 +1,10 @@
 import backoff
+import base64
 from collections import OrderedDict
 from datetime import datetime
+from docker.auth import auth as docker_auth
 import functools
+import json
 import logging
 import random
 import re
@@ -398,6 +401,7 @@ def _scale_pods(self, scale_types):
         version = "v{}".format(release.version)
         image = release.image
         envs = self._build_env_vars(release.build.type, version, image, release.config.values)
+        registry = release.config.registry
 
         # see if the app config has deploy batch preference, otherwise use global
         batches = release.config.values.get('DEIS_DEPLOY_BATCHES', settings.DEIS_DEPLOY_BATCHES)
@@ -408,6 +412,9 @@ def _scale_pods(self, scale_types):
         # get application level pod termination grace period
         pod_termination_grace_period_seconds = release.config.values.get('KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS', settings.KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS)  # noqa
 
+        # create image pull secret if needed
+        image_pull_secret_name = self.image_pull_secret(self.id, registry, image)
+
         tasks = []
         for scale_type, replicas in scale_types.items():
             # only web / cmd are routable
@@ -427,7 +434,7 @@ def _scale_pods(self, scale_types):
                 'cpu': release.config.cpu,
                 'tags': release.config.tags,
                 'envs': envs,
-                'registry': release.config.registry,
+                'registry': registry,
                 'version': version,
                 'replicas': replicas,
                 'app_type': scale_type,
@@ -437,6 +444,7 @@ def _scale_pods(self, scale_types):
                 'deploy_batches': batches,
                 'deploy_timeout': deploy_timeout,
                 'pod_termination_grace_period_seconds': pod_termination_grace_period_seconds,
+                'image_pull_secret_name': image_pull_secret_name,
             }
 
             # gather all proc types to be deployed
@@ -482,6 +490,12 @@ def deploy(self, release, force_deploy=False):
             self.structure = self._default_structure(release)
             self.save()
 
+        image = release.image
+        registry = release.config.registry
+        version = "v{}".format(release.version)
+        envs = self._build_env_vars(release.build.type, version, image, release.config.values)
+        tags = release.config.tags
+
         # see if the app config has deploy batch preference, otherwise use global
         batches = release.config.values.get('DEIS_DEPLOY_BATCHES', settings.DEIS_DEPLOY_BATCHES)
 
@@ -493,12 +507,11 @@ def deploy(self, release, force_deploy=False):
         # get application level pod termination grace period
         pod_termination_grace_period_seconds = release.config.values.get('KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS', settings.KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS)  # noqa
 
+        # create image pull secret if needed
+        image_pull_secret_name = self.image_pull_secret(self.id, registry, image)
+
         # deploy application to k8s. Also handles initial scaling
         deploys = {}
-        image = release.image
-        version = "v{}".format(release.version)
-        envs = self._build_env_vars(release.build.type, version, image, release.config.values)
-        tags = release.config.tags
 
         for scale_type, replicas in self.structure.items():
             # only web / cmd are routable
@@ -518,7 +531,7 @@ def deploy(self, release, force_deploy=False):
                 'cpu': release.config.cpu,
                 'tags': tags,
                 'envs': envs,
-                'registry': release.config.registry,
+                'registry': registry,
                 'replicas': replicas,
                 'version': version,
                 'app_type': scale_type,
@@ -530,6 +543,7 @@ def deploy(self, release, force_deploy=False):
                 'deployment_history_limit': deployment_history,
                 'release_summary': release.summary,
                 'pod_termination_grace_period_seconds': pod_termination_grace_period_seconds,
+                'image_pull_secret_name': image_pull_secret_name,
             }
 
         # Sort deploys so routable comes first
@@ -733,28 +747,34 @@ def pod_name(size=5, chars=string.ascii_lowercase + string.digits):
         if release.build is None:
             raise DeisException('No build associated with this release to run this command')
 
+        image = release.image
+        registry = release.config.registry
+        version = "v{}".format(release.version)
+        envs = self._build_env_vars(release.build.type, version, image, release.config.values)
+
         # see if the app config has deploy timeout preference, otherwise use global
         deploy_timeout = release.config.values.get('DEIS_DEPLOY_TIMEOUT', settings.DEIS_DEPLOY_TIMEOUT)  # noqa
 
         # get application level pod termination grace period
         pod_termination_grace_period_seconds = release.config.values.get('KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS', settings.KUBERNETES_POD_TERMINATION_GRACE_PERIOD_SECONDS)  # noqa
 
+        # create image pull secret if needed
+        image_pull_secret_name = self.image_pull_secret(self.id, registry, image)
+
         name = self._get_job_id(scale_type) + '-' + pod_name()
         self.log("{} on {} runs '{}'".format(user.username, name, command))
 
-        image = release.image
-        version = "v{}".format(release.version)
-        envs = self._build_env_vars(release.build.type, version, image, release.config.values)
         kwargs = {
             'memory': release.config.memory,
             'cpu': release.config.cpu,
             'tags': release.config.tags,
             'envs': envs,
-            'registry': release.config.registry,
+            'registry': registry,
             'version': version,
             'build_type': release.build.type,
             'deploy_timeout': deploy_timeout,
             'pod_termination_grace_period_seconds': pod_termination_grace_period_seconds,
+            'image_pull_secret_name': image_pull_secret_name,
         }
 
         try:
@@ -962,3 +982,70 @@ def autoscale(self, proc_type, autoscale):
             else:
                 # let the user know about any other errors
                 raise ServiceUnavailable(str(e)) from e
+
+    def image_pull_secret(self, namespace, registry, image):
+        """
+        Take registry information and set as an imagePullSecret for an RC / Deployment
+        http://kubernetes.io/docs/user-guide/images/#specifying-imagepullsecrets-on-a-pod
+        """
+        docker_config, name, create = self._get_private_registry_config(image, registry)
+        if create is None:
+            return
+        elif create:
+            data = {'.dockerconfigjson': docker_config}
+            try:
+                self._scheduler.secret.get(namespace, name)
+            except KubeHTTPException:
+                self._scheduler.secret.create(
+                    namespace,
+                    name,
+                    data,
+                    secret_type='kubernetes.io/dockerconfigjson'
+                )
+            else:
+                self._scheduler.secret.update(
+                    namespace,
+                    name,
+                    data,
+                    secret_type='kubernetes.io/dockerconfigjson'
+                )
+
+        return name
+
+    def _get_private_registry_config(self, image, registry=None):
+        name = settings.REGISTRY_SECRET_PREFIX
+        if registry:
+            # try to get the hostname information
+            hostname = registry.get('hostname', None)
+            if not hostname:
+                hostname, _ = docker_auth.split_repo_name(image)
+
+            if hostname == docker_auth.INDEX_NAME:
+                hostname = 'https://index.docker.io/v1/'
+
+            username = registry.get('username')
+            password = registry.get('password')
+        elif settings.REGISTRY_LOCATION == 'off-cluster':
+            secret = self._scheduler.secret.get('deis', 'registry-secret').json()
+            username = secret['data']['username']
+            password = secret['data']['password']
+            hostname = secret['data']['hostname']
+            if hostname == '':
+                hostname = 'https://index.docker.io/v1/'
+            name = name + '-' + settings.REGISTRY_LOCATION
+        elif settings.REGISTRY_LOCATION in ['ecr', 'gcr']:
+            return None, name + '-' + settings.REGISTRY_LOCATION, False
+        else:
+            return None, None, None
+
+        # create / update private registry secret
+        auth = bytes('{}:{}'.format(username, password), 'UTF-8')
+        # value has to be a base64 encoded JSON
+        docker_config = json.dumps({
+            'auths': {
+                hostname: {
+                    'auth': base64.b64encode(auth).decode(encoding='UTF-8')
+                }
+            }
+        })
+        return docker_config, name, True

rootfs/api/tests/test_app.py

@@ -3,6 +3,8 @@
 
 Run the tests with "./manage.py test api"
 """
+import base64
+import json
 import logging
 from unittest import mock
 import random
@@ -11,6 +13,7 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.cache import cache
+from django.test.utils import override_settings
 from rest_framework.authtoken.models import Token
 
 from api.models import App
@@ -508,6 +511,60 @@ def test_app_service_metadata(self, mock_requests):
         self.assertIn('labels', svc['metadata'])
         self.assertIn('annotations', svc['metadata'])
 
+    def test_get_private_registry_config(self, mock_requests):
+        registry = {'username': 'test', 'password': 'test'}
+        auth = bytes('{}:{}'.format("test", "test"), 'UTF-8')
+        encAuth = base64.b64encode(auth).decode(encoding='UTF-8')
+        image = 'test/test'
+
+        docker_config, name, create = App()._get_private_registry_config(image, registry)
+        dockerConfig = json.loads(docker_config)
+        expected = {"https://index.docker.io/v1/": {"auth": encAuth}}
+        self.assertEqual(dockerConfig.get('auths'), expected)
+        self.assertEqual(name, "private-registry")
+        self.assertEqual(create, True)
+
+        image = "quay.io/test/test"
+        docker_config, name, create = App()._get_private_registry_config(image, registry)
+        dockerConfig = json.loads(docker_config)
+        expected = {"quay.io": {"auth": encAuth}}
+        self.assertEqual(dockerConfig.get('auths'), expected)
+        self.assertEqual(name, "private-registry")
+        self.assertEqual(create, True)
+
+    @override_settings(REGISTRY_LOCATION="ecr")
+    def test_get_private_registry_config_ecr(self, mock_requests):
+        registry = {}
+        image = "test.com/test/test"
+        docker_config, name, create = App()._get_private_registry_config(image, registry)
+        self.assertEqual(docker_config, None)
+        self.assertEqual(name, "private-registry-ecr")
+        self.assertEqual(create, False)
+
+    @override_settings(REGISTRY_LOCATION="off-cluster")
+    def test_get_private_registry_config_off_cluster(self, mock_requests):
+        registry = {}
+        auth = bytes('{}:{}'.format("test", "test"), 'UTF-8')
+        encAuth = base64.b64encode(auth).decode(encoding='UTF-8')
+        image = "test.com/test/test"
+        docker_config, name, create = App()._get_private_registry_config(image, registry)
+        dockerConfig = json.loads(docker_config)
+        expected = {"https://index.docker.io/v1/": {
+            "auth": encAuth
+        }}
+        self.assertEqual(dockerConfig.get('auths'), expected)
+        self.assertEqual(name, "private-registry-off-cluster")
+        self.assertEqual(create, True)
+
+    @override_settings(REGISTRY_LOCATION="ecra")
+    def test_get_private_registry_config_bad_registry_location(self, mock_requests):
+        registry = {}
+        image = "test.com/test/test"
+        docker_config, name, create = App()._get_private_registry_config(image, registry)
+        self.assertEqual(docker_config, None)
+        self.assertEqual(name, None)
+        self.assertEqual(create, None)
+
 
 FAKE_LOG_DATA = """
 2013-08-15 12:41:25 [33454] [INFO] Starting gunicorn 17.5

rootfs/scheduler/resources/pod.py

@@ -1,7 +1,4 @@
-import base64
 from datetime import datetime, timedelta
-from docker.auth import auth as docker_auth
-import json
 import operator
 import os
 import time
@@ -159,7 +156,9 @@ def manifest(self, namespace, name, image, **kwargs):
         container_name = namespace + '-' + app_type
         self._set_container(namespace, container_name, container, **kwargs)
         # add image to the mix
-        self._set_image_secret(spec, namespace, **kwargs)
+        if kwargs.get('image_pull_secret_name', None) is not None:
+            # apply image pull secret to a Pod spec
+            spec['imagePullSecrets'] = [{'name': kwargs.get('image_pull_secret_name')}]
 
         spec['containers'] = [container]
 
@@ -317,72 +316,6 @@ def _default_dockerapp_readiness_probe(self, port, delay=5, timeout=5, period_se
         }
         return readinessprobe
 
-    def _get_private_registry_config(self, registry, image):
-        secret_name = settings.REGISTRY_SECRET_PREFIX
-        if registry:
-            # try to get the hostname information
-            hostname = registry.get('hostname', None)
-            if not hostname:
-                hostname, _ = docker_auth.split_repo_name(image)
-            if hostname == docker_auth.INDEX_NAME:
-                hostname = "https://index.docker.io/v1/"
-            username = registry.get('username')
-            password = registry.get('password')
-        elif settings.REGISTRY_LOCATION == 'off-cluster':
-            secret = self.secret.get('deis', 'registry-secret').json()
-            username = secret['data']['username']
-            password = secret['data']['password']
-            hostname = secret['data']['hostname']
-            if hostname == '':
-                hostname = "https://index.docker.io/v1/"
-            secret_name = secret_name+"-"+settings.REGISTRY_LOCATION
-        elif settings.REGISTRY_LOCATION in ['ecr', 'gcr']:
-            return None, secret_name+"-"+settings.REGISTRY_LOCATION, False
-        else:
-            return None, None, None
-
-        # create / update private registry secret
-        auth = bytes('{}:{}'.format(username, password), 'UTF-8')
-        # value has to be a base64 encoded JSON
-        docker_config = json.dumps({
-            "auths": {
-                hostname: {
-                    "auth": base64.b64encode(auth).decode(encoding='UTF-8')
-                }
-            }
-        })
-        return docker_config, secret_name, True
-
-    def _set_image_secret(self, data, namespace, **kwargs):
-        """
-        Take registry information and set as an imagePullSecret for an RC / Deployment
-        http://kubernetes.io/docs/user-guide/images/#specifying-imagepullsecrets-on-a-pod
-        """
-        docker_config, secret_name, secret_create = self._get_private_registry_config(kwargs.get('registry', {}), kwargs.get('image'))  # noqa
-        if secret_create is None:
-            return
-        elif secret_create:
-            secret_data = {'.dockerconfigjson': docker_config}
-            try:
-                self.secret.get(namespace, secret_name)
-            except KubeHTTPException:
-                self.secret.create(
-                    namespace,
-                    secret_name,
-                    secret_data,
-                    secret_type='kubernetes.io/dockerconfigjson'
-                )
-            else:
-                self.secret.update(
-                    namespace,
-                    secret_name,
-                    secret_data,
-                    secret_type='kubernetes.io/dockerconfigjson'
-                )
-
-        # apply image pull secret to a Pod spec
-        data['imagePullSecrets'] = [{'name': secret_name}]
-
     def delete(self, namespace, name):
         # get timeout info from pod
         pod = self.pod.get(namespace, name).json()