From 8a30ad3462521df0c21321691c163552896a14b2 Mon Sep 17 00:00:00 2001 From: Jake Yip Date: Wed, 31 Jan 2024 18:37:57 +1100 Subject: [PATCH] Add feature to specify driver explicitly Allow ClusterTemplate to explicitly specify a driver to use for creating Clusters. This is initially sourced from the image property 'magnum_driver', but may be improved to be specified via client in the future. Falls back to old driver discovery using (coe, server_type, os) tuple to keep existing behaviour. Change-Id: I9e206b589951a02360d3cef0282a9538236ef53b --- magnum/api/controllers/v1/cluster_template.py | 6 ++++ magnum/api/validation.py | 7 ++-- magnum/common/exception.py | 4 +++ .../conductor/handlers/cluster_conductor.py | 12 ++++--- ...32afc4fd_add_driver_to_cluster_template.py | 34 +++++++++++++++++++ magnum/db/sqlalchemy/models.py | 1 + magnum/drivers/common/driver.py | 16 +++++++-- magnum/objects/cluster_template.py | 4 ++- .../controllers/v1/test_cluster_template.py | 12 +++++++ magnum/tests/unit/db/utils.py | 1 + magnum/tests/unit/objects/test_objects.py | 2 +- ...ove-driver-discovery-df61e03c8749a34d.yaml | 14 ++++++++ 12 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 magnum/db/sqlalchemy/alembic/versions/c0f832afc4fd_add_driver_to_cluster_template.py create mode 100644 releasenotes/notes/improve-driver-discovery-df61e03c8749a34d.yaml diff --git a/magnum/api/controllers/v1/cluster_template.py b/magnum/api/controllers/v1/cluster_template.py index 463463e95c..dbd08e4dd0 100644 --- a/magnum/api/controllers/v1/cluster_template.py +++ b/magnum/api/controllers/v1/cluster_template.py @@ -158,6 +158,9 @@ class ClusterTemplate(base.APIBase): tags = wtypes.StringType(min_length=0, max_length=255) """A comma separated list of tags.""" + driver = wtypes.StringType(min_length=0, max_length=255) + """Driver name set explicitly""" + def __init__(self, **kwargs): self.fields = [] for field in objects.ClusterTemplate.fields: @@ -413,6 +416,9 @@ class ClusterTemplatesController(base.Controller): cluster_template_dict['cluster_distro'] = image_data['os_distro'] cluster_template_dict['project_id'] = context.project_id cluster_template_dict['user_id'] = context.user_id + # NOTE(jake): read driver from image for now, update client to provide + # this as param in the future + cluster_template_dict['driver'] = image_data.get('magnum_driver') # check permissions for making cluster_template public or hidden if cluster_template_dict['public'] or cluster_template_dict['hidden']: if not policy.enforce(context, "clustertemplate:publish", None, diff --git a/magnum/api/validation.py b/magnum/api/validation.py index 96493bbdbe..9d695750e9 100644 --- a/magnum/api/validation.py +++ b/magnum/api/validation.py @@ -68,7 +68,8 @@ def enforce_driver_supported(): def wrapper(func, *args, **kwargs): cluster_template = args[1] cluster_distro = cluster_template.cluster_distro - if not cluster_distro: + driver_name = cluster_template.driver + if not cluster_distro or not driver_name: try: cli = clients.OpenStackClients(pecan.request.context) image_id = cluster_template.image_id @@ -76,11 +77,13 @@ def enforce_driver_supported(): image_id, 'images') cluster_distro = image.get('os_distro') + driver_name = image.get('magnum_driver') except Exception: pass cluster_type = (cluster_template.server_type, cluster_distro, - cluster_template.coe) + cluster_template.coe, + driver_name) driver.Driver.get_driver(*cluster_type) return func(*args, **kwargs) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 52aa5f9561..952f73d80b 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -267,6 +267,10 @@ class ClusterTypeNotSupported(NotSupported): " not supported.") +class ClusterDriverNotSupported(NotSupported): + message = _("Cluster driver (%(driver_name)s) not supported.") + + class RequiredParameterNotProvided(Invalid): message = _("Required parameter %(heat_param)s not provided.") diff --git a/magnum/conductor/handlers/cluster_conductor.py b/magnum/conductor/handlers/cluster_conductor.py index 36e0501151..8a2f44a637 100644 --- a/magnum/conductor/handlers/cluster_conductor.py +++ b/magnum/conductor/handlers/cluster_conductor.py @@ -146,7 +146,8 @@ class Handler(object): ct = conductor_utils.retrieve_cluster_template(context, cluster) cluster_driver = driver.Driver.get_driver(ct.server_type, ct.cluster_distro, - ct.coe) + ct.coe, + ct.driver) # Update cluster try: conductor_utils.notify_about_cluster_operation( @@ -182,7 +183,8 @@ class Handler(object): ct = conductor_utils.retrieve_cluster_template(context, cluster) cluster_driver = driver.Driver.get_driver(ct.server_type, ct.cluster_distro, - ct.coe) + ct.coe, + ct.driver) try: conductor_utils.notify_about_cluster_operation( context, taxonomy.ACTION_DELETE, taxonomy.OUTCOME_PENDING, @@ -263,7 +265,8 @@ class Handler(object): ct = conductor_utils.retrieve_cluster_template(context, cluster) cluster_driver = driver.Driver.get_driver(ct.server_type, ct.cluster_distro, - ct.coe) + ct.coe, + ct.driver) # Backup the old node count so that we can restore it # in case of an exception. old_node_count = nodegroup.node_count @@ -327,7 +330,8 @@ class Handler(object): ct = conductor_utils.retrieve_cluster_template(context, cluster) cluster_driver = driver.Driver.get_driver(ct.server_type, ct.cluster_distro, - ct.coe) + ct.coe, + ct.driver) # Upgrade cluster try: diff --git a/magnum/db/sqlalchemy/alembic/versions/c0f832afc4fd_add_driver_to_cluster_template.py b/magnum/db/sqlalchemy/alembic/versions/c0f832afc4fd_add_driver_to_cluster_template.py new file mode 100644 index 0000000000..750fa1d5af --- /dev/null +++ b/magnum/db/sqlalchemy/alembic/versions/c0f832afc4fd_add_driver_to_cluster_template.py @@ -0,0 +1,34 @@ +# +# 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 driver to cluster_template + +Revision ID: c0f832afc4fd +Revises: 7da8489d6a68 +Create Date: 2024-01-29 13:18:15.181043 + +""" + +# revision identifiers, used by Alembic. +revision = 'c0f832afc4fd' +down_revision = '7da8489d6a68' + +from alembic import op # noqa: E402 +import sqlalchemy as sa # noqa: E402 + + +def upgrade(): + op.add_column('cluster_template', + sa.Column('driver', sa.String(length=255), + nullable=True)) + # ### end Alembic commands ### diff --git a/magnum/db/sqlalchemy/models.py b/magnum/db/sqlalchemy/models.py index fa31dfa853..0b7ae94189 100644 --- a/magnum/db/sqlalchemy/models.py +++ b/magnum/db/sqlalchemy/models.py @@ -191,6 +191,7 @@ class ClusterTemplate(Base): floating_ip_enabled = Column(Boolean, default=True) hidden = Column(Boolean, default=False) tags = Column(String(255)) + driver = Column(String(255)) class X509KeyPair(Base): diff --git a/magnum/drivers/common/driver.py b/magnum/drivers/common/driver.py index b636d0ddcb..b1d6b11591 100644 --- a/magnum/drivers/common/driver.py +++ b/magnum/drivers/common/driver.py @@ -17,6 +17,7 @@ import abc import importlib_metadata as metadata from oslo_config import cfg from stevedore import driver +from stevedore import exception as stevedore_exception from magnum.common import exception from magnum.objects import cluster_template @@ -84,7 +85,7 @@ class Driver(object, metaclass=abc.ABCMeta): return cls.definitions @classmethod - def get_driver(cls, server_type, os, coe): + def get_driver(cls, server_type, os, coe, driver_name=None): """Get Driver. Returns the Driver class for the provided cluster_type. @@ -121,6 +122,16 @@ class Driver(object, metaclass=abc.ABCMeta): definition_map = cls.get_drivers() cluster_type = (server_type, os, coe) + # if driver_name is specified, use that + if driver_name: + try: + found = driver.DriverManager("magnum.drivers", + driver_name).driver() + return found + except stevedore_exception.NoMatches: + raise exception.ClusterDriverNotSupported( + driver_name=driver_name) + if cluster_type not in definition_map: raise exception.ClusterTypeNotSupported( server_type=server_type, @@ -137,7 +148,8 @@ class Driver(object, metaclass=abc.ABCMeta): def get_driver_for_cluster(cls, context, cluster): ct = cluster_template.ClusterTemplate.get_by_uuid( context, cluster.cluster_template_id) - return cls.get_driver(ct.server_type, ct.cluster_distro, ct.coe) + return cls.get_driver(ct.server_type, ct.cluster_distro, ct.coe, + ct.driver) def update_cluster_status(self, context, cluster, use_admin_ctx=False): """Update the cluster status based on underlying orchestration diff --git a/magnum/objects/cluster_template.py b/magnum/objects/cluster_template.py index d352feb6dd..15a1d3a33a 100644 --- a/magnum/objects/cluster_template.py +++ b/magnum/objects/cluster_template.py @@ -43,7 +43,8 @@ class ClusterTemplate(base.MagnumPersistentObject, base.MagnumObject, # Version 1.18: DockerStorageDriver is a StringField (was an Enum) # Version 1.19: Added 'hidden' field # Version 1.20: Added 'tags' field - VERSION = '1.20' + # Version 1.21: Added 'driver' field + VERSION = '1.21' dbapi = dbapi.get_instance() @@ -81,6 +82,7 @@ class ClusterTemplate(base.MagnumPersistentObject, base.MagnumObject, 'floating_ip_enabled': fields.BooleanField(default=True), 'hidden': fields.BooleanField(default=False), 'tags': fields.StringField(nullable=True), + 'driver': fields.StringField(nullable=True), } @staticmethod 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 219f5d4985..11d70ada59 100644 --- a/magnum/tests/unit/api/controllers/v1/test_cluster_template.py +++ b/magnum/tests/unit/api/controllers/v1/test_cluster_template.py @@ -1177,6 +1177,18 @@ class TestPost(api_base.FunctionalTest): response.json['created_at']).replace(tzinfo=None) self.assertEqual(test_time, return_created_at) + @mock.patch('magnum.api.attr_validator.validate_image') + def test_create_cluster_template_with_driver_name(self, mock_image_data): + mock_image = {'name': 'mock_name', + 'os_distro': 'fedora-atomic', + 'magnum_driver': 'mock_driver'} + mock_image_data.return_value = mock_image + bdict = apiutils.cluster_template_post_data() + resp = self.post_json('/clustertemplates', bdict) + self.assertEqual(201, resp.status_int) + self.assertEqual(resp.json['driver'], + mock_image.get('magnum_driver')) + class TestDelete(api_base.FunctionalTest): diff --git a/magnum/tests/unit/db/utils.py b/magnum/tests/unit/db/utils.py index 017ecf2ac7..7152b376b2 100644 --- a/magnum/tests/unit/db/utils.py +++ b/magnum/tests/unit/db/utils.py @@ -58,6 +58,7 @@ def get_test_cluster_template(**kw): 'floating_ip_enabled': kw.get('floating_ip_enabled', True), 'hidden': kw.get('hidden', False), 'tags': kw.get('tags', ""), + 'driver': kw.get('driver', ""), } diff --git a/magnum/tests/unit/objects/test_objects.py b/magnum/tests/unit/objects/test_objects.py index acda3750e8..3cebe0f550 100644 --- a/magnum/tests/unit/objects/test_objects.py +++ b/magnum/tests/unit/objects/test_objects.py @@ -356,7 +356,7 @@ class TestObject(test_base.TestCase, _TestObject): # https://docs.openstack.org/magnum/latest/contributor/objects.html object_data = { 'Cluster': '1.23-dfaf9ecb65a5fcab4f6c36497a8bc866', - 'ClusterTemplate': '1.20-a9334881d1dc6e077faec68214fa9d1d', + 'ClusterTemplate': '1.21-2d23d472f415b5e7571603a8689898e3', 'Certificate': '1.2-64f24db0e10ad4cbd72aea21d2075a80', 'MyObj': '1.0-34c4b1aadefd177b13f9a2f894cc23cd', 'X509KeyPair': '1.2-d81950af36c59a71365e33ce539d24f9', diff --git a/releasenotes/notes/improve-driver-discovery-df61e03c8749a34d.yaml b/releasenotes/notes/improve-driver-discovery-df61e03c8749a34d.yaml new file mode 100644 index 0000000000..4760f4d598 --- /dev/null +++ b/releasenotes/notes/improve-driver-discovery-df61e03c8749a34d.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + Add a feature to prevent drivers clashing when multiple drivers are able to + provide the same functionality. + + Drivers used to be selected based on a tuple of (server_type, os, coe). This + can be a problem if multiple drivers provides the same functionality, e.g. a + tuple like (vm, ubuntu, kubernetes). + + To allow for this, it is now possible to explicitly specify a driver name, + instead of relying on the lookup. The driver name is the same as the + entrypoint name, and can be specified by a Cluster Template through the + Glance image property "magnum_driver".