Revert "Disabled field in compute host (#97)"

This reverts commit 55d79b4.
ChameleonCloud · Nov 20, 2023 · 182f816 · 182f816
1 parent 3ceda9f
commit 182f816
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 222 deletions.
diff --git a/blazar/db/migration/alembic_migrations/versions/d5a379ff6ba3_add_disabled_in_compute_host.py b/blazar/db/migration/alembic_migrations/versions/d5a379ff6ba3_add_disabled_in_compute_host.py
diff --git a/blazar/db/sqlalchemy/models.py b/blazar/db/sqlalchemy/models.py
@@ -282,8 +282,6 @@ class ComputeHost(mb.BlazarBase, mb.SoftDeleteMixinWithUuid):
     trust_id = sa.Column(sa.String(36), nullable=False)
     reservable = sa.Column(sa.Boolean, nullable=False,
                            server_default=sa.true())
-    disabled = sa.Column(sa.Boolean, nullable=False,
-                           server_default=sa.false())
     computehost_extra_capabilities = relationship('ComputeHostExtraCapability',
                                                   cascade="all,delete",
                                                   backref='computehost',

diff --git a/blazar/plugins/oshosts/host_plugin.py b/blazar/plugins/oshosts/host_plugin.py
@@ -20,7 +20,7 @@
 from oslo_config import cfg
 from oslo_utils import strutils
 
-from blazar import context, policy, exceptions
+from blazar import context, policy
 from blazar.db import api as db_api
 from blazar.db import exceptions as db_ex
 from blazar.db import utils as db_utils
@@ -343,9 +343,6 @@ def create_computehost(self, host_values):
             # Do not use nova's primary key for this host.
             # Instead, generate a new one.
             del host_details['id']
-        if 'disabled' in host_values:
-            self.handle_disabled_key(host_id, host_values['disabled'])
-            del host_values['disabled']
         # NOTE(sbauza): Only last duplicate name for same extra capability
         # will be stored
         to_store = set(host_values.keys()) - set(host_details.keys())
@@ -423,9 +420,7 @@ def update_computehost(self, host_id, values):
         # nothing to update
         if not values:
             return self.get_computehost(host_id)
-        if 'disabled' in values:
-            self.handle_disabled_key(host_id, values['disabled'])
-            del values['disabled']
+
         cant_update_extra_capability = []
         cant_delete_extra_capability = []
         previous_capabilities = self._get_extra_capabilities(host_id)
@@ -520,25 +515,6 @@ def delete_computehost(self, host_id):
             # they have to rerun
             raise manager_ex.CantDeleteHost(host=host_id, msg=str(e))
 
-    def handle_disabled_key(self, host_id, value):
-        # only admin can set/unset 'disabled' flag
-        if self._is_admin():
-            host_details = self.get_computehost(host_id)
-            # set True if value contains any string other than None
-            new_disabled_flag = False if value is None else True
-            self.set_disabled(host_details, new_disabled_flag)
-
-    def set_disabled(self, resource, is_disabled):
-        host_update_values = {"disabled": is_disabled}
-        # if a host is set as disabled, then it should not be reservable
-        if is_disabled:
-            host_update_values['reservable'] = False
-        db_api.host_update(resource["id"], host_update_values)
-        LOG.warn(
-            f"{resource['hypervisor_hostname']}",
-            f"is set disabled {is_disabled}"
-        )
-
     def list_allocations(self, query, detail=False):
         hosts_id_list = [h['id'] for h in db_api.host_list()]
         options = self.get_query_options(query, QUERY_TYPE_ALLOCATION)
@@ -659,13 +635,6 @@ def allocation_candidates(self, values):
 
         return host_ids
 
-    def _is_admin(self):
-        ctx = context.current()
-        if policy.enforce(ctx, 'admin', {}, do_raise=False):
-            return True
-        else:
-            return False
-
     def _matching_hosts(self, hypervisor_properties, resource_properties,
                         count_range, start_date, end_date, project_id):
         """Return the matching hosts (preferably not allocated)
@@ -689,12 +658,7 @@ def _matching_hosts(self, hypervisor_properties, resource_properties,
         if resource_properties:
             filter_array += plugins_utils.convert_requirements(
                 resource_properties)
-        # admin can create a lease for host with 'reservable' False
-        if self._is_admin():
-            hosts = db_api.host_get_all_by_queries(filter_array)
-        else:
-            hosts = db_api.reservable_host_get_all_by_queries(filter_array)
-        for host in hosts:
+        for host in db_api.reservable_host_get_all_by_queries(filter_array):
             if not self.is_project_allowed(project_id, resource_properties):
                 continue
             if not db_api.host_allocation_get_all_by_values(
@@ -933,16 +897,14 @@ def notification_callback(self, event_type, payload):
                     ['reservable == 0',
                      'hypervisor_hostname == ' + data['host']])
                 if recovered_hosts:
-                    self.set_reservable(recovered_hosts[0], True)
+                    db_api.host_update(recovered_hosts[0]['id'],
+                                       {'reservable': True})
                     LOG.warn('%s recovered.',
                              recovered_hosts[0]['hypervisor_hostname'])
 
         return reservation_flags
 
     def set_reservable(self, resource, is_reservable):
-        if resource.get('disabled', False):
-            LOG.debug(f"{resource['hypervisor_hostname']} is disabled - cannot set reservable")
-            return
         db_api.host_update(resource["id"], {"reservable": is_reservable})
         LOG.warn('%s %s.', resource["hypervisor_hostname"],
                  "recovered" if is_reservable else "failed")

diff --git a/blazar/tests/plugins/oshosts/test_physical_host_plugin.py b/blazar/tests/plugins/oshosts/test_physical_host_plugin.py
@@ -25,7 +25,7 @@
 from oslo_config import fixture as conf_fixture
 import testtools
 
-from blazar import context, policy
+from blazar import context
 from blazar import status
 from blazar.db import api as db_api
 from blazar.db import exceptions as db_exceptions
@@ -783,56 +783,6 @@ def test_create_reservation_hosts_available(self):
         ]
         host_allocation_create.assert_has_calls(calls)
 
-    def test_create_reservation_hosts_non_reservable(self):
-        values = {
-            'lease_id': '018c1b43-e69e-4aef-a543-09681539cf4c',
-            'min': 1,
-            'max': 1,
-            'hypervisor_properties': '["=", "$memory_mb", "256"]',
-            'resource_properties': '',
-            'start_date': datetime.datetime(2013, 12, 19, 20, 00),
-            'end_date': datetime.datetime(2013, 12, 19, 21, 00),
-            'resource_type': plugin.RESOURCE_TYPE,
-            'project_id': 'fake-project'
-        }
-        self.rp_create.return_value = mock.MagicMock(id=1)
-        host_reservation_create = self.patch(self.db_api,
-                                             'host_reservation_create')
-        matching_hosts = self.patch(self.fake_phys_plugin, '_matching_hosts')
-        matching_hosts.return_value = ['host1', 'host2']
-        host_allocation_create = self.patch(
-            self.db_api,
-            'host_allocation_create')
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
-        self.fake_phys_plugin.reserve_resource(
-            '441c1476-9f8f-4700-9f30-cd9b6fef3509',
-            values)
-        host_values = {
-            'reservation_id': '441c1476-9f8f-4700-9f30-cd9b6fef3509',
-            'aggregate_id': 1,
-            'resource_properties': '',
-            'hypervisor_properties': '["=", "$memory_mb", "256"]',
-            'count_range': '1-1',
-            'status': 'pending',
-            'before_end': 'default',
-            'on_start': 'default'
-        }
-        host_reservation_create.assert_called_once_with(host_values)
-        calls = [
-            mock.call(
-                {'compute_host_id': 'host1',
-                 'reservation_id': '441c1476-9f8f-4700-9f30-cd9b6fef3509',
-                 }),
-            mock.call(
-                {'compute_host_id': 'host2',
-                 'reservation_id': '441c1476-9f8f-4700-9f30-cd9b6fef3509',
-                 }),
-        ]
-        host_allocation_create.assert_has_calls(calls)
-
     @ddt.data({"params": {'max': 0}},
               {"params": {'max': -1}},
               {"params": {'max': 'one'}},
@@ -2406,10 +2356,6 @@ def host_allocation_get_all_by_values(**kwargs):
             (datetime.datetime(2013, 12, 19, 20, 00),
              datetime.datetime(2013, 12, 19, 21, 00)),
         ]
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
         result = self.fake_phys_plugin._matching_hosts(
             '[]', '[]', '1-3',
             datetime.datetime(2013, 12, 19, 20, 00),
@@ -2441,10 +2387,6 @@ def host_allocation_get_all_by_values(**kwargs):
             (datetime.datetime(2013, 12, 19, 20, 00),
              datetime.datetime(2013, 12, 19, 21, 00)),
         ]
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
         result = self.fake_phys_plugin._matching_hosts(
             '[]', '[]', '3-3',
             datetime.datetime(2013, 12, 19, 20, 00),
@@ -2479,10 +2421,6 @@ def host_allocation_get_all_by_values(**kwargs):
              datetime.datetime(2013, 12, 19, 21, 00)
              + datetime.timedelta(minutes=5))
         ]
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
         result = self.fake_phys_plugin._matching_hosts(
             '[]', '[]', '3-3',
             datetime.datetime(2013, 12, 19, 20, 00),
@@ -2496,10 +2434,6 @@ def test_matching_hosts_not_matching(self):
             self.db_api,
             'reservable_host_get_all_by_queries')
         host_get.return_value = []
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
         result = self.fake_phys_plugin._matching_hosts(
             '["=", "$memory_mb", "2048"]', '[]', '1-1',
             datetime.datetime(2013, 12, 19, 20, 00),
@@ -2611,31 +2545,6 @@ def test_update_resource_property(self):
         db_resource_property_update.assert_called_once_with(
             'physical:host', 'foo', resource_property_values)
 
-    def test_update_disabled_property_as_admin(self):
-        resource_property_values = {'disabled': 'true'}
-        host_get = self.patch(self.db_api, 'host_get')
-        host_get.return_value = {'id': 'host1', 'hypervisor_hostname': 'host1'}
-        host_update = self.patch(self.db_api, 'host_update')
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = True
-        self.fake_phys_plugin.update_computehost(
-            'host1', {'disabled': 'true'})
-        host_update.assert_called_once_with('host1', {"disabled": True, "reservable": False})
-
-    def test_update_disabled_property_as_user(self):
-        resource_property_values = {'disabled': 'true'}
-        host_get = self.patch(self.db_api, 'host_get')
-        host_get.return_value = {'id': 'host1', 'hypervisor_hostname': 'host1'}
-        host_update = self.patch(self.db_api, 'host_update')
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = False
-        self.fake_phys_plugin.update_computehost(
-            'host1', {'disabled': 'true'})
-        self.assertFalse(host_update.called)
 
 class PhysicalHostMonitorPluginTestCase(tests.TestCase):
 
@@ -2647,7 +2556,7 @@ def setUp(self):
         self.host_monitor_plugin = host_plugin.PhysicalHostMonitorPlugin()
 
     def test_notification_callback_disabled_true(self):
-        failed_host = {'hypervisor_hostname': 'hypvsr1', 'id': '1', 'disabled': False}
+        failed_host = {'hypervisor_hostname': 'hypvsr1', 'id': '1'}
         event_type = 'service.update'
         payload = {
             'nova_object.namespace': 'nova',
@@ -2676,6 +2585,7 @@ def test_notification_callback_disabled_true(self):
                                                                 payload)
         host_get_all.assert_called_once_with(
             ['hypervisor_hostname == ' + payload['nova_object.data']['host']])
+        self.assertEqual({}, result)
 
     def test_notification_callback_no_failure(self):
         event_type = 'service.update'
@@ -2741,42 +2651,6 @@ def test_notification_callback_recover(self):
                                             {'reservable': True})
         self.assertEqual({}, result)
 
-    def test_notification_callback_not_recover_with_blazar_disabled(self):
-        recovered_host = {'hypervisor_hostname': 'hypvsr1', 'id': 1, 'disabled': True}
-        event_type = 'service.update'
-        payload = {
-            'nova_object.namespace': 'nova',
-            'nova_object.name': 'ServiceStatusPayload',
-            'nova_object.version': '1.1',
-            'nova_object.data': {
-                'host': 'compute-1',
-                'disabled': False,
-                'last_seen_up': '2012-10-29T13:42:05Z',
-                'binary': 'nova-compute',
-                'topic': 'compute',
-                'disabled_reason': None,
-                'report_count': 1,
-                'forced_down': False,
-                'version': 22,
-                'availability_zone': None,
-                'uuid': 'fa69c544-906b-4a6a-a9c6-c1f7a8078c73'
-            }
-        }
-        host_get_all = self.patch(db_api, 'host_get_all_by_queries')
-        host_get_all.return_value = [recovered_host]
-        host_update = self.patch(db_api, 'host_update')
-        is_admin = self.patch(
-            policy, 'enforce'
-        )
-        is_admin.return_value = True
-        result = self.host_monitor_plugin.notification_callback(event_type,
-                                                                payload)
-        host_get_all.assert_called_once_with(
-            ['reservable == 0',
-             'hypervisor_hostname == ' + payload['nova_object.data']['host']])
-        self.assertFalse(host_update.called)
-        self.assertEqual({}, result)
-
     def test_poll_resource_failures_state_down(self):
         hosts = [
             {'id': '1',

diff --git a/setup.cfg b/setup.cfg
@@ -46,7 +46,7 @@ blazar.resource.plugins =
 blazar.device.driver.plugins =
     k8s.plugin=blazar.plugins.devices.k8s_plugin:K8sPlugin
     zun.plugin=blazar.plugins.devices.zun_plugin:ZunPlugin
-
+    
 blazar.network.usage.type.plugins =
     storage.plugin=blazar.plugins.networks.storage_plugin:StoragePlugin