diff --git a/doc/source/userguide.rst b/doc/source/userguide.rst index a02be9c240..fb15fd5adc 100644 --- a/doc/source/userguide.rst +++ b/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 \ 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 \ 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 \ + 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 \ The number of servers that will serve as node in the cluster. The default is 1. diff --git a/magnum/api/attr_validator.py b/magnum/api/attr_validator.py index bcd7999d0b..537bf76c5d 100644 --- a/magnum/api/attr_validator.py +++ b/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} diff --git a/magnum/api/controllers/v1/bay.py b/magnum/api/controllers/v1/bay.py index a7ac8c0941..a8faecd486 100644 --- a/magnum/api/controllers/v1/bay.py +++ b/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 diff --git a/magnum/api/controllers/v1/cluster.py b/magnum/api/controllers/v1/cluster.py index a4fe83f6a9..a7d9c9f232 100644 --- a/magnum/api/controllers/v1/cluster.py +++ b/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()) diff --git a/magnum/api/controllers/v1/cluster_template.py b/magnum/api/controllers/v1/cluster_template.py index 37d760a08d..4a4d6b9aa8 100644 --- a/magnum/api/controllers/v1/cluster_template.py +++ b/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) diff --git a/magnum/db/sqlalchemy/alembic/versions/bc46ba6cf949_add_keypair_to_cluster.py b/magnum/db/sqlalchemy/alembic/versions/bc46ba6cf949_add_keypair_to_cluster.py new file mode 100644 index 0000000000..d40c265fc5 --- /dev/null +++ b/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)) diff --git a/magnum/db/sqlalchemy/models.py b/magnum/db/sqlalchemy/models.py index e80956e9dc..ec6a72a6b2 100644 --- a/magnum/db/sqlalchemy/models.py +++ b/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) diff --git a/magnum/drivers/common/template_def.py b/magnum/drivers/common/template_def.py index c5c1357fc8..5129c349ff 100644 --- a/magnum/drivers/common/template_def.py +++ b/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') diff --git a/magnum/objects/cluster.py b/magnum/objects/cluster.py index 33c4d9f8bb..20975acd78 100644 --- a/magnum/objects/cluster.py +++ b/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), diff --git a/magnum/tests/functional/api/v1/test_baymodel.py b/magnum/tests/functional/api/v1/test_baymodel.py index b2f732a546..5dfaef9942 100644 --- a/magnum/tests/functional/api/v1/test_baymodel.py +++ b/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 diff --git a/magnum/tests/functional/api/v1/test_cluster_template.py b/magnum/tests/functional/api/v1/test_cluster_template.py index 79b5a922a7..29b8a441f6 100644 --- a/magnum/tests/functional/api/v1/test_cluster_template.py +++ b/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): diff --git a/magnum/tests/functional/common/datagen.py b/magnum/tests/functional/common/datagen.py index c92cbf51ae..c987cfb1a5 100644 --- a/magnum/tests/functional/common/datagen.py +++ b/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, diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster.py b/magnum/tests/unit/api/controllers/v1/test_cluster.py index 6ec7968364..9eb84188c1 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster.py +++ b/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): diff --git a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py index 27963996e2..277e640578 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py +++ b/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, diff --git a/magnum/tests/unit/api/test_attr_validator.py b/magnum/tests/unit/api/test_attr_validator.py index 007ea3f3d4..04d7778f33 100644 --- a/magnum/tests/unit/api/test_attr_validator.py +++ b/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) diff --git a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py index 00d9084333..03ac12464b 100644 --- a/magnum/tests/unit/conductor/handlers/test_k8s_cluster_conductor.py +++ b/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', diff --git a/magnum/tests/unit/conductor/handlers/test_mesos_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_mesos_cluster_conductor.py index 0d452a5da6..4faa72b20b 100644 --- a/magnum/tests/unit/conductor/handlers/test_mesos_cluster_conductor.py +++ b/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', diff --git a/magnum/tests/unit/conductor/handlers/test_swarm_cluster_conductor.py b/magnum/tests/unit/conductor/handlers/test_swarm_cluster_conductor.py index 7d1455184a..68f22d5c40 100644 --- a/magnum/tests/unit/conductor/handlers/test_swarm_cluster_conductor.py +++ b/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', diff --git a/magnum/tests/unit/objects/test_cluster.py b/magnum/tests/unit/objects/test_cluster.py index 0b62700e76..c13b8bd2ff 100644 --- a/magnum/tests/unit/objects/test_cluster.py +++ b/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): diff --git a/magnum/tests/unit/objects/test_objects.py b/magnum/tests/unit/objects/test_objects.py index 5d689ec311..cea5e81e96 100644 --- a/magnum/tests/unit/objects/test_objects.py +++ b/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', diff --git a/releasenotes/notes/bp-keypair-override-on-create-ca8f12ffca41cd62.yaml b/releasenotes/notes/bp-keypair-override-on-create-ca8f12ffca41cd62.yaml new file mode 100644 index 0000000000..ba3c392012 --- /dev/null +++ b/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.