From 0586b5dfb0d9fe78e032e6cfeaf7403349432615 Mon Sep 17 00:00:00 2001 From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com> Date: Mon, 24 Feb 2020 00:06:51 +0530 Subject: [PATCH] Don't allow duplicate cluster names --- cloudman/clusterman/api.py | 24 ++++-- cloudman/clusterman/exceptions.py | 2 +- .../management/commands/create_cluster.py | 2 +- .../clusterman/migrations/0001_initial.py | 83 +++++++++++++++++++ cloudman/clusterman/models.py | 2 +- cloudman/clusterman/serializers.py | 18 ++-- cloudman/clusterman/tests/test_cluster_api.py | 29 +++++-- .../clusterman/tests/test_mgmt_commands.py | 12 ++- .../commands/projman_create_project.py | 2 +- 9 files changed, 149 insertions(+), 25 deletions(-) create mode 100644 cloudman/clusterman/migrations/0001_initial.py diff --git a/cloudman/clusterman/api.py b/cloudman/clusterman/api.py index e4cc6f9b..eb80f9ba 100644 --- a/cloudman/clusterman/api.py +++ b/cloudman/clusterman/api.py @@ -1,9 +1,13 @@ """CloudMan Service API.""" import uuid + +from django.db import IntegrityError + from rest_framework.exceptions import PermissionDenied from cloudlaunch import models as cl_models from cloudlaunch_cli.api.client import APIClient +from . import exceptions from . import models from . import resources @@ -109,14 +113,18 @@ def get(self, cluster_id): def create(self, name, cluster_type, connection_settings, autoscale=True): self.check_permissions('clusters.add_cluster') - obj = models.CMCluster.objects.create( - name=name, cluster_type=cluster_type, - connection_settings=connection_settings, - autoscale=autoscale) - cluster = self.to_api_object(obj) - template = cluster.get_cluster_template() - template.setup() - return cluster + try: + obj = models.CMCluster.objects.create( + name=name, cluster_type=cluster_type, + connection_settings=connection_settings, + autoscale=autoscale) + cluster = self.to_api_object(obj) + template = cluster.get_cluster_template() + template.setup() + return cluster + except IntegrityError as e: + raise exceptions.CMDuplicateNameException( + "A cluster with name: %s already exists" % name) def update(self, cluster): self.check_permissions('clusters.change_cluster', cluster) diff --git a/cloudman/clusterman/exceptions.py b/cloudman/clusterman/exceptions.py index 6304e82f..64fc6fb0 100644 --- a/cloudman/clusterman/exceptions.py +++ b/cloudman/clusterman/exceptions.py @@ -1,5 +1,5 @@ # Exception hierarchy for cloudman -class InvalidStateException(Exception): +class CMDuplicateNameException(Exception): pass diff --git a/cloudman/clusterman/management/commands/create_cluster.py b/cloudman/clusterman/management/commands/create_cluster.py index 54eaee1b..62cea798 100644 --- a/cloudman/clusterman/management/commands/create_cluster.py +++ b/cloudman/clusterman/management/commands/create_cluster.py @@ -50,4 +50,4 @@ def create_cluster(name, cluster_type, settings): print("cluster created successfully.") except Exception as e: log.exception("An error occurred while creating the initial cluster!!:") - print("An error occurred while creating the initial cluster!!:", e) + print("An error occurred while creating the initial cluster!!:", str(e)) diff --git a/cloudman/clusterman/migrations/0001_initial.py b/cloudman/clusterman/migrations/0001_initial.py new file mode 100644 index 00000000..96aa764b --- /dev/null +++ b/cloudman/clusterman/migrations/0001_initial.py @@ -0,0 +1,83 @@ +# Generated by Django 2.2.10 on 2020-02-23 17:26 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('cloudlaunch', '0001_initial'), + ('djcloudbridge', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='CMAutoScaler', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=60)), + ('vm_type', models.CharField(max_length=200)), + ('min_nodes', models.IntegerField(default=0)), + ('max_nodes', models.IntegerField(default=None, null=True)), + ], + options={ + 'verbose_name': 'Cluster Autoscaler', + 'verbose_name_plural': 'Cluster Autoscalers', + }, + ), + migrations.CreateModel( + name='CMCluster', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('added', models.DateTimeField(auto_now_add=True)), + ('updated', models.DateTimeField(auto_now=True)), + ('name', models.CharField(max_length=60, unique=True)), + ('cluster_type', models.CharField(max_length=255)), + ('autoscale', models.BooleanField(default=True, help_text='Whether autoscaling is activated')), + ('_connection_settings', models.TextField(blank=True, db_column='connection_settings', help_text='External provider specific settings for this cluster.', max_length=16384, null=True)), + ], + options={ + 'verbose_name': 'Cluster', + 'verbose_name_plural': 'Clusters', + }, + ), + migrations.CreateModel( + name='GlobalSettings_SettingsStore', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('key', models.CharField(max_length=255)), + ('value', models.TextField()), + ], + ), + migrations.CreateModel( + name='CMClusterNode', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(max_length=60)), + ('autoscaler', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='nodegroup', to='clusterman.CMAutoScaler')), + ('cluster', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='node_list', to='clusterman.CMCluster')), + ('deployment', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='cm_cluster_node', to='cloudlaunch.ApplicationDeployment')), + ], + options={ + 'verbose_name': 'Cluster Node', + 'verbose_name_plural': 'Cluster Nodes', + }, + ), + migrations.AddField( + model_name='cmautoscaler', + name='cluster', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='autoscaler_list', to='clusterman.CMCluster'), + ), + migrations.AddField( + model_name='cmautoscaler', + name='zone', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='autoscaler_list', to='djcloudbridge.Zone'), + ), + migrations.AlterUniqueTogether( + name='cmautoscaler', + unique_together={('cluster', 'name')}, + ), + ] diff --git a/cloudman/clusterman/models.py b/cloudman/clusterman/models.py index 600d29c4..10cd7c05 100644 --- a/cloudman/clusterman/models.py +++ b/cloudman/clusterman/models.py @@ -21,7 +21,7 @@ class CMCluster(models.Model): added = models.DateTimeField(auto_now_add=True) # Automatically add timestamps when object is updated updated = models.DateTimeField(auto_now=True) - name = models.CharField(max_length=60) + name = models.CharField(max_length=60, unique=True) cluster_type = models.CharField(max_length=255, blank=False, null=False) autoscale = models.BooleanField( default=True, help_text="Whether autoscaling is activated") diff --git a/cloudman/clusterman/serializers.py b/cloudman/clusterman/serializers.py index 85027969..dd69d73f 100644 --- a/cloudman/clusterman/serializers.py +++ b/cloudman/clusterman/serializers.py @@ -1,11 +1,15 @@ """DRF serializers for the CloudMan Create API endpoints.""" from rest_framework import serializers +from rest_framework import status +from rest_framework.exceptions import ValidationError + from cloudlaunch import serializers as cl_serializers from djcloudbridge import models as cb_models from djcloudbridge.drf_helpers import CustomHyperlinkedIdentityField + from .api import CloudManAPI -from rest_framework.exceptions import ValidationError +from .exceptions import CMDuplicateNameException class CMClusterSerializer(serializers.Serializer): @@ -21,10 +25,14 @@ class CMClusterSerializer(serializers.Serializer): lookup_url_kwarg='cluster_pk') def create(self, valid_data): - return CloudManAPI.from_request(self.context['request']).clusters.create( - valid_data.get('name'), valid_data.get('cluster_type'), - valid_data.get('connection_settings'), - autoscale=valid_data.get('autoscale')) + try: + cmapi = CloudManAPI.from_request(self.context['request']) + return cmapi.clusters.create( + valid_data.get('name'), valid_data.get('cluster_type'), + valid_data.get('connection_settings'), + autoscale=valid_data.get('autoscale')) + except CMDuplicateNameException as e: + raise ValidationError(detail=str(e)) def update(self, instance, valid_data): instance.name = valid_data.get('name') or instance.name diff --git a/cloudman/clusterman/tests/test_cluster_api.py b/cloudman/clusterman/tests/test_cluster_api.py index 5eff5234..38dfe8f5 100644 --- a/cloudman/clusterman/tests/test_cluster_api.py +++ b/cloudman/clusterman/tests/test_cluster_api.py @@ -15,6 +15,8 @@ import responses +from clusterman.exceptions import CMDuplicateNameException + def load_test_data(filename): cluster_data_path = os.path.join( @@ -65,9 +67,6 @@ def setUp(self): responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusters/c-abcd1?action=generateKubeconfig', json={'config': load_kube_config()}, status=200) - def tearDown(self): - self.client.logout() - class CMClusterServiceTests(CMClusterServiceTestBase): @@ -148,6 +147,11 @@ def test_crud_cluster(self): # check it no longer exists self._check_no_clusters_exist() + def test_create_duplicate_name(self): + self._create_cluster() + response = self._create_cluster() + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST, response.data) + def test_create_unauthorized(self): self.client.force_login( User.objects.get_or_create(username='notaclusteradmin', is_staff=False)[0]) @@ -226,6 +230,20 @@ def create_mock_provider(self, name, config): responses.add_passthru('http://localhost') responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusterregistrationtoken', json={'nodeCommand': 'docker run rancher --worker'}, status=200) + responses.add(responses.GET, 'https://127.0.0.1:4430/v3/nodes/?clusterId=c-abcd1', + json= + {'data': [ + {'id': 'c-ph9ck:m-01606aca4649', + 'ipAddress': '10.1.1.1', + 'externalIpAddress': None + } + ]}, + status=200) + responses.add(responses.POST, 'https://127.0.0.1:4430/v3/nodes/c-ph9ck:m-01606aca4649?action=drain', + json={}, status=200) + responses.add(responses.DELETE, 'https://127.0.0.1:4430/v3/nodes/c-ph9ck:m-01606aca4649', + json={}, status=200) + super().setUp() @@ -267,12 +285,13 @@ def _check_cluster_node_exists(self, cluster_id, node_id): def _delete_cluster_node(self, cluster_id, node_id): responses.add(responses.GET, 'https://127.0.0.1:4430/v3/nodes/?clusterId=c-abcd1', - json=[ + json= + {'data': [ {'id': 'c-ph9ck:m-01606aca4649', 'ipAddress': '10.1.1.1', 'externalIpAddress': None } - ], + ]}, status=200) responses.add(responses.POST, 'https://127.0.0.1:4430/v3/nodes/c-ph9ck:m-01606aca4649?action=drain', json={}, status=200) diff --git a/cloudman/clusterman/tests/test_mgmt_commands.py b/cloudman/clusterman/tests/test_mgmt_commands.py index 11203f8d..e74c9f15 100644 --- a/cloudman/clusterman/tests/test_mgmt_commands.py +++ b/cloudman/clusterman/tests/test_mgmt_commands.py @@ -6,6 +6,7 @@ from django.contrib.auth.models import User from django.core.management import call_command from django.core.management.base import CommandError +from django.db import transaction from django.test import TestCase from djcloudbridge import models as cb_models @@ -36,9 +37,6 @@ def setUp(self): self.client.force_login( User.objects.get_or_create(username='admin', is_superuser=True)[0]) - def tearDown(self): - self.client.logout() - def test_import_cloud_data_no_args(self): with self.assertRaisesRegex(CommandError, "required: filename"): call_command('import_cloud_data') @@ -60,6 +58,14 @@ def test_create_cluster(self): cluster = cm_models.CMCluster.objects.get(name='test_cluster') self.assertEquals(cluster.cluster_type, 'KUBE_RANCHER') + def test_create_cluster_existing(self): + with transaction.atomic(): + call_command('create_cluster', 'test_cluster', 'KUBE_RANCHER', self.INITIAL_CLUSTER_DATA) + self.assertEqual(cm_models.CMCluster.objects.all().count(), 1) + with transaction.atomic(): + call_command('create_cluster', 'test_cluster', 'KUBE_RANCHER', self.INITIAL_CLUSTER_DATA) + self.assertEqual(cm_models.CMCluster.objects.all().count(), 1) + class CreateAutoScaleUserCommandTestCase(TestCase): diff --git a/cloudman/projman/management/commands/projman_create_project.py b/cloudman/projman/management/commands/projman_create_project.py index 1b3b77a1..d40e1292 100644 --- a/cloudman/projman/management/commands/projman_create_project.py +++ b/cloudman/projman/management/commands/projman_create_project.py @@ -29,6 +29,6 @@ def create_project(name): except Exception as e: log.exception(f"An error occurred while " f"creating the project '{name}':", e) - print(f"An error occurred while creating the project '{name}':", e) + print(f"An error occurred while creating the project '{name}':", str(e)) # Re-raise the exception raise e