From 874a726f522dae3a08832f32230e2590a787d45c Mon Sep 17 00:00:00 2001
From: zhangbailin <zhangbailin@inspur.com>
Date: Wed, 31 Jul 2019 12:24:04 +0800
Subject: [PATCH] Microversion 2.79: Add delete_on_termination to volume-attach
 API

Added ``--disable-delete-on-termination`` and
``--enable-delete-on-termination`` options to the
``openstack server add volume`` command that enables users to mark
whether to delete the attached volume when the server is destroyed.

Depends-On: https://review.opendev.org/#/c/681267/

Part of blueprint support-delete-on-termination-in-server-attach-volume

Change-Id: I6b5cd54b82a1135335a71b9768a1a2c2012f755b
---
 lower-constraints.txt                         |   2 +-
 openstackclient/compute/v2/server.py          |  41 ++++-
 .../tests/unit/compute/v2/test_server.py      | 172 ++++++++++++++++++
 ...store-shelved-server-9179045f04815bbb.yaml |   9 +
 requirements.txt                              |   2 +-
 5 files changed, 223 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/bp-support-specifying-az-when-restore-shelved-server-9179045f04815bbb.yaml

diff --git a/lower-constraints.txt b/lower-constraints.txt
index 201726ac65..ad911e771d 100644
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@@ -100,7 +100,7 @@ python-mimeparse==1.6.0
 python-mistralclient==3.1.0
 python-muranoclient==0.8.2
 python-neutronclient==6.7.0
-python-novaclient==15.0.0
+python-novaclient==15.1.0
 python-octaviaclient==1.11.0
 python-rsdclient==1.0.1
 python-saharaclient==1.4.0
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index fb7596a9fd..414abb626a 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -446,6 +446,21 @@ class AddServerVolume(command.Command):
             metavar='<device>',
             help=_('Server internal device name for volume'),
         )
+        termination_group = parser.add_mutually_exclusive_group()
+        termination_group.add_argument(
+            '--enable-delete-on-termination',
+            action='store_true',
+            help=_("Specify if the attached volume should be deleted when "
+                   "the server is destroyed. (Supported with "
+                   "``--os-compute-api-version`` 2.79 or greater.)"),
+        )
+        termination_group.add_argument(
+            '--disable-delete-on-termination',
+            action='store_true',
+            help=_("Specify if the attached volume should not be deleted "
+                   "when the server is destroyed. (Supported with "
+                   "``--os-compute-api-version`` 2.79 or greater.)"),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -461,10 +476,34 @@ class AddServerVolume(command.Command):
             parsed_args.volume,
         )
 
+        support_set_delete_on_termination = (compute_client.api_version >=
+                                             api_versions.APIVersion('2.79'))
+
+        if not support_set_delete_on_termination:
+            if parsed_args.enable_delete_on_termination:
+                msg = _('--os-compute-api-version 2.79 or greater '
+                        'is required to support the '
+                        '--enable-delete-on-termination option.')
+                raise exceptions.CommandError(msg)
+            if parsed_args.disable_delete_on_termination:
+                msg = _('--os-compute-api-version 2.79 or greater '
+                        'is required to support the '
+                        '--disable-delete-on-termination option.')
+                raise exceptions.CommandError(msg)
+
+        kwargs = {
+            "device": parsed_args.device
+        }
+
+        if parsed_args.enable_delete_on_termination:
+            kwargs['delete_on_termination'] = True
+        if parsed_args.disable_delete_on_termination:
+            kwargs['delete_on_termination'] = False
+
         compute_client.volumes.create_server_volume(
             server.id,
             volume.id,
-            parsed_args.device,
+            **kwargs
         )
 
 
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 5c98188a76..c70d6d72d0 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -43,6 +43,10 @@ class TestServer(compute_fakes.TestComputev2):
         self.servers_mock = self.app.client_manager.compute.servers
         self.servers_mock.reset_mock()
 
+        # Get a shortcut to the compute client volumeManager Mock
+        self.servers_volumes_mock = self.app.client_manager.compute.volumes
+        self.servers_volumes_mock.reset_mock()
+
         # Get a shortcut to the compute client FlavorManager Mock
         self.flavors_mock = self.app.client_manager.compute.flavors
         self.flavors_mock.reset_mock()
@@ -457,6 +461,174 @@ class TestServerAddPort(TestServer):
         self.find_port.assert_not_called()
 
 
+class TestServerVolume(TestServer):
+
+    def setUp(self):
+        super(TestServerVolume, self).setUp()
+
+        self.volume = volume_fakes.FakeVolume.create_one_volume()
+        self.volumes_mock.get.return_value = self.volume
+
+        self.methods = {
+            'create_server_volume': None,
+        }
+
+        # Get the command object to test
+        self.cmd = server.AddServerVolume(self.app, None)
+
+    def test_server_add_volume(self):
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            '--device', '/dev/sdb',
+            servers[0].id,
+            self.volume.id,
+        ]
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('device', '/dev/sdb'),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_volumes_mock.create_server_volume.assert_called_once_with(
+            servers[0].id, self.volume.id, device='/dev/sdb')
+        self.assertIsNone(result)
+
+
+class TestServerVolumeV279(TestServerVolume):
+
+    def test_server_add_volume_with_enable_delete_on_termination(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.79')
+
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            '--enable-delete-on-termination',
+            '--device', '/dev/sdb',
+            servers[0].id,
+            self.volume.id,
+        ]
+
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('device', '/dev/sdb'),
+            ('enable_delete_on_termination', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_volumes_mock.create_server_volume.assert_called_once_with(
+            servers[0].id, self.volume.id,
+            device='/dev/sdb', delete_on_termination=True)
+        self.assertIsNone(result)
+
+    def test_server_add_volume_with_disable_delete_on_termination(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.79')
+
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            '--disable-delete-on-termination',
+            '--device', '/dev/sdb',
+            servers[0].id,
+            self.volume.id,
+        ]
+
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('device', '/dev/sdb'),
+            ('disable_delete_on_termination', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_volumes_mock.create_server_volume.assert_called_once_with(
+            servers[0].id, self.volume.id,
+            device='/dev/sdb', delete_on_termination=False)
+        self.assertIsNone(result)
+
+    def test_server_add_volume_with_enable_delete_on_termination_pre_v279(
+            self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.78')
+
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            servers[0].id,
+            self.volume.id,
+            '--enable-delete-on-termination',
+        ]
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('enable_delete_on_termination', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd.take_action,
+                               parsed_args)
+        self.assertIn('--os-compute-api-version 2.79 or greater is required',
+                      str(ex))
+
+    def test_server_add_volume_with_disable_delete_on_termination_pre_v279(
+            self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.78')
+
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            servers[0].id,
+            self.volume.id,
+            '--disable-delete-on-termination',
+        ]
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('disable_delete_on_termination', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(exceptions.CommandError,
+                               self.cmd.take_action,
+                               parsed_args)
+        self.assertIn('--os-compute-api-version 2.79 or greater is required',
+                      str(ex))
+
+    def test_server_add_volume_with_disable_and_enable_delete_on_termination(
+            self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.79')
+
+        servers = self.setup_servers_mock(count=1)
+        arglist = [
+            '--enable-delete-on-termination',
+            '--disable-delete-on-termination',
+            '--device', '/dev/sdb',
+            servers[0].id,
+            self.volume.id,
+        ]
+
+        verifylist = [
+            ('server', servers[0].id),
+            ('volume', self.volume.id),
+            ('device', '/dev/sdb'),
+            ('enable_delete_on_termination', True),
+            ('disable_delete_on_termination', True),
+        ]
+        ex = self.assertRaises(utils.ParserException,
+                               self.check_parser,
+                               self.cmd, arglist, verifylist)
+        self.assertIn('Argument parse failed', str(ex))
+
+
 class TestServerAddNetwork(TestServer):
 
     def setUp(self):
diff --git a/releasenotes/notes/bp-support-specifying-az-when-restore-shelved-server-9179045f04815bbb.yaml b/releasenotes/notes/bp-support-specifying-az-when-restore-shelved-server-9179045f04815bbb.yaml
new file mode 100644
index 0000000000..ec207eac0c
--- /dev/null
+++ b/releasenotes/notes/bp-support-specifying-az-when-restore-shelved-server-9179045f04815bbb.yaml
@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    Add ``--disable-delete-on-termination`` and
+    ``--enable-delete-on-termination`` options to the ``server add volume``
+    command to indicate when to delete the attached volume when the server
+    is deleted.
+    Note that it requires ``--os-compute-api-version 2.79`` or greater.
+    [Blueprint support-specifying-az-when-restore-shelved-server `<https://blueprints.launchpad.net/nova/+spec/support-delete-on-termination-in-server-attach-volume>`_]
diff --git a/requirements.txt b/requirements.txt
index 67139bbdbc..eea51e5163 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -13,5 +13,5 @@ oslo.i18n>=3.15.3 # Apache-2.0
 oslo.utils>=3.33.0 # Apache-2.0
 python-glanceclient>=2.8.0 # Apache-2.0
 python-keystoneclient>=3.17.0 # Apache-2.0
-python-novaclient>=15.0.0 # Apache-2.0
+python-novaclient>=15.1.0 # Apache-2.0
 python-cinderclient>=3.3.0 # Apache-2.0