From 71b0461d8acb8073bb6fd80d342adc2249ed886c Mon Sep 17 00:00:00 2001 From: helgi Date: Wed, 17 Aug 2016 16:20:36 -0700 Subject: [PATCH] fix(registry): tell a user they need PORT when using off-cluster native iaas registry registry:set does the same kind of warning but since this is a whole platform setting then we need to warn on deploy as well Fixes #986 --- rootfs/api/models/release.py | 6 ++-- rootfs/api/tests/test_release.py | 35 +++++++++++++++++++----- rootfs/scheduler/tests/test_scheduler.py | 16 ++++++++--- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/rootfs/api/models/release.py b/rootfs/api/models/release.py index 551e0c0a4..e0bcc0c1e 100644 --- a/rootfs/api/models/release.py +++ b/rootfs/api/models/release.py @@ -152,8 +152,10 @@ def get_port(self): # application has registry auth - $PORT is required if (creds is not None) or (settings.REGISTRY_LOCATION != 'on-cluster'): if envs.get('PORT', None) is None: - self.app.log('Private registry detected but no $PORT defined. Defaulting to $PORT 5000', logging.WARNING) # noqa - return 5000 + raise DeisException( + 'PORT needs to be set in the config ' + 'when using a private registry' + ) # User provided PORT return int(envs.get('PORT')) diff --git a/rootfs/api/tests/test_release.py b/rootfs/api/tests/test_release.py index 9d894de27..365627055 100644 --- a/rootfs/api/tests/test_release.py +++ b/rootfs/api/tests/test_release.py @@ -10,12 +10,13 @@ from django.contrib.auth.models import User from django.core.cache import cache -from django.conf import settings +from django.test.utils import override_settings from unittest import mock from rest_framework.authtoken.models import Token from api.models import App, Release from scheduler import KubeHTTPException +from api.exceptions import DeisException from api.tests import adapter, mock_port, DeisTransactionTestCase import requests_mock @@ -380,20 +381,19 @@ def test_release_get_port(self, mock_requests): # TODO(bacongobbler): test dockerfile ports + @override_settings(REGISTRY_LOCATION="off-cluster") def test_release_external_registry(self, mock_requests): """ Test that get_port always returns the proper value. """ - app_id = "test" - body = {'id': app_id} - response = self.client.post('/v2/apps', body,) - self.assertEqual(response.status_code, 201, response.data) + app_id = self.create_app() + + # set the required port for external registries body = {'values': json.dumps({'PORT': '3000'})} - config_response = self.client.post('/v2/apps/test/config', body) + config_response = self.client.post('/v2/apps/{}/config'.format(app_id), body) self.assertEqual(config_response.status_code, 201, config_response.data) app = App.objects.get(id=app_id) - settings.REGISTRY_LOCATION = "off-cluster" url = '/v2/apps/{app_id}/builds'.format(**locals()) body = {'image': 'test/autotest/example'} response = self.client.post(url, body) @@ -403,3 +403,24 @@ def test_release_external_registry(self, mock_requests): self.assertEqual(release.get_port(), 3000) self.assertEqual(release.image, 'test/autotest/example') + + @override_settings(REGISTRY_LOCATION="off-cluster") + def test_release_external_registry_no_port(self, mock_requests): + """ + Test that an exception is raised when registry if off-cluster but + no port is provided. + """ + app_id = self.create_app() + + app = App.objects.get(id=app_id) + url = '/v2/apps/{app_id}/builds'.format(**locals()) + body = {'image': 'test/autotest/example'} + response = self.client.post(url, body) + self.assertEqual(response.status_code, 400, response.data) + + with self.assertRaises( + DeisException, + msg='PORT needs to be set in the config when using a private registry' + ): + release = app.release_set.latest() + release.get_port() diff --git a/rootfs/scheduler/tests/test_scheduler.py b/rootfs/scheduler/tests/test_scheduler.py index 6941b8a81..7317ec7f6 100644 --- a/rootfs/scheduler/tests/test_scheduler.py +++ b/rootfs/scheduler/tests/test_scheduler.py @@ -5,7 +5,7 @@ """ from django.core.cache import cache from django.test import TestCase -from django.conf import settings +from django.test.utils import override_settings from scheduler import mock import base64 @@ -143,7 +143,8 @@ def test_get_private_registry_config(self): self.assertEqual(secret_name, "private-registry") self.assertEqual(secret_create, True) - settings.REGISTRY_LOCATION = "ecr" + @override_settings(REGISTRY_LOCATION="ecr") + def test_get_private_registry_config_ecr(self): registry = {} image = "test.com/test/test" docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image) # noqa @@ -151,7 +152,12 @@ def test_get_private_registry_config(self): self.assertEqual(secret_name, "private-registry-ecr") self.assertEqual(secret_create, False) - settings.REGISTRY_LOCATION = "off-cluster" + @override_settings(REGISTRY_LOCATION="off-cluster") + def test_get_private_registry_config_off_cluster(self): + registry = {} + auth = bytes('{}:{}'.format("test", "test"), 'UTF-8') + encAuth = base64.b64encode(auth).decode(encoding='UTF-8') + image = "test.com/test/test" docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image) # noqa dockerConfig = json.loads(docker_config) expected = {"https://index.docker.io/v1/": { @@ -161,7 +167,9 @@ def test_get_private_registry_config(self): self.assertEqual(secret_name, "private-registry-off-cluster") self.assertEqual(secret_create, True) - settings.REGISTRY_LOCATION = "ecra" + @override_settings(REGISTRY_LOCATION="ecra") + def test_get_private_registry_config_bad_registry_location(self): + registry = {} image = "test.com/test/test" docker_config, secret_name, secret_create = self.scheduler._get_private_registry_config(registry, image) # noqa self.assertEqual(docker_config, None)