Allow keypair to be added during cluster create

Keypair can now be added during Cluster create command. This allows
Clusters to be created from the same ClusterTemplate but have
different keypair values when created. If not specified on create
then the value from the ClusterTemplate will be used. Adds keypair_id
to Cluster object and uses of keypair_id will use the value from Cluster
instead of ClusterTemplate.

Added release note for new features in magnum and magnum CLI.

Change-Id: I177a5aa06f881156944a9f74c9ccc3cd2abac492
Implements: blueprint keypair-override-on-create

doc/source/userguide.rst

@@ -101,13 +101,15 @@ They are loosely grouped as: mandatory, infrastructure, COE specific.
   Mesos      Ubuntu
   ========== =====================
 
-This is a mandatory parameter and there is no default value.
+  This is a mandatory parameter and there is no default value.
 
 --keypair-id \<keypair-id\>
   The name or UUID of the SSH keypair to configure in the cluster servers
   for ssh access.  You will need the key to be able to ssh to the
   servers in the cluster.  The login name is specific to the cluster
-  driver.  This is a mandatory parameter and there is no default value.
+  driver. If keypair is not provided in template it will be required at
+  Cluster create. This value will be overridden by any keypair value that
+  is provided during Cluster create.
 
 --external-network-id \<external-network-id\>
   The name or network ID of a Neutron network to provide connectivity
@@ -427,6 +429,15 @@ follows:
   name will be generated using a string and a number, for example
   "gamma-7-cluster".
 
+--keypair \<keypair\>
+  The name or UUID of the SSH keypair to configure in the cluster servers
+  for ssh access.  You will need the key to be able to ssh to the
+  servers in the cluster.  The login name is specific to the cluster
+  driver. If keypair is not provided it will attempt to use the value in
+  the ClusterTemplate. If the ClusterTemplate is also missing a keypair value
+  then an error will be returned.  The keypair value provided here will
+  override the keypair value from the ClusterTemplate.
+
 --node-count \<node-count\>
   The number of servers that will serve as node in the cluster.
   The default is 1.

magnum/api/attr_validator.py

@@ -166,7 +166,7 @@ def validate_labels_executor_env_variables(labels):
         raise exception.InvalidParameterValue(err)
 
 
-def validate_os_resources(context, cluster_template):
+def validate_os_resources(context, cluster_template, cluster=None):
     """Validate ClusterTemplate's OpenStack Resources"""
 
     cli = clients.OpenStackClients(context)
@@ -178,6 +178,9 @@ def validate_os_resources(context, cluster_template):
             else:
                 validate_method(cluster_template[attr])
 
+    if cluster:
+        validate_keypair(cli, cluster['keypair'])
+
 
 def validate_master_count(cluster, cluster_template):
     if cluster['master_count'] > 1 and \
@@ -190,7 +193,6 @@ def validate_master_count(cluster, cluster_template):
 validators = {'image_id': validate_image,
               'flavor_id': validate_flavor,
               'master_flavor_id': validate_flavor,
-              'keypair_id': validate_keypair,
               'external_network_id': validate_external_network,
               'fixed_network': validate_fixed_network,
               'labels': validate_labels}

magnum/api/controllers/v1/bay.py

@@ -415,9 +415,13 @@ class BaysController(base.Controller):
                        action='bay:create')
         baymodel = objects.ClusterTemplate.get_by_uuid(context,
                                                        bay.baymodel_id)
-        attr_validator.validate_os_resources(context, baymodel.as_dict())
-        attr_validator.validate_master_count(bay.as_dict(), baymodel.as_dict())
+
         bay_dict = bay.as_dict()
+        bay_dict['keypair'] = baymodel.keypair_id
+        attr_validator.validate_os_resources(context, baymodel.as_dict(),
+                                             bay_dict)
+        attr_validator.validate_master_count(bay.as_dict(), baymodel.as_dict())
+
         bay_dict['project_id'] = context.project_id
         bay_dict['user_id'] = context.user_id
         # NOTE(yuywz): We will generate a random human-readable name for
@@ -426,7 +430,6 @@ class BaysController(base.Controller):
         bay_dict['name'] = name
         bay_dict['coe_version'] = None
         bay_dict['container_version'] = None
-
         new_bay = objects.Cluster(context, **bay_dict)
         new_bay.uuid = uuid.uuid4()
         return new_bay

magnum/api/controllers/v1/cluster.py

@@ -95,6 +95,10 @@ class Cluster(base.APIBase):
                                           mandatory=True)
     """The cluster_template UUID"""
 
+    keypair = wsme.wsattr(wtypes.StringType(min_length=1, max_length=255),
+                          default=None)
+    """The name or id of the nova ssh keypair"""
+
     node_count = wsme.wsattr(wtypes.IntegerType(minimum=1), default=1)
     """The node count for this cluster. Default to 1 if not set"""
 
@@ -152,7 +156,7 @@ class Cluster(base.APIBase):
     def _convert_with_links(cluster, url, expand=True):
         if not expand:
             cluster.unset_fields_except(['uuid', 'name', 'cluster_template_id',
-                                         'node_count', 'status',
+                                         'keypair', 'node_count', 'status',
                                          'create_timeout', 'master_count',
                                          'stack_id'])
 
@@ -174,6 +178,7 @@ class Cluster(base.APIBase):
         sample = cls(uuid='27e3153e-d5bf-4b7e-b517-fb518e17f34c',
                      name='example',
                      cluster_template_id=temp_id,
+                     keypair=None,
                      node_count=2,
                      master_count=1,
                      create_timeout=15,
@@ -360,10 +365,15 @@ class ClustersController(base.Controller):
         temp_id = cluster.cluster_template_id
         cluster_template = objects.ClusterTemplate.get_by_uuid(context,
                                                                temp_id)
+        # If keypair not present, use cluster_template value
+        if cluster.keypair is None:
+            cluster.keypair = cluster_template.keypair_id
+
         cluster_dict = cluster.as_dict()
 
         attr_validator.validate_os_resources(context,
-                                             cluster_template.as_dict())
+                                             cluster_template.as_dict(),
+                                             cluster_dict)
         attr_validator.validate_master_count(cluster_dict,
                                              cluster_template.as_dict())
 

magnum/api/controllers/v1/cluster_template.py

@@ -64,7 +64,7 @@ class ClusterTemplate(base.APIBase):
     """The DNS nameserver address"""
 
     keypair_id = wsme.wsattr(wtypes.StringType(min_length=1, max_length=255),
-                             mandatory=True)
+                             default=None)
     """The name or id of the nova ssh keypair"""
 
     external_network_id = wtypes.StringType(min_length=1, max_length=255)

magnum/db/sqlalchemy/alembic/versions/bc46ba6cf949_add_keypair_to_cluster.py

@@ -0,0 +1,32 @@
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""add keypair to cluster
+
+Revision ID: bc46ba6cf949
+Revises: 720f640f43d1
+Create Date: 2016-10-03 10:47:08.584635
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'bc46ba6cf949'
+down_revision = '720f640f43d1'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.add_column('cluster', sa.Column('keypair', sa.String(length=255),
+                                       nullable=True))

magnum/db/sqlalchemy/models.py

@@ -115,6 +115,7 @@ class Cluster(Base):
     uuid = Column(String(36))
     name = Column(String(255))
     cluster_template_id = Column(String(255))
+    keypair = Column(String(255))
     stack_id = Column(String(255))
     api_address = Column(String(255))
     node_addresses = Column(JSONEncodedList)

magnum/drivers/common/template_def.py

@@ -329,7 +329,7 @@ class BaseTemplateDefinition(TemplateDefinition):
         self._osc = None
 
         self.add_parameter('ssh_key_name',
-                           cluster_template_attr='keypair_id',
+                           cluster_attr='keypair',
                            required=True)
         self.add_parameter('server_image',
                            cluster_template_attr='image_id')

magnum/objects/cluster.py

@@ -41,8 +41,9 @@ class Cluster(base.MagnumPersistentObject, base.MagnumObject,
     # Version 1.9: Rename table name from 'bay' to 'cluster'
     #              Rename 'baymodel_id' to 'cluster_template_id'
     #              Rename 'bay_create_timeout' to 'create_timeout'
+    # Version 1.10: Added 'keypair' field
 
-    VERSION = '1.9'
+    VERSION = '1.10'
 
     dbapi = dbapi.get_instance()
 
@@ -53,6 +54,7 @@ class Cluster(base.MagnumPersistentObject, base.MagnumObject,
         'project_id': fields.StringField(nullable=True),
         'user_id': fields.StringField(nullable=True),
         'cluster_template_id': fields.StringField(nullable=True),
+        'keypair': fields.StringField(nullable=True),
         'stack_id': fields.StringField(nullable=True),
         'status': m_fields.ClusterStatusField(nullable=True),
         'status_reason': fields.StringField(nullable=True),

magnum/tests/functional/api/v1/test_baymodel.py

@@ -185,13 +185,6 @@ class BayModelTest(base.BaseTempestTest):
             exceptions.BadRequest,
             self.baymodel_client.post_baymodel, gen_model)
 
-    @testtools.testcase.attr('negative')
-    def test_create_baymodel_missing_keypair(self):
-        gen_model = datagen.baymodel_data_with_missing_keypair()
-        self.assertRaises(
-            exceptions.NotFound,
-            self.baymodel_client.post_baymodel, gen_model)
-
     @testtools.testcase.attr('negative')
     def test_update_baymodel_invalid_patch(self):
         # get json object

magnum/tests/functional/api/v1/test_cluster_template.py

@@ -200,13 +200,11 @@ class ClusterTemplateTest(base.BaseTempestTest):
             exceptions.BadRequest,
             self.cluster_template_client.post_cluster_template, gen_model)
 
-    @testtools.testcase.attr('negative')
+    @testtools.testcase.attr('positive')
     def test_create_cluster_template_missing_keypair(self):
         gen_model = \
             datagen.cluster_template_data_with_missing_keypair()
-        self.assertRaises(
-            exceptions.NotFound,
-            self.cluster_template_client.post_cluster_template, gen_model)
+        resp, model = self._create_cluster_template(gen_model)
 
     @testtools.testcase.attr('negative')
     def test_update_cluster_template_invalid_patch(self):

magnum/tests/functional/common/datagen.py

@@ -502,7 +502,6 @@ def valid_swarm_cluster_template(is_public=False):
                                  public=is_public,
                                  dns_nameserver=config.Config.dns_nameserver,
                                  master_flavor_id=master_flavor_id,
-                                 keypair_id=config.Config.keypair_id,
                                  coe="swarm", docker_volume_size=3,
                                  cluster_distro=None,
                                  external_network_id=config.Config.nic_id,
@@ -535,6 +534,7 @@ def cluster_data(name=data_utils.rand_name('cluster'),
     data = {
         "name": name,
         "cluster_template_id": cluster_template_id,
+        "keypair": config.Config.keypair_id,
         "node_count": node_count,
         "discovery_url": None,
         "create_timeout": create_timeout,

magnum/tests/unit/api/controllers/v1/test_cluster.py

@@ -741,6 +741,24 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(400, response.status_int)
 
+    def test_create_cluster_with_keypair(self):
+        bdict = apiutils.cluster_post_data()
+        bdict['keypair'] = 'keypair2'
+        response = self.post_json('/clusters', bdict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        cluster, timeout = self.mock_cluster_create.call_args
+        self.assertEqual('keypair2', cluster[0].keypair)
+
+    def test_create_cluster_without_keypair(self):
+        bdict = apiutils.cluster_post_data()
+        response = self.post_json('/clusters', bdict)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(202, response.status_int)
+        cluster, timeout = self.mock_cluster_create.call_args
+        # Verify keypair from ClusterTemplate is used
+        self.assertEqual('keypair1', cluster[0].keypair)
+
 
 class TestDelete(api_base.FunctionalTest):
     def setUp(self):

magnum/tests/unit/api/controllers/v1/test_cluster_template.py

@@ -442,7 +442,7 @@ class TestPatch(api_base.FunctionalTest):
         self.assertTrue(response.json['errors'])
 
     def test_remove_mandatory_property_fail(self):
-        mandatory_properties = ('/image_id', '/keypair_id', '/coe',
+        mandatory_properties = ('/image_id', '/coe',
                                 '/external_network_id', '/server_type',
                                 '/tls_disabled', '/public',
                                 '/registry_enabled',
@@ -860,12 +860,15 @@ class TestPost(api_base.FunctionalTest):
                                   expect_errors=True)
         self.assertEqual(400, response.status_int)
 
-    def test_create_cluster_template_without_keypair_id(self):
+    @mock.patch('magnum.api.attr_validator.validate_image')
+    def test_create_cluster_template_without_keypair_id(self,
+                                                        mock_image_data):
+        mock_image_data.return_value = {'name': 'mock_name',
+                                        'os_distro': 'fedora-atomic'}
         bdict = apiutils.cluster_template_post_data()
         del bdict['keypair_id']
-        response = self.post_json('/clustertemplates', bdict,
-                                  expect_errors=True)
-        self.assertEqual(400, response.status_int)
+        response = self.post_json('/clustertemplates', bdict)
+        self.assertEqual(201, response.status_int)
 
     @mock.patch('magnum.api.attr_validator.validate_image')
     def test_create_cluster_template_with_dns(self,

magnum/tests/unit/api/test_attr_validator.py

@@ -297,3 +297,18 @@ class TestAttrValidator(base.BaseTestCase):
         mock_context = mock.MagicMock()
         attr_validator.validate_os_resources(mock_context,
                                              mock_cluster_template)
+
+    @mock.patch('magnum.common.clients.OpenStackClients')
+    def test_validate_os_resources_with_cluster(self, mock_os_cli):
+        mock_cluster_template = {}
+        mock_cluster = {'keypair': 'test-keypair'}
+        mock_keypair = mock.MagicMock()
+        mock_keypair.id = 'test-keypair'
+        mock_nova = mock.MagicMock()
+        mock_nova.keypairs.get.return_value = mock_keypair
+        mock_os_cli = mock.MagicMock()
+        mock_os_cli.nova.return_value = mock_nova
+        mock_context = mock.MagicMock()
+        attr_validator.validate_os_resources(mock_context,
+                                             mock_cluster_template,
+                                             mock_cluster)

magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py

@@ -56,6 +56,7 @@ class TestClusterConductorWithK8s(base.TestCase):
         self.cluster_dict = {
             'uuid': '5d12f6fd-a196-4bf0-ae4c-1f639a523a52',
             'cluster_template_id': 'xx-xx-xx-xx',
+            'keypair': 'keypair_id',
             'name': 'cluster1',
             'stack_id': 'xx-xx-xx-xx',
             'api_address': '172.17.2.3',

magnum/tests/unit/conductor/handlers/test_mesos_cluster_conductor.py

@@ -52,6 +52,7 @@ class TestClusterConductorWithMesos(base.TestCase):
             'id': 1,
             'uuid': '5d12f6fd-a196-4bf0-ae4c-1f639a523a52',
             'cluster_template_id': 'xx-xx-xx-xx',
+            'keypair': 'keypair_id',
             'name': 'cluster1',
             'stack_id': 'xx-xx-xx-xx',
             'api_address': '172.17.2.3',

magnum/tests/unit/conductor/handlers/test_swarm_cluster_conductor.py

@@ -55,6 +55,7 @@ class TestClusterConductorWithSwarm(base.TestCase):
             'id': 1,
             'uuid': '5d12f6fd-a196-4bf0-ae4c-1f639a523a52',
             'cluster_template_id': 'xx-xx-xx-xx',
+            'keypair': 'keypair_id',
             'name': 'cluster1',
             'stack_id': 'xx-xx-xx-xx',
             'api_address': '172.17.2.3',

magnum/tests/unit/objects/test_cluster.py

@@ -37,6 +37,7 @@ class TestClusterObject(base.DbTestCase):
         cluster_template_id = self.fake_cluster['cluster_template_id']
         self.fake_cluster_template = objects.ClusterTemplate(
             uuid=cluster_template_id)
+        self.fake_cluster['keypair'] = 'keypair1'
 
     @mock.patch('magnum.objects.ClusterTemplate.get_by_uuid')
     def test_get_by_id(self, mock_cluster_template_get):

magnum/tests/unit/objects/test_objects.py

@@ -362,7 +362,7 @@ class TestObject(test_base.TestCase, _TestObject):
 # For more information on object version testing, read
 # http://docs.openstack.org/developer/magnum/objects.html
 object_data = {
-    'Cluster': '1.9-f9838e23eef5f1a7d9606c1ccce21800',
+    'Cluster': '1.10-377082b6d7895cd800a39fa004765538',
     'ClusterTemplate': '1.17-65a95ef932dd08800a83871eb3cf312b',
     'Certificate': '1.1-1924dc077daa844f0f9076332ef96815',
     'MyObj': '1.0-b43567e512438205e32f4e95ca616697',

releasenotes/notes/bp-keypair-override-on-create-ca8f12ffca41cd62.yaml

@@ -0,0 +1,17 @@
+---
+prelude: >
+    Magnum's keypair-override-on-create blueprint [1]
+    allows for optional keypair value in ClusterTemplates
+    and the ability to specify a keypair value during
+    cluster creation.
+features:
+  - Added parameter in cluster-create to specify the
+    keypair. If keypair is not provided, the default
+    value from the matching ClusterTemplate will be used.
+  - Keypair is now optional for ClusterTemplate, in order
+    to allow Clusters to use keypairs separate from their
+    parent ClusterTemplate.
+deprecations:
+  - --keypair-id parameter in magnum CLI
+    cluster-template-create has been renamed to
+    --keypair.