From 8db3933feb35f91f3ff3d121c155286973c66122 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= <slawek@kaplonski.pl>
Date: Thu, 10 May 2018 17:16:48 +0200
Subject: [PATCH] Don't display router's is_ha and is_distributed attributes
 always

In case when is_ha or is_distributed attribute of Neutron's router is
set to None, it means that it wasn't returned from server and should
not be displayed.
Otherwise it might be confusing for user is making
openstack router show <router_name>
call as an admin will return e.g. is_ha=True but same call done as
regular user will return False or None.
It might happen like that because returning of those attributes is
forbidden for regular users in Neutron's policy.json

Depends-On: https://review.openstack.org/567606/
Change-Id: I626b5193d9ecb308baad7b27939f9673c32b4182
Closes-Bug: #1689510
Task: 19789
Story: 2002110
---
 openstackclient/network/v2/router.py          | 24 ++++++++---
 .../tests/unit/network/v2/test_router.py      | 43 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/openstackclient/network/v2/router.py b/openstackclient/network/v2/router.py
index f0a5196744..4819f279f2 100644
--- a/openstackclient/network/v2/router.py
+++ b/openstackclient/network/v2/router.py
@@ -71,7 +71,15 @@ def _get_columns(item):
     }
     if hasattr(item, 'interfaces_info'):
         column_map['interfaces_info'] = 'interfaces_info'
-    return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map)
+    invisible_columns = []
+    if item.is_ha is None:
+        invisible_columns.append('is_ha')
+        column_map.pop('is_ha')
+    if item.is_distributed is None:
+        invisible_columns.append('is_distributed')
+        column_map.pop('is_distributed')
+    return sdk_utils.get_osc_show_columns_for_sdk_resource(
+        item, column_map, invisible_columns)
 
 
 def _get_attrs(client_manager, parsed_args):
@@ -330,8 +338,6 @@ class ListRouter(command.Lister):
             'name',
             'status',
             'is_admin_state_up',
-            'is_distributed',
-            'is_ha',
             'project_id',
         )
         column_headers = (
@@ -339,8 +345,6 @@ class ListRouter(command.Lister):
             'Name',
             'Status',
             'State',
-            'Distributed',
-            'HA',
             'Project',
         )
 
@@ -376,6 +380,16 @@ class ListRouter(command.Lister):
         else:
             data = client.routers(**args)
 
+        # check if "HA" and "Distributed" columns should be displayed also
+        data = list(data)
+        for d in data:
+            if (d.is_distributed is not None and
+                    'is_distributed' not in columns):
+                columns = columns + ('is_distributed',)
+                column_headers = column_headers + ('Distributed',)
+            if d.is_ha is not None and 'is_ha' not in columns:
+                columns = columns + ('is_ha',)
+                column_headers = column_headers + ('HA',)
         if parsed_args.long:
             columns = columns + (
                 'routes',
diff --git a/openstackclient/tests/unit/network/v2/test_router.py b/openstackclient/tests/unit/network/v2/test_router.py
index f383c1ddb7..7a7bcf90a4 100644
--- a/openstackclient/tests/unit/network/v2/test_router.py
+++ b/openstackclient/tests/unit/network/v2/test_router.py
@@ -400,9 +400,9 @@ class TestListRouter(TestRouter):
         'Name',
         'Status',
         'State',
+        'Project',
         'Distributed',
         'HA',
-        'Project',
     )
     columns_long = columns + (
         'Routes',
@@ -423,9 +423,9 @@ class TestListRouter(TestRouter):
             r.name,
             r.status,
             router._format_admin_state(r.admin_state_up),
+            r.tenant_id,
             r.distributed,
             r.ha,
-            r.tenant_id,
         ))
 
     router_agent_data = []
@@ -496,6 +496,25 @@ class TestListRouter(TestRouter):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, list(data))
 
+    def test_router_list_no_ha_no_distributed(self):
+        _routers = network_fakes.FakeRouter.create_routers({
+            'ha': None,
+            'distributed': None},
+            count=3)
+
+        arglist = []
+        verifylist = [
+            ('long', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        with mock.patch.object(
+                self.network, "routers", return_value=_routers):
+            columns, data = self.cmd.take_action(parsed_args)
+
+        self.assertNotIn("is_distributed", columns)
+        self.assertNotIn("is_ha", columns)
+
     def test_router_list_long(self):
         arglist = [
             '--long',
@@ -1196,6 +1215,26 @@ class TestShowRouter(TestRouter):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
+    def test_show_no_ha_no_distributed(self):
+        _router = network_fakes.FakeRouter.create_one_router({
+            'ha': None,
+            'distributed': None})
+
+        arglist = [
+            _router.name,
+        ]
+        verifylist = [
+            ('router', _router.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        with mock.patch.object(
+                self.network, "find_router", return_value=_router):
+            columns, data = self.cmd.take_action(parsed_args)
+
+        self.assertNotIn("is_distributed", columns)
+        self.assertNotIn("is_ha", columns)
+
 
 class TestUnsetRouter(TestRouter):