From 65a999c2c6f1d25c369137b464001fc55204b4fe Mon Sep 17 00:00:00 2001
From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
Date: Sun, 23 Feb 2020 00:41:49 +0530
Subject: [PATCH 1/4] Impersonate the admin user when autoscaling

---
 cloudman/clusterman/rules.py                  |  2 ++
 cloudman/clusterman/tests/test_cluster_api.py |  2 +-
 cloudman/clusterman/views.py                  | 32 ++++++++++++++++---
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/cloudman/clusterman/rules.py b/cloudman/clusterman/rules.py
index 6ea5cfd5..17729731 100644
--- a/cloudman/clusterman/rules.py
+++ b/cloudman/clusterman/rules.py
@@ -35,3 +35,5 @@ def has_autoscale_permissions(user, obj):
 rules.add_perm('clusternodes.add_clusternode', is_node_owner | has_autoscale_permissions | rules.is_staff)
 rules.add_perm('clusternodes.change_clusternode', is_node_owner | has_autoscale_permissions | rules.is_staff)
 rules.add_perm('clusternodes.delete_clusternode', is_node_owner | has_autoscale_permissions | rules.is_staff)
+
+rules.add_perm('autoscalers.can_autoscale', has_autoscale_permissions | rules.is_staff)
diff --git a/cloudman/clusterman/tests/test_cluster_api.py b/cloudman/clusterman/tests/test_cluster_api.py
index bef49ffa..9516dc85 100644
--- a/cloudman/clusterman/tests/test_cluster_api.py
+++ b/cloudman/clusterman/tests/test_cluster_api.py
@@ -61,7 +61,7 @@ def setUp(self):
         self.addCleanup(patcher_migrate_result.stop)
 
         self.client.force_login(
-            User.objects.get_or_create(username='clusteradmin', is_staff=True)[0])
+            User.objects.get_or_create(username='clusteradmin', is_superuser=True, is_staff=True)[0])
         responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusters/c-abcd1?action=generateKubeconfig',
                       json={'config': load_kube_config()}, status=200)
 
diff --git a/cloudman/clusterman/views.py b/cloudman/clusterman/views.py
index daa7088c..7942cf9d 100644
--- a/cloudman/clusterman/views.py
+++ b/cloudman/clusterman/views.py
@@ -1,4 +1,6 @@
 """CloudMan Create views."""
+from django.contrib.auth.models import User
+
 from rest_framework.authentication import SessionAuthentication, BasicAuthentication
 from rest_framework.permissions import IsAuthenticated
 from rest_framework import viewsets, mixins
@@ -6,6 +8,8 @@
 from djcloudbridge import drf_helpers
 from . import serializers
 from .api import CloudManAPI
+from .api import CMServiceContext
+
 
 
 class ClusterViewSet(drf_helpers.CustomModelViewSet):
@@ -91,10 +95,20 @@ class ClusterScaleUpSignalViewSet(CustomCreateOnlyModelViewSet):
     authentication_classes = [SessionAuthentication, BasicAuthentication]
 
     def perform_create(self, serializer):
+        # first, check whether the current user has permissions to
+        # autoscale
+        cmapi = CloudManAPI.from_request(self.request)
+        cmapi.check_permissions('autoscalers.can_autoscale')
+        # If so, the remaining actions must be carried out as the admin user
+        # since we must use cloud credentials from the admin profile.
         zone_name = serializer.validated_data.get(
             'commonLabels', {}).get('availability_zone')
-        cluster = CloudManAPI.from_request(self.request).clusters.get(
-            self.kwargs["cluster_pk"])
+        # TODO: This is brittle because it assumes the first superuser
+        # found has cloud credentials. Instead, we could globally store
+        # which admin account the autoscale user should impersonate.
+        admin = User.objects.filter(is_superuser=True).first()
+        cmapi = CloudManAPI(CMServiceContext(user=admin))
+        cluster = cmapi.clusters.get(self.kwargs["cluster_pk"])
         if cluster:
             return cluster.scaleup(zone_name=zone_name)
         else:
@@ -111,10 +125,20 @@ class ClusterScaleDownSignalViewSet(CustomCreateOnlyModelViewSet):
     authentication_classes = [SessionAuthentication, BasicAuthentication]
 
     def perform_create(self, serializer):
+        # first, check whether the current user has permissions to
+        # autoscale
+        cmapi = CloudManAPI.from_request(self.request)
+        cmapi.check_permissions('autoscalers.can_autoscale')
+        # If so, the remaining actions must be carried out as the admin user
+        # since we must use cloud credentials from the admin profile.
         zone_name = serializer.validated_data.get(
             'commonLabels', {}).get('availability_zone')
-        cluster = CloudManAPI.from_request(self.request).clusters.get(
-            self.kwargs["cluster_pk"])
+        # TODO: This is brittle because it assumes the first superuser
+        # found has cloud credentials. Instead, we could globally store
+        # which admin account the autoscale user should impersonate.
+        admin = User.objects.filter(is_superuser=True).first()
+        cmapi = CloudManAPI(CMServiceContext(user=admin))
+        cluster = cmapi.clusters.get(self.kwargs["cluster_pk"])
         if cluster:
             return cluster.scaledown(zone_name=zone_name)
         else:

From 6af372baa55179438282aac9c6cf69de9099ea88 Mon Sep 17 00:00:00 2001
From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
Date: Sun, 23 Feb 2020 02:18:29 +0530
Subject: [PATCH 2/4] Added support for impersonating a user when autoscaling

---
 .../commands/create_autoscale_user.py         | 16 ++++++-
 .../clusterman/migrations/0001_initial.py     | 10 +++-
 cloudman/clusterman/models.py                 | 10 ++++
 cloudman/clusterman/tests/test_cluster_api.py | 48 +++++++++++++++----
 .../clusterman/tests/test_mgmt_commands.py    | 20 +++++++-
 cloudman/clusterman/views.py                  | 28 +++++------
 setup.py                                      |  5 +-
 7 files changed, 109 insertions(+), 28 deletions(-)

diff --git a/cloudman/clusterman/management/commands/create_autoscale_user.py b/cloudman/clusterman/management/commands/create_autoscale_user.py
index c2a45c84..0e736595 100644
--- a/cloudman/clusterman/management/commands/create_autoscale_user.py
+++ b/cloudman/clusterman/management/commands/create_autoscale_user.py
@@ -4,6 +4,8 @@
 from django.contrib.auth.models import User
 from django.core.management.base import BaseCommand
 
+from clusterman.models import GlobalSettings
+
 
 class Command(BaseCommand):
     help = 'Creates a user for managing autoscaling. This user has permissions to scale' \
@@ -16,12 +18,17 @@ def add_arguments(self, parser):
         parser.add_argument(
             '--password', required=False,
             help='Password for this user, autogenerated if not specified')
+        parser.add_argument(
+            '--impersonate_account', required=False,
+            help='User account to impersonate when scaling. This account is assumed to have stored'
+                 ' cloud credentials or IAM access. Defaults to the first super admin found.')
 
     def handle(self, *args, **options):
         username = options['username']
         password = options['password']
+        account = options['impersonate_account']
 
-        return self.create_autoscale_user(username, password)
+        return self.create_autoscale_user(username, password, account)
 
     @staticmethod
     def _add_permissions(user, perm_names):
@@ -31,7 +38,7 @@ def _add_permissions(user, perm_names):
         return user
 
     @staticmethod
-    def create_autoscale_user(username, password):
+    def create_autoscale_user(username, password, account):
         try:
             print("Creating autoscale user: {0}".format(username))
             user, created = User.objects.get_or_create(username=username)
@@ -41,6 +48,11 @@ def create_autoscale_user(username, password):
                     user, ['view_cmcluster', 'add_cmclusternode',
                            'delete_cmclusternode'])
                 user.save()
+                if account:
+                    impersonate_user = User.objects.get(username=account)
+                else:
+                    impersonate_user = User.objects.filter(is_superuser=True).first()
+                GlobalSettings().settings.autoscale_impersonate = impersonate_user.username
                 return "Autoscale user created successfully."
             else:
                 return "Autoscale user already exists."
diff --git a/cloudman/clusterman/migrations/0001_initial.py b/cloudman/clusterman/migrations/0001_initial.py
index cc265024..bf85d497 100644
--- a/cloudman/clusterman/migrations/0001_initial.py
+++ b/cloudman/clusterman/migrations/0001_initial.py
@@ -1,4 +1,4 @@
-# Generated by Django 2.2.10 on 2020-02-17 18:49
+# Generated by Django 2.2.10 on 2020-02-22 19:59
 
 from django.db import migrations, models
 import django.db.models.deletion
@@ -44,6 +44,14 @@ class Migration(migrations.Migration):
                 '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=[
diff --git a/cloudman/clusterman/models.py b/cloudman/clusterman/models.py
index b9ea56d7..600d29c4 100644
--- a/cloudman/clusterman/models.py
+++ b/cloudman/clusterman/models.py
@@ -1,10 +1,20 @@
 from django.db import models
 
+from hierarkey.models import GlobalSettingsBase, Hierarkey
+
 from cloudlaunch import models as cl_models
 from djcloudbridge import models as cb_models
 import yaml
 
 
+hierarkey = Hierarkey(attribute_name='settings')
+
+
+@hierarkey.set_global()
+class GlobalSettings(GlobalSettingsBase):
+    pass
+
+
 class CMCluster(models.Model):
     """CloudMan cluster details."""
     # Automatically add timestamps when object is created
diff --git a/cloudman/clusterman/tests/test_cluster_api.py b/cloudman/clusterman/tests/test_cluster_api.py
index 9516dc85..cb3324a4 100644
--- a/cloudman/clusterman/tests/test_cluster_api.py
+++ b/cloudman/clusterman/tests/test_cluster_api.py
@@ -196,6 +196,9 @@ class LiveServerSingleThreadedTestCase(APILiveServerTestCase):
 class CMClusterNodeTestBase(CMClusterServiceTestBase, LiveServerSingleThreadedTestCase):
 
     def setUp(self):
+        self.client.force_login(
+            User.objects.get_or_create(username='clusteradmin', is_superuser=True, is_staff=True)[0])
+
         cloudlaunch_url = f'{self.live_server_url}/cloudman/cloudlaunch/api/v1'
         patcher1 = patch('clusterman.api.CMServiceContext.cloudlaunch_url',
                              new_callable=PropertyMock,
@@ -553,8 +556,6 @@ def _deactivate_autoscaling(self, cluster_id):
         return self.client.put(url, cluster_data, format='json')
 
     def _create_cluster_node(self, cluster_id):
-        responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusterregistrationtoken',
-                      json={'nodeCommand': 'docker run rancher --worker'}, status=200)
         url = reverse('clusterman:node-list', args=[cluster_id])
         return self.client.post(url, self.NODE_DATA, format='json')
 
@@ -565,15 +566,11 @@ def _count_cluster_nodes(self, cluster_id):
         return response.data['count']
 
     def _signal_scaleup(self, cluster_id, data=SCALE_SIGNAL_DATA):
-        responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusterregistrationtoken',
-                      json={'nodeCommand': 'docker run rancher --worker'}, status=200)
         url = reverse('clusterman:scaleupsignal-list', args=[cluster_id])
         response = self.client.post(url, data, format='json')
         return response
 
     def _signal_scaledown(self, cluster_id, data=SCALE_SIGNAL_DATA):
-        responses.add(responses.POST, 'https://127.0.0.1:4430/v3/clusterregistrationtoken',
-                      json={'nodeCommand': 'docker run rancher --worker'}, status=200)
         url = reverse('clusterman:scaledownsignal-list', args=[cluster_id])
         response = self.client.post(url, data, format='json')
         return response
@@ -771,8 +768,12 @@ def test_scaling_within_zone_group(self):
         count = self._count_cluster_nodes(cluster_id)
         self.assertEqual(count, 1)
 
-    def _login_as_autoscaling_user(self):
-        call_command('create_autoscale_user', "--username", "autoscaletestuser")
+    def _login_as_autoscaling_user(self, impersonate_user=None):
+        if impersonate_user:
+            call_command('create_autoscale_user', "--impersonate_account",
+                         impersonate_user, "--username", "autoscaletestuser")
+        else:
+            call_command('create_autoscale_user', "--username", "autoscaletestuser")
         self.client.force_login(
             User.objects.get_or_create(username='autoscaletestuser')[0])
 
@@ -833,3 +834,34 @@ def test_autoscale_down_signal_unauthorized(self):
             User.objects.get_or_create(username='notaclusteradmin', is_staff=False)[0])
         response = self._signal_scaledown(cluster_id)
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN, response.data)
+
+    @responses.activate
+    def test_create_autoscale_user_impersonate(self):
+        # create a parent cluster
+        cluster_id = self._create_cluster()
+        self._login_as_autoscaling_user(impersonate_user='clusteradmin')
+        count = self._count_cluster_nodes(cluster_id)
+        self.assertEqual(count, 0)
+        response = self._signal_scaleup(cluster_id)
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.data)
+        count = self._count_cluster_nodes(cluster_id)
+        self.assertEqual(count, 1)
+
+    @responses.activate
+    def test_create_autoscale_user_impersonate_no_perms(self):
+        # create a parent cluster
+        cluster_id = self._create_cluster()
+        # create a non admin user
+        self.client.force_login(
+            User.objects.get_or_create(username='notaclusteradmin', is_staff=False)[0])
+        # log back in as admin
+        self.client.force_login(
+            User.objects.get(username='clusteradmin'))
+        # impersonate non admin user
+        self._login_as_autoscaling_user(impersonate_user='notaclusteradmin')
+        count = self._count_cluster_nodes(cluster_id)
+        self.assertEqual(count, 0)
+        response = self._signal_scaleup(cluster_id)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN, response.data)
+        count = self._count_cluster_nodes(cluster_id)
+        self.assertEqual(count, 0)
diff --git a/cloudman/clusterman/tests/test_mgmt_commands.py b/cloudman/clusterman/tests/test_mgmt_commands.py
index f90f143a..11203f8d 100644
--- a/cloudman/clusterman/tests/test_mgmt_commands.py
+++ b/cloudman/clusterman/tests/test_mgmt_commands.py
@@ -63,6 +63,10 @@ def test_create_cluster(self):
 
 class CreateAutoScaleUserCommandTestCase(TestCase):
 
+    def setUp(self):
+        self.client.force_login(
+            User.objects.get_or_create(username='admin', is_superuser=True)[0])
+
     def test_create_autoscale_user_no_args(self):
         call_command('create_autoscale_user')
         self.assertTrue(User.objects.get(username='autoscaleuser'))
@@ -84,9 +88,23 @@ def test_create_autoscale_user_existing(self):
                      "--password", "hello", stdout=out)
         self.assertIn("already exists", out.getvalue())
 
-    def test_create_autoscale_does_not_clobber_existing(self):
+    def test_create_autoscale_user_does_not_clobber_existing(self):
         User.objects.create_user(username="hello", password="world")
         call_command('create_autoscale_user', "--username", "hello",
                      "--password", "overwrite")
         # Password should remain unchanged
         self.assertTrue(self.client.login(username="hello", password="world"))
+
+    def test_create_autoscale_user_with_impersonate(self):
+        out = StringIO()
+        call_command('create_autoscale_user', "--username", "hello",
+                     "--password", "overwrite", "--impersonate_account", "admin",
+                     stdout=out)
+        self.assertIn("created successfully", out.getvalue())
+
+    def test_create_autoscale_user_with_non_existent_impersonate(self):
+        out = StringIO()
+        call_command('create_autoscale_user', "--username", "hello",
+                     "--password", "overwrite", "--impersonate_account", "non_existent",
+                     stdout=out)
+        self.assertNotIn("created successfully", out.getvalue())
diff --git a/cloudman/clusterman/views.py b/cloudman/clusterman/views.py
index 7942cf9d..7eda8f4d 100644
--- a/cloudman/clusterman/views.py
+++ b/cloudman/clusterman/views.py
@@ -9,7 +9,7 @@
 from . import serializers
 from .api import CloudManAPI
 from .api import CMServiceContext
-
+from .models import GlobalSettings
 
 
 class ClusterViewSet(drf_helpers.CustomModelViewSet):
@@ -99,15 +99,14 @@ def perform_create(self, serializer):
         # autoscale
         cmapi = CloudManAPI.from_request(self.request)
         cmapi.check_permissions('autoscalers.can_autoscale')
-        # If so, the remaining actions must be carried out as the admin user
-        # since we must use cloud credentials from the admin profile.
+        # If so, the remaining actions must be carried out as an impersonated user
+        # whose profile contains the relevant cloud credentials, usually an admin
         zone_name = serializer.validated_data.get(
             'commonLabels', {}).get('availability_zone')
-        # TODO: This is brittle because it assumes the first superuser
-        # found has cloud credentials. Instead, we could globally store
-        # which admin account the autoscale user should impersonate.
-        admin = User.objects.filter(is_superuser=True).first()
-        cmapi = CloudManAPI(CMServiceContext(user=admin))
+        impersonate = (User.objects.filter(
+            username=GlobalSettings().settings.autoscale_impersonate).first()
+                       or User.objects.filter(is_superuser=True).first())
+        cmapi = CloudManAPI(CMServiceContext(user=impersonate))
         cluster = cmapi.clusters.get(self.kwargs["cluster_pk"])
         if cluster:
             return cluster.scaleup(zone_name=zone_name)
@@ -129,15 +128,14 @@ def perform_create(self, serializer):
         # autoscale
         cmapi = CloudManAPI.from_request(self.request)
         cmapi.check_permissions('autoscalers.can_autoscale')
-        # If so, the remaining actions must be carried out as the admin user
-        # since we must use cloud credentials from the admin profile.
+        # If so, the remaining actions must be carried out as an impersonated user
+        # whose profile contains the relevant cloud credentials, usually an admin
         zone_name = serializer.validated_data.get(
             'commonLabels', {}).get('availability_zone')
-        # TODO: This is brittle because it assumes the first superuser
-        # found has cloud credentials. Instead, we could globally store
-        # which admin account the autoscale user should impersonate.
-        admin = User.objects.filter(is_superuser=True).first()
-        cmapi = CloudManAPI(CMServiceContext(user=admin))
+        impersonate = (User.objects.filter(
+            username=GlobalSettings().settings.autoscale_impersonate).first()
+                       or User.objects.filter(is_superuser=True).first())
+        cmapi = CloudManAPI(CMServiceContext(user=impersonate))
         cluster = cmapi.clusters.get(self.kwargs["cluster_pk"])
         if cluster:
             return cluster.scaledown(zone_name=zone_name)
diff --git a/setup.py b/setup.py
index 7624f88c..90e9676d 100755
--- a/setup.py
+++ b/setup.py
@@ -63,7 +63,10 @@ def get_version(*file_paths):
     'rules',
     # ======== CloudLaunch =========
     'cloudlaunch-server>=0.1.1',
-    'cloudlaunch-cli'
+    'cloudlaunch-cli',
+    # ===== CloudMan =====
+    # To store generic key-value pairs
+    'django-hierarkey'
 ]
 
 REQS_PROD = ([

From aa31fcc6aaf807cddae7e2152ab5927d86b889c2 Mon Sep 17 00:00:00 2001
From: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
Date: Sun, 23 Feb 2020 23:09:56 +0530
Subject: [PATCH 3/4] When deleting nodes, also drain and delete from rancher

---
 cloudman/clusterman/cluster_templates.py      |  3 +
 .../clusterman/migrations/0001_initial.py     | 83 -------------------
 .../plugins/rancher_kubernetes_app.py         | 26 +++++-
 cloudman/clusterman/rancher.py                | 77 ++++++++++++-----
 cloudman/clusterman/tests/test_cluster_api.py | 13 ++-
 5 files changed, 93 insertions(+), 109 deletions(-)
 delete mode 100644 cloudman/clusterman/migrations/0001_initial.py

diff --git a/cloudman/clusterman/cluster_templates.py b/cloudman/clusterman/cluster_templates.py
index 6a24212b..0637607e 100644
--- a/cloudman/clusterman/cluster_templates.py
+++ b/cloudman/clusterman/cluster_templates.py
@@ -117,7 +117,10 @@ def add_node(self, name, vm_type=None, zone=None):
             'config_app': {
                 'rancher_action': 'add_node',
                 'config_rancher_kube': {
+                    'rancher_url': self.rancher_url,
+                    'rancher_api_key': self.rancher_api_key,
                     'rancher_cluster_id': self.rancher_cluster_id,
+                    'rancher_project_id': self.rancher_project_id,
                     'rancher_node_command': (
                         self.rancher_client.get_cluster_registration_command()
                         + " --worker")
diff --git a/cloudman/clusterman/migrations/0001_initial.py b/cloudman/clusterman/migrations/0001_initial.py
deleted file mode 100644
index bf85d497..00000000
--- a/cloudman/clusterman/migrations/0001_initial.py
+++ /dev/null
@@ -1,83 +0,0 @@
-# Generated by Django 2.2.10 on 2020-02-22 19:59
-
-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)),
-                ('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/plugins/rancher_kubernetes_app.py b/cloudman/clusterman/plugins/rancher_kubernetes_app.py
index 7618f23d..f1e8bca8 100644
--- a/cloudman/clusterman/plugins/rancher_kubernetes_app.py
+++ b/cloudman/clusterman/plugins/rancher_kubernetes_app.py
@@ -1,10 +1,14 @@
 """Plugin implementation for a simple web application."""
+import time
+
 from celery.utils.log import get_task_logger
 
 from cloudlaunch.backend_plugins.base_vm_app import BaseVMAppPlugin
 from cloudlaunch.backend_plugins.cloudman2_app import get_iam_handler_for
 from cloudlaunch.configurers import AnsibleAppConfigurer
 
+from clusterman.rancher import RancherClient
+
 from rest_framework.serializers import ValidationError
 
 log = get_task_logger('cloudlaunch')
@@ -48,6 +52,12 @@ def deploy(self, name, task, app_config, provider_config, **kwargs):
             name, task, app_config, provider_config)
         return result
 
+    def _create_rancher_client(self, rancher_cfg):
+        return RancherClient(rancher_cfg.get('rancher_url'),
+                             rancher_cfg.get('rancher_api_key'),
+                             rancher_cfg.get('rancher_cluster_id'),
+                             rancher_cfg.get('rancher_project_id'))
+
     def delete(self, provider, deployment):
         """
         Delete resource(s) associated with the supplied deployment.
@@ -58,8 +68,20 @@ def delete(self, provider, deployment):
         *Note* that this method will delete resource(s) associated with
         the deployment - this is an un-recoverable action.
         """
-        # key += get_required_val(rancher_config, "RANCHER_API_KEY")
-        # Contact rancher API and delete node
+        app_config = deployment.get('app_config')
+        rancher_cfg = app_config.get('config_rancher_kube')
+        rancher_client = self._create_rancher_client(rancher_cfg)
+        node_ip = deployment.get(
+            'launch_result', {}).get('cloudLaunch', {}).get('publicIP')
+        rancher_node_id = rancher_client.find_node(ip=node_ip)
+        if rancher_node_id:
+            rancher_client.drain_node(rancher_node_id)
+            # during tests, node_ip is None, so skip sleep if so
+            if node_ip:
+                time.sleep(60)
+            # remove node from rancher
+            rancher_client.delete_node(rancher_node_id)
+        # delete the VM
         return super().delete(provider, deployment)
 
     def _get_configurer(self, app_config):
diff --git a/cloudman/clusterman/rancher.py b/cloudman/clusterman/rancher.py
index 9aa5ea87..4dfe65ec 100644
--- a/cloudman/clusterman/rancher.py
+++ b/cloudman/clusterman/rancher.py
@@ -1,4 +1,5 @@
 import requests
+from string import Template
 from requests.auth import AuthBase
 
 
@@ -17,11 +18,14 @@ def __call__(self, r):
 
 class RancherClient(object):
 
-    KUBE_CONFIG_URL = ("{rancher_url}/v3/clusters/{cluster_id}"
+    KUBE_CONFIG_URL = ("$rancher_url/v3/clusters/$cluster_id"
                        "?action=generateKubeconfig")
-    INSTALLED_APP_URL = ("{rancher_url}/v3/projects/{project_id}/app"
+    INSTALLED_APP_URL = ("$rancher_url/v3/projects/$project_id/app"
                          "?targetNamespace=galaxy-ns")
-    NODE_COMMAND_URL = "{rancher_url}/v3/clusterregistrationtoken"
+    NODE_COMMAND_URL = "$rancher_url/v3/clusterregistrationtoken"
+    NODE_LIST_URL = "$rancher_url/v3/nodes/?clusterId=$cluster_id"
+    NODE_DRAIN_URL = "$rancher_url/v3/nodes/$node_id?action=drain"
+    NODE_DELETE_URL = "$rancher_url/v3/nodes/$node_id"
 
     def __init__(self, rancher_url, api_key, cluster_id, project_id):
         self.rancher_url = rancher_url
@@ -29,32 +33,32 @@ def __init__(self, rancher_url, api_key, cluster_id, project_id):
         self.cluster_id = cluster_id
         self.project_id = project_id
 
-    def format_url(self, url):
-        return url.format(rancher_url=self.rancher_url,
-                          cluster_id=self.cluster_id,
-                          project_id=self.project_id)
+    def _format_url(self, url):
+        result = Template(url).safe_substitute({
+            'rancher_url': self.rancher_url,
+            'cluster_id': self.cluster_id,
+            'project_id': self.project_id
+        })
+        return result
 
-    def get_auth(self):
+    def _get_auth(self):
         return RancherAuth(self)
 
-    def _api_get(self, url):
-        return requests.get(self.format_url(url), auth=self.get_auth(),
-                            verify=False).json()
-
-    def _api_post(self, url, data):
-        return requests.post(self.format_url(url), auth=self.get_auth(),
-                             verify=False, json=data).json()
-
-    def _api_put(self, url, data):
-        return requests.put(self.format_url(url), auth=self.get_auth(),
+    def _api_get(self, url, data):
+        return requests.get(self._format_url(url), auth=self._get_auth(),
                             verify=False, json=data).json()
 
-    def list_installed_charts(self):
-        return self._api_get(self.INSTALLED_APP_URL).get('data')
+    def _api_post(self, url, data, json_response=True):
+        r = requests.post(self._format_url(url), auth=self._get_auth(),
+                          verify=False, json=data)
+        if json_response:
+            return r.json()
+        else:
+            return r
 
-    def update_installed_chart(self, data):
-        r = self._api_put(data.get('links').get('self'), data)
-        return r
+    def _api_delete(self, url, data):
+        return requests.delete(self._format_url(url), auth=self._get_auth(),
+                               verify=False, json=data).json()
 
     def fetch_kube_config(self):
         return self._api_post(self.KUBE_CONFIG_URL, data=None).get('config')
@@ -65,3 +69,30 @@ def get_cluster_registration_command(self):
             data={"type": "clusterRegistrationToken",
                   "clusterId": f"{self.cluster_id}"}
         ).get('nodeCommand')
+
+    def get_nodes(self):
+        return self._api_get(self.NODE_LIST_URL, data=None)
+
+    def find_node(self, ip):
+        matches = [n for n in self.get_nodes()['data']
+                   if n.get('ipAddress') == ip or
+                   n.get('externalIpAddress') == ip]
+        return matches[0]['id'] if matches else None
+
+    def drain_node(self, node_id):
+        node_url = Template(self.NODE_DRAIN_URL).safe_substitute({
+            'node_id': node_id
+        })
+        return self._api_post(node_url, data={
+            "deleteLocalData": True,
+            "force": True,
+            "ignoreDaemonSets": True,
+            "gracePeriod": "-1",
+            "timeout": "60"
+        }, json_response=False)
+
+    def delete_node(self, node_id):
+        node_url = Template(self.NODE_DELETE_URL).safe_substitute({
+            'node_id': node_id
+        })
+        return self._api_delete(node_url, data=None)
diff --git a/cloudman/clusterman/tests/test_cluster_api.py b/cloudman/clusterman/tests/test_cluster_api.py
index cb3324a4..5eff5234 100644
--- a/cloudman/clusterman/tests/test_cluster_api.py
+++ b/cloudman/clusterman/tests/test_cluster_api.py
@@ -226,7 +226,6 @@ 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)
-
         super().setUp()
 
 
@@ -267,6 +266,18 @@ def _check_cluster_node_exists(self, cluster_id, node_id):
         return response.data['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=[
+                          {'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)
         url = reverse('clusterman:node-detail', args=[cluster_id, node_id])
         return self.client.delete(url)
 

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 4/4] 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