Skip to content

Commit

Permalink
Don't allow duplicate cluster names
Browse files Browse the repository at this point in the history
  • Loading branch information
nuwang committed Feb 23, 2020
1 parent aa31fcc commit 0586b5d
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 25 deletions.
24 changes: 16 additions & 8 deletions cloudman/clusterman/api.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cloudman/clusterman/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Exception hierarchy for cloudman


class InvalidStateException(Exception):
class CMDuplicateNameException(Exception):
pass
2 changes: 1 addition & 1 deletion cloudman/clusterman/management/commands/create_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
83 changes: 83 additions & 0 deletions cloudman/clusterman/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -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')},
),
]
2 changes: 1 addition & 1 deletion cloudman/clusterman/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
18 changes: 13 additions & 5 deletions cloudman/clusterman/serializers.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Expand Down
29 changes: 24 additions & 5 deletions cloudman/clusterman/tests/test_cluster_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

import responses

from clusterman.exceptions import CMDuplicateNameException


def load_test_data(filename):
cluster_data_path = os.path.join(
Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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()


Expand Down Expand Up @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions cloudman/clusterman/tests/test_mgmt_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -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):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 0586b5d

Please sign in to comment.