From 68bc87876f6479fe49b6a13a4c032eefe0ff016f Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 1 Sep 2020 09:53:10 +0100 Subject: [PATCH] virt: Remove 'reset_network' API This one is tied into an admin action in the server actions API, which means we must remove that API action also. Otherwise, this isn't too crazy. Change-Id: I58343b94b67915062d044fa0f53aeab01b77738f Signed-off-by: Stephen Finucane --- api-ref/source/servers-admin-action.inc | 13 ++++--- nova/api/openstack/compute/admin_actions.py | 14 ++----- nova/compute/api.py | 5 --- nova/compute/manager.py | 6 +-- nova/compute/rpcapi.py | 7 ---- nova/policies/admin_actions.py | 11 ------ .../admin-actions-reset-network.json.tpl | 3 -- .../api_sample_tests/test_admin_actions.py | 6 +-- .../openstack/compute/test_admin_actions.py | 16 ++++---- nova/tests/unit/compute/test_compute.py | 39 ++----------------- nova/tests/unit/compute/test_compute_mgr.py | 21 ---------- nova/tests/unit/compute/test_rpcapi.py | 4 -- nova/tests/unit/fake_policy.py | 1 - .../tests/unit/policies/test_admin_actions.py | 8 ---- nova/tests/unit/test_policy.py | 1 - nova/virt/driver.py | 7 ---- ...remove-xenapi-driver-194756049f22dc9e.yaml | 2 + 17 files changed, 28 insertions(+), 136 deletions(-) delete mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-admin-actions/admin-actions-reset-network.json.tpl diff --git a/api-ref/source/servers-admin-action.inc b/api-ref/source/servers-admin-action.inc index bc3f898caa53..03a40d38ce57 100644 --- a/api-ref/source/servers-admin-action.inc +++ b/api-ref/source/servers-admin-action.inc @@ -170,16 +170,19 @@ Response If successful, this method does not return content in the response body. -Reset Networking On A Server (resetNetwork Action) -================================================== +Reset Networking On A Server (resetNetwork Action) (DEPRECATED) +=============================================================== .. rest_method:: POST /servers/{server_id}/action Resets networking on a server. -.. note:: +.. warning:: - No longer supported by any in-tree virt driver. + This action was only supported by the XenAPI virt driver, which was + deprecated in the 20.0.0 (Train) release and removed in the 22.0.0 + (Victoria) release. This action should be avoided in new applications. It + was removed in the 23.0.0 (Wallaby) release. Specify the ``resetNetwork`` action in the request body. @@ -190,7 +193,7 @@ through the ``policy.json`` file. Normal response codes: 202 Error response codes: unauthorized(401), forbidden(403), itemNotFound(404), -conflict(409) +conflict(409), gone(410) Request ------- diff --git a/nova/api/openstack/compute/admin_actions.py b/nova/api/openstack/compute/admin_actions.py index 6de6956bcfde..2b0b182a9d55 100644 --- a/nova/api/openstack/compute/admin_actions.py +++ b/nova/api/openstack/compute/admin_actions.py @@ -34,19 +34,11 @@ class AdminActionsController(wsgi.Controller): super(AdminActionsController, self).__init__() self.compute_api = compute.API() - @wsgi.response(202) - @wsgi.expected_errors((404, 409)) + @wsgi.expected_errors(410) @wsgi.action('resetNetwork') def _reset_network(self, req, id, body): - """Permit admins to reset networking on a server.""" - context = req.environ['nova.context'] - instance = common.get_instance(self.compute_api, context, id) - context.can(aa_policies.POLICY_ROOT % 'reset_network', - target={'project_id': instance.project_id}) - try: - self.compute_api.reset_network(context, instance) - except exception.InstanceIsLocked as e: - raise exc.HTTPConflict(explanation=e.format_message()) + """(Removed) Permit admins to reset networking on a server.""" + raise exc.HTTPGone() @wsgi.response(202) @wsgi.expected_errors((404, 409)) diff --git a/nova/compute/api.py b/nova/compute/api.py index f51190c015a2..74e85e0da8d5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4530,11 +4530,6 @@ class API(base.Base): action=fields_obj.NotificationAction.UNLOCK, source=fields_obj.NotificationSource.API) - @check_instance_lock - def reset_network(self, context, instance): - """Reset networking on the instance.""" - self.compute_rpcapi.reset_network(context, instance=instance) - @check_instance_lock def inject_network_info(self, context, instance): """Inject network info for the instance.""" diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 94b0a7591bcd..500a4bd03686 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -6075,7 +6075,6 @@ class ComputeManager(manager.Manager): instance, network_id) self._inject_network_info(instance, network_info) - self.reset_network(context, instance) # NOTE(russellb) We just want to bump updated_at. See bug 1143466. instance.updated_at = timeutils.utcnow() @@ -6098,7 +6097,6 @@ class ComputeManager(manager.Manager): instance, address) self._inject_network_info(instance, network_info) - self.reset_network(context, instance) # NOTE(russellb) We just want to bump updated_at. See bug 1143466. instance.updated_at = timeutils.utcnow() @@ -6629,11 +6627,9 @@ class ComputeManager(manager.Manager): # TODO(stephenfin): Remove this in RPC 6.0 since it's nova-network only @messaging.expected_exceptions(NotImplementedError) - @wrap_instance_fault def reset_network(self, context, instance): """Reset networking on the given instance.""" - LOG.debug('Reset network', instance=instance) - self.driver.reset_network(instance) + raise NotImplementedError() def _inject_network_info(self, instance, network_info): """Inject network info for the given instance.""" diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index dabdd015fe30..3558c9f1477a 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -1124,13 +1124,6 @@ class ComputeAPI(object): server=_compute_host(None, instance), version=version) cctxt.cast(ctxt, 'rescue_instance', **msg_args) - # Remove as it only supports nova network - def reset_network(self, ctxt, instance): - version = '5.0' - cctxt = self.router.client(ctxt).prepare( - server=_compute_host(None, instance), version=version) - cctxt.cast(ctxt, 'reset_network', instance=instance) - def resize_instance(self, ctxt, instance, migration, image, instance_type, request_spec, clean_shutdown=True): msg_args = {'instance': instance, 'migration': migration, diff --git a/nova/policies/admin_actions.py b/nova/policies/admin_actions.py index bab6bd6452f7..b0e6b40c2107 100644 --- a/nova/policies/admin_actions.py +++ b/nova/policies/admin_actions.py @@ -44,17 +44,6 @@ admin_actions_policies = [ } ], scope_types=['system', 'project']), - policy.DocumentedRuleDefault( - name=POLICY_ROOT % 'reset_network', - check_str=base.SYSTEM_ADMIN, - description="Reset networking on a server", - operations=[ - { - 'method': 'POST', - 'path': '/servers/{server_id}/action (resetNetwork)' - } - ], - scope_types=['system', 'project']) ] diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-admin-actions/admin-actions-reset-network.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-admin-actions/admin-actions-reset-network.json.tpl deleted file mode 100644 index 7c79cb68a514..000000000000 --- a/nova/tests/functional/api_sample_tests/api_samples/os-admin-actions/admin-actions-reset-network.json.tpl +++ /dev/null @@ -1,3 +0,0 @@ -{ - "resetNetwork": null -} diff --git a/nova/tests/functional/api_sample_tests/test_admin_actions.py b/nova/tests/functional/api_sample_tests/test_admin_actions.py index fd35b96a195e..602305da99f3 100644 --- a/nova/tests/functional/api_sample_tests/test_admin_actions.py +++ b/nova/tests/functional/api_sample_tests/test_admin_actions.py @@ -31,9 +31,9 @@ class AdminActionsSamplesJsonTest(test_servers.ServersSampleBase): def test_post_reset_network(self): # Get api samples to reset server network request. - response = self._do_post('servers/%s/action' % self.uuid, - 'admin-actions-reset-network', {}) - self.assertEqual(202, response.status_code) + self.api.api_post( + 'servers/%s/action' % self.uuid, {'resetNetwork': None}, + check_response_status=[410]) def test_post_inject_network_info(self): # Get api samples to inject network info request. diff --git a/nova/tests/unit/api/openstack/compute/test_admin_actions.py b/nova/tests/unit/api/openstack/compute/test_admin_actions.py index 77640a5a0b70..60292ae3e569 100644 --- a/nova/tests/unit/api/openstack/compute/test_admin_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_admin_actions.py @@ -29,20 +29,18 @@ class AdminActionsTestV21(admin_only_action_common.CommonTests): lambda *a, **k: self.controller) def test_actions(self): - actions = ['_reset_network', '_inject_network_info'] - method_translations = {'_reset_network': 'reset_network', - '_inject_network_info': 'inject_network_info'} + actions = ['_inject_network_info'] + method_translations = {'_inject_network_info': 'inject_network_info'} self._test_actions(actions, method_translations) def test_actions_with_non_existed_instance(self): - actions = ['_reset_network', '_inject_network_info'] + actions = ['_inject_network_info'] self._test_actions_with_non_existed_instance(actions) def test_actions_with_locked_instance(self): - actions = ['_reset_network', '_inject_network_info'] - method_translations = {'_reset_network': 'reset_network', - '_inject_network_info': 'inject_network_info'} + actions = ['_inject_network_info'] + method_translations = {'_inject_network_info': 'inject_network_info'} - self._test_actions_with_locked_instance(actions, - method_translations=method_translations) + self._test_actions_with_locked_instance( + actions, method_translations=method_translations) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 82e0c91a2c8c..cd188e75458c 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -3360,26 +3360,6 @@ class ComputeTestCase(BaseTestCase, self.assertTrue(called['inject']) self.compute.terminate_instance(self.context, instance, []) - def test_reset_network(self): - # Ensure we can reset networking on an instance. - called = {'count': 0} - - def fake_driver_reset_network(self, instance): - called['count'] += 1 - - self.stub_out('nova.virt.fake.FakeDriver.reset_network', - fake_driver_reset_network) - - instance = self._create_fake_instance_obj() - self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, - block_device_mapping=[]) - - self.compute.reset_network(self.context, instance) - - self.assertEqual(called['count'], 1) - - self.compute.terminate_instance(self.context, instance, []) - def _get_snapshotting_instance(self): # Ensure instance can be snapshotted. instance = self._create_fake_instance_obj() @@ -4250,10 +4230,8 @@ class ComputeTestCase(BaseTestCase, def dummy(*args, **kwargs): pass - self.stub_out('nova.compute.manager.ComputeManager.' - 'inject_network_info', dummy) - self.stub_out('nova.compute.manager.ComputeManager.' - 'reset_network', dummy) + self.stub_out( + 'nova.compute.manager.ComputeManager.inject_network_info', dummy) instance = self._create_fake_instance_obj() @@ -4270,10 +4248,8 @@ class ComputeTestCase(BaseTestCase, def dummy(*args, **kwargs): pass - self.stub_out('nova.compute.manager.ComputeManager.' - 'inject_network_info', dummy) - self.stub_out('nova.compute.manager.ComputeManager.' - 'reset_network', dummy) + self.stub_out( + 'nova.compute.manager.ComputeManager.inject_network_info', dummy) instance = self._create_fake_instance_obj() @@ -11213,13 +11189,6 @@ class ComputeAPITestCase(BaseTestCase): instance = self.compute_api.get(self.context, instance['uuid']) self.compute_api.inject_network_info(self.context, instance) - def test_reset_network(self): - instance = self._create_fake_instance_obj() - self.compute.build_and_run_instance(self.context, - instance, {}, {}, {}, block_device_mapping=[]) - instance = self.compute_api.get(self.context, instance['uuid']) - self.compute_api.reset_network(self.context, instance) - @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.context.RequestContext.elevated') @mock.patch('nova.compute.api.API._record_action_start') diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index ca8d42d63090..4c30ef8c13eb 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -5136,27 +5136,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, do_test() - def test_reset_network_driver_not_implemented(self): - instance = fake_instance.fake_instance_obj(self.context) - - @mock.patch.object(self.compute.driver, 'reset_network', - side_effect=NotImplementedError()) - @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') - def do_test(mock_add_fault, mock_reset): - self.assertRaises(messaging.ExpectedException, - self.compute.reset_network, - self.context, - instance) - - self.compute = utils.ExceptionHelper(self.compute) - - self.assertRaises(NotImplementedError, - self.compute.reset_network, - self.context, - instance) - - do_test() - @mock.patch.object(manager.ComputeManager, '_set_migration_status') @mock.patch.object(manager.ComputeManager, '_do_rebuild_instance_with_claim') diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index 5e8d45293c02..6beee6a5be24 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -776,10 +776,6 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): rescue_image_ref='fake_image_ref', clean_shutdown=True, version='5.0') - def test_reset_network(self): - self._test_compute_api('reset_network', 'cast', - instance=self.fake_instance_obj) - def test_resize_instance(self): self._test_compute_api('resize_instance', 'cast', instance=self.fake_instance_obj, migration={'id': 'fake_id'}, diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 9f19d29ef0e5..bfc90e119e21 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -51,7 +51,6 @@ policy_data = """ "os_compute_api:servers:migrations:show": "", "os_compute_api:servers:migrations:delete": "", "os_compute_api:os-admin-actions:inject_network_info": "", - "os_compute_api:os-admin-actions:reset_network": "", "os_compute_api:os-admin-actions:reset_state": "", "os_compute_api:os-admin-password": "", "os_compute_api:os-aggregates:set_metadata": "", diff --git a/nova/tests/unit/policies/test_admin_actions.py b/nova/tests/unit/policies/test_admin_actions.py index 70abc4e203e4..c5522616ffb8 100644 --- a/nova/tests/unit/policies/test_admin_actions.py +++ b/nova/tests/unit/policies/test_admin_actions.py @@ -75,14 +75,6 @@ class AdminActionsPolicyTest(base.BasePolicyTest): self.controller._inject_network_info, self.req, self.instance.uuid, body={}) - def test_reset_network_policy(self): - rule_name = "os_compute_api:os-admin-actions:reset_network" - with mock.patch.object(self.controller.compute_api, "reset_network"): - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, - rule_name, self.controller._reset_network, - self.req, self.instance.uuid, body={}) - class AdminActionsScopeTypePolicyTest(AdminActionsPolicyTest): """Test Admin Actions APIs policies with system scope enabled. diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index aae8e6346f06..5e7751e6ca1d 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -326,7 +326,6 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:show:host_status:unknown-only", "os_compute_api:servers:migrations:force_complete", "os_compute_api:servers:migrations:delete", -"os_compute_api:os-admin-actions:reset_network", "os_compute_api:os-admin-actions:inject_network_info", "os_compute_api:os-admin-actions:reset_state", "os_compute_api:os-aggregates:index", diff --git a/nova/virt/driver.py b/nova/virt/driver.py index c7e10d7fadb9..c9d969a1ebc5 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -1347,13 +1347,6 @@ class ComputeDriver(object): """ raise NotImplementedError() - # TODO(stephenfin): This was only implemented (properly) for XenAPI and - # should be removed. - def reset_network(self, instance): - """reset networking for specified instance.""" - # TODO(Vek): Need to pass context in for access to auth_token - pass - def set_admin_password(self, instance, new_pass): """Set the root password on the specified instance. diff --git a/releasenotes/notes/remove-xenapi-driver-194756049f22dc9e.yaml b/releasenotes/notes/remove-xenapi-driver-194756049f22dc9e.yaml index a09b29b24d28..efdc9de164e5 100644 --- a/releasenotes/notes/remove-xenapi-driver-194756049f22dc9e.yaml +++ b/releasenotes/notes/remove-xenapi-driver-194756049f22dc9e.yaml @@ -12,6 +12,7 @@ upgrade: * ``POST /os-agents`` * ``PUT /os-agents/{agent_id}`` * ``DELETE /os-agents/{agent_id}`` + * ``POST /servers/{server_id}/action (resetNetwork)`` The ``XenAPI`` specific policies have been removed: @@ -20,6 +21,7 @@ upgrade: * ``os_compute_api:os-agents:create`` * ``os_compute_api:os-agents:update`` * ``os_compute_api:os-agents:delete`` + * ``os_compute_api:os-admin-actions:reset_network`` The ``XenAPI`` specific configuration options have been removed.