From 1aed004a50760d095ce659c1df67c1bf86c53a88 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Tue, 13 Dec 2016 18:11:29 +0300 Subject: [PATCH] Rollback cluster/bay on update failure Rollback mechanism was added in Magnum in patch https://review.openstack.org/343478 but currently there is no support on the client side. This patch add rollback support for Magnum cluster/bay, user can enable rollback on cluster update failure by specifying option '--rollback' when executing 'magnum cluster-update', for example: magnum cluster-update XXX replace node_count=5 --rollback The code is based on outdated patch https://review.openstack.org/347586 and improves it: * added rollback support for clusters * added API version check in CLI (should be >= 1.3) * added several unit tests Change-Id: Iaae51e084e8ffd69be38512eec8c4c54b158b55e Co-authored-by: Wenzhi Yu Implements: blueprint bay-rollback-on-update-failure --- magnumclient/tests/v1/test_bays.py | 18 ++++++++++++++++++ magnumclient/tests/v1/test_bays_shell.py | 19 +++++++++++++++++-- magnumclient/tests/v1/test_clusters.py | 20 ++++++++++++++++++++ magnumclient/tests/v1/test_clusters_shell.py | 19 +++++++++++++++++-- magnumclient/v1/baseunit.py | 7 +++++-- magnumclient/v1/bays_shell.py | 10 +++++++++- magnumclient/v1/clusters_shell.py | 10 +++++++++- 7 files changed, 95 insertions(+), 8 deletions(-) diff --git a/magnumclient/tests/v1/test_bays.py b/magnumclient/tests/v1/test_bays.py index 480d0e2..f8c2b66 100644 --- a/magnumclient/tests/v1/test_bays.py +++ b/magnumclient/tests/v1/test_bays.py @@ -81,6 +81,13 @@ fake_responses = { UPDATED_BAY, ), }, + '/v1/bays/%s/?rollback=True' % BAY1['id']: + { + 'PATCH': ( + {}, + UPDATED_BAY, + ), + }, '/v1/bays/%s' % BAY1['name']: { 'GET': ( @@ -297,3 +304,14 @@ class BayManagerTest(testtools.TestCase): ] self.assertEqual(expect, self.api.calls) self.assertEqual(NEW_NAME, bay.name) + + def test_bay_update_with_rollback(self): + patch = {'op': 'replace', + 'value': NEW_NAME, + 'path': '/name'} + bay = self.mgr.update(id=BAY1['id'], patch=patch, rollback=True) + expect = [ + ('PATCH', '/v1/bays/%s/?rollback=True' % BAY1['id'], {}, patch), + ] + self.assertEqual(expect, self.api.calls) + self.assertEqual(NEW_NAME, bay.name) diff --git a/magnumclient/tests/v1/test_bays_shell.py b/magnumclient/tests/v1/test_bays_shell.py index 0fc5e44..3581651 100644 --- a/magnumclient/tests/v1/test_bays_shell.py +++ b/magnumclient/tests/v1/test_bays_shell.py @@ -245,14 +245,29 @@ class ShellTest(shell_test_base.TestCommandLineArgument): def test_bay_update_success(self, mock_update): self._test_arg_success('bay-update test add test=test') patch = [{'op': 'add', 'path': '/test', 'value': 'test'}] - mock_update.assert_called_once_with('test', patch) + mock_update.assert_called_once_with('test', patch, False) @mock.patch('magnumclient.v1.bays.BayManager.update') def test_bay_update_success_many_attribute(self, mock_update): self._test_arg_success('bay-update test add test=test test1=test1') patch = [{'op': 'add', 'path': '/test', 'value': 'test'}, {'op': 'add', 'path': '/test1', 'value': 'test1'}] - mock_update.assert_called_once_with('test', patch) + mock_update.assert_called_once_with('test', patch, False) + + @mock.patch('magnumclient.v1.bays.BayManager.update') + def test_bay_update_success_rollback(self, mock_update): + self._test_arg_success('bay-update test add test=test --rollback') + patch = [{'op': 'add', 'path': '/test', 'value': 'test'}] + mock_update.assert_called_once_with('test', patch, True) + + @mock.patch('magnumclient.v1.bays.BayManager.update') + def test_bay_update_rollback_old_api_version(self, mock_update): + self.assertRaises( + exceptions.CommandError, + self.shell, + '--magnum-api-version 1.2 bay-update ' + 'test add test=test --rollback') + mock_update.assert_not_called() @mock.patch('magnumclient.v1.bays.BayManager.update') def test_bay_update_failure_wrong_op(self, mock_update): diff --git a/magnumclient/tests/v1/test_clusters.py b/magnumclient/tests/v1/test_clusters.py index e1e7f25..0c59937 100644 --- a/magnumclient/tests/v1/test_clusters.py +++ b/magnumclient/tests/v1/test_clusters.py @@ -81,6 +81,13 @@ fake_responses = { UPDATED_CLUSTER, ), }, + '/v1/clusters/%s/?rollback=True' % CLUSTER1['id']: + { + 'PATCH': ( + {}, + UPDATED_CLUSTER, + ), + }, '/v1/clusters/%s' % CLUSTER1['name']: { 'GET': ( @@ -313,3 +320,16 @@ class ClusterManagerTest(testtools.TestCase): ] self.assertEqual(expect, self.api.calls) self.assertEqual(NEW_NAME, cluster.name) + + def test_cluster_update_with_rollback(self): + patch = {'op': 'replace', + 'value': NEW_NAME, + 'path': '/name'} + cluster = self.mgr.update(id=CLUSTER1['id'], patch=patch, + rollback=True) + expect = [ + ('PATCH', '/v1/clusters/%s/?rollback=True' % CLUSTER1['id'], + {}, patch), + ] + self.assertEqual(expect, self.api.calls) + self.assertEqual(NEW_NAME, cluster.name) diff --git a/magnumclient/tests/v1/test_clusters_shell.py b/magnumclient/tests/v1/test_clusters_shell.py index 41e4e37..a55a1e2 100644 --- a/magnumclient/tests/v1/test_clusters_shell.py +++ b/magnumclient/tests/v1/test_clusters_shell.py @@ -295,14 +295,29 @@ class ShellTest(shell_test_base.TestCommandLineArgument): def test_cluster_update_success(self, mock_update): self._test_arg_success('cluster-update test add test=test') patch = [{'op': 'add', 'path': '/test', 'value': 'test'}] - mock_update.assert_called_once_with('test', patch) + mock_update.assert_called_once_with('test', patch, False) @mock.patch('magnumclient.v1.clusters.ClusterManager.update') def test_cluster_update_success_many_attribute(self, mock_update): self._test_arg_success('cluster-update test add test=test test1=test1') patch = [{'op': 'add', 'path': '/test', 'value': 'test'}, {'op': 'add', 'path': '/test1', 'value': 'test1'}] - mock_update.assert_called_once_with('test', patch) + mock_update.assert_called_once_with('test', patch, False) + + @mock.patch('magnumclient.v1.clusters.ClusterManager.update') + def test_cluster_update_success_rollback(self, mock_update): + self._test_arg_success('cluster-update test add test=test --rollback') + patch = [{'op': 'add', 'path': '/test', 'value': 'test'}] + mock_update.assert_called_once_with('test', patch, True) + + @mock.patch('magnumclient.v1.clusters.ClusterManager.update') + def test_cluster_update_rollback_old_api_version(self, mock_update): + self.assertRaises( + exceptions.CommandError, + self.shell, + '--magnum-api-version 1.2 cluster-update ' + 'test add test=test --rollback') + mock_update.assert_not_called() @mock.patch('magnumclient.v1.clusters.ClusterManager.update') def test_cluster_update_failure_wrong_op(self, mock_update): diff --git a/magnumclient/v1/baseunit.py b/magnumclient/v1/baseunit.py index 4017c6f..7df9dcf 100644 --- a/magnumclient/v1/baseunit.py +++ b/magnumclient/v1/baseunit.py @@ -104,5 +104,8 @@ class BaseTemplateManager(base.Manager): def delete(self, id): return self._delete(self._path(id)) - def update(self, id, patch): - return self._update(self._path(id), patch) + def update(self, id, patch, rollback=False): + url = self._path(id) + if rollback: + url += '/?rollback=True' + return self._update(url, patch) diff --git a/magnumclient/v1/bays_shell.py b/magnumclient/v1/bays_shell.py index 79b14b5..7cfc7f7 100644 --- a/magnumclient/v1/bays_shell.py +++ b/magnumclient/v1/bays_shell.py @@ -176,6 +176,9 @@ def do_bay_show(cs, args): @utils.arg('bay', metavar='', help="UUID or name of bay") +@utils.arg('--rollback', + action='store_true', default=False, + help='Rollback bay on update failure.') @utils.arg( 'op', metavar='', @@ -195,8 +198,13 @@ def do_bay_update(cs, args): (Deprecated in favor of cluster-update.) """ + if args.rollback and args.magnum_api_version and \ + args.magnum_api_version in ('1.0', '1.1', '1.2'): + raise exceptions.CommandError( + "Rollback is not supported in API v%s. " + "Please use API v1.3+." % args.magnum_api_version) patch = magnum_utils.args_array_to_patch(args.op, args.attributes[0]) - bay = cs.bays.update(args.bay, patch) + bay = cs.bays.update(args.bay, patch, args.rollback) if args.magnum_api_version and args.magnum_api_version == '1.1': _show_bay(bay) else: diff --git a/magnumclient/v1/clusters_shell.py b/magnumclient/v1/clusters_shell.py index e5d8551..fff4a2a 100644 --- a/magnumclient/v1/clusters_shell.py +++ b/magnumclient/v1/clusters_shell.py @@ -184,6 +184,9 @@ def do_cluster_show(cs, args): @utils.arg('cluster', metavar='', help="UUID or name of cluster") +@utils.arg('--rollback', + action='store_true', default=False, + help='Rollback cluster on update failure.') @utils.arg( 'op', metavar='', @@ -199,8 +202,13 @@ def do_cluster_show(cs, args): "(only PATH is necessary on remove)") def do_cluster_update(cs, args): """Update information about the given cluster.""" + if args.rollback and args.magnum_api_version and \ + args.magnum_api_version in ('1.0', '1.1', '1.2'): + raise exceptions.CommandError( + "Rollback is not supported in API v%s. " + "Please use API v1.3+." % args.magnum_api_version) patch = magnum_utils.args_array_to_patch(args.op, args.attributes[0]) - cluster = cs.clusters.update(args.cluster, patch) + cluster = cs.clusters.update(args.cluster, patch, args.rollback) if args.magnum_api_version and args.magnum_api_version == '1.1': _show_cluster(cluster) else: