From 415545ab9fd842bdc19b7fbfa63e3332dd63fe6c Mon Sep 17 00:00:00 2001
From: yanpuqing <yanpq@awcloud.com>
Date: Thu, 20 Jun 2019 11:14:22 +0800
Subject: [PATCH] Add an error message when server bind floating IP

If we add a floating IP for the server with no fixed IP, CLI doesn't
report an error and nothing happens. The patch adds an error message
when the server which don't have fixed IP bind floating IP.

Change-Id: I400f2bab08521bb7fa443d87c7f45cc79eb80694
Task: 27941
Story: 2004346
---
 openstackclient/compute/v2/server.py          |  4 ++
 .../tests/unit/compute/v2/test_server.py      | 48 +++++++++++++++----
 ...ing-ip-with-no-ports-399c5559e1699816.yaml |  7 +++
 3 files changed, 51 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/story-2004346-add-floating-ip-with-no-ports-399c5559e1699816.yaml

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index e5a7a3283..30057111b 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -292,6 +292,10 @@ class AddFloatingIP(network_common.NetworkAndComputeCommand):
             parsed_args.server,
         )
         ports = list(client.ports(device_id=server.id))
+        if not ports:
+            msg = _('No attached ports found to associate floating IP with')
+            raise exceptions.CommandError(msg)
+
         # If the fixed IP address was specified, we need to find the
         # corresponding port.
         if parsed_args.fixed_ip_address:
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 59282b4a1..52a6f5171 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -249,7 +249,7 @@ class TestServerAddFloatingIPNetwork(
         # Get the command object to test
         self.cmd = server.AddFloatingIP(self.app, self.namespace)
 
-    def test_server_add_floating_ip_default(self):
+    def test_server_add_floating_ip(self):
         _server = compute_fakes.FakeServer.create_one_server()
         self.servers_mock.get.return_value = _server
         _port = network_fakes.FakePort.create_one_port()
@@ -284,8 +284,41 @@ class TestServerAddFloatingIPNetwork(
             **attrs
         )
 
-    def test_server_add_floating_ip_default_no_external_gateway(self,
-                                                                success=False):
+    def test_server_add_floating_ip_no_ports(self):
+        server = compute_fakes.FakeServer.create_one_server()
+        floating_ip = network_fakes.FakeFloatingIP.create_one_floating_ip()
+
+        self.servers_mock.get.return_value = server
+        self.network.find_ip = mock.Mock(return_value=floating_ip)
+        self.network.ports = mock.Mock(return_value=[])
+
+        arglist = [
+            server.id,
+            floating_ip['floating_ip_address'],
+        ]
+        verifylist = [
+            ('server', server.id),
+            ('ip_address', floating_ip['floating_ip_address']),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            'No attached ports found to associate floating IP with',
+            str(ex))
+
+        self.network.find_ip.assert_called_once_with(
+            floating_ip['floating_ip_address'],
+            ignore_missing=False,
+        )
+        self.network.ports.assert_called_once_with(
+            device_id=server.id,
+        )
+
+    def test_server_add_floating_ip_no_external_gateway(self, success=False):
         _server = compute_fakes.FakeServer.create_one_server()
         self.servers_mock.get.return_value = _server
         _port = network_fakes.FakePort.create_one_port()
@@ -338,11 +371,10 @@ class TestServerAddFloatingIPNetwork(
                 **attrs
             )
 
-    def test_server_add_floating_ip_default_one_external_gateway(self):
-        self.test_server_add_floating_ip_default_no_external_gateway(
-            success=True)
+    def test_server_add_floating_ip_one_external_gateway(self):
+        self.test_server_add_floating_ip_no_external_gateway(success=True)
 
-    def test_server_add_floating_ip_fixed(self):
+    def test_server_add_floating_ip_with_fixed_ip(self):
         _server = compute_fakes.FakeServer.create_one_server()
         self.servers_mock.get.return_value = _server
         _port = network_fakes.FakePort.create_one_port()
@@ -384,7 +416,7 @@ class TestServerAddFloatingIPNetwork(
             **attrs
         )
 
-    def test_server_add_floating_ip_fixed_no_port_found(self):
+    def test_server_add_floating_ip_with_fixed_ip_no_port_found(self):
         _server = compute_fakes.FakeServer.create_one_server()
         self.servers_mock.get.return_value = _server
         _port = network_fakes.FakePort.create_one_port()
diff --git a/releasenotes/notes/story-2004346-add-floating-ip-with-no-ports-399c5559e1699816.yaml b/releasenotes/notes/story-2004346-add-floating-ip-with-no-ports-399c5559e1699816.yaml
new file mode 100644
index 000000000..ebcb7f631
--- /dev/null
+++ b/releasenotes/notes/story-2004346-add-floating-ip-with-no-ports-399c5559e1699816.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Associating a floating IP with a server using the ``server add floating
+    ip`` command requires the server have at least one port associated with it.
+    Previously, this was not validated, meaning the operation would silently
+    fail. This has been resolved.