Repository: softlayer/softlayer-python

Issue:
Sub features were missing in slcli hardware list, slcli hardware power-cycle 
1. slcli hardware list 
Options: --order, --owner, --private-ip, --public-ip were missing, please add it.
Options:
  -o, --order           Filter by ID of the order which purchased hardware server
       --owner        Filter by ID of the owner
  -v, --private-ip   Filter by private IP address
  -p, --public-ip    Filter by public IP address
Expected:
```
./slcli hardware list --help
Usage: slcli hardware list [OPTIONS]

        List hardware servers.

┌────┬──────────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ -c │ --cpu        │ Filter by number of CPU cores                                                                                                            │
│ -D │ --domain     │ Filter by domain                                                                                                                         │
│ -d │ --datacenter │ Filter by datacenter                                                                                                                     │
│ -H │ --hostname   │ Filter by hostname                                                                                                                       │
│ -m │ --memory     │ Filter by memory in gigabytes                                                                                                            │
│ -n │ --network    │ Filter by network port speed in Mbps                                                                                                     │
│    │ --search     │ Use the more flexible Search API to list instances. See `slcli search --types` for list of searchable fields.                            │
│    │ --tag        │ Filter by tags (multiple occurrence permitted)                                                                                           │
│    │ --sortby     │ Column to sort by  [default: hostname]                                                                                                   │
│    │ --columns    │ Columns to display. [options: guid, primary_ip, backend_ip, datacenter, action, created_by, tags]  [default:                             │
│    │              │ id,hostname,primary_ip,backend_ip,datacenter,action]                                                                                     │
│ -l │ --limit      │ How many results to get in one api call, default is 100  [default: 100]                                                                  │
│ -h │ --help       │ Show this message and exit.                                                                                                              │
│ -o │ --order       │ Filter by ID of the order which purchased hardware server                                                                                         │
│    │ --owner       │ Filter by ID of the owner                                                                                                              │
│ -v │ --private-ip       │ Filter by private IP address                                                                                                              │
│ -p │ --public-ip       │ Filter by public IP address                                                                                                              │
└────┴──────────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
```

2. slcli hardware power-cycle 
Options: --force was missing, please add it.
Options: 
  -f, --force   Force operation without confirmation
Expected:
```
 ./slcli hardware power-cycle --help  
Usage: slcli hardware power-cycle [OPTIONS] IDENTIFIER

        Power cycle a server.

┌────┬────────────┬─────────────────────────────┐
│    │ identifier │                             │
│ -h │ --help     │ Show this message and exit. │
│ -f │ --force     │ Force operation without confirmation │
└────┴────────────┴─────────────────────────────┘
```


Discussion:

Patch:
diff --git a/SoftLayer/CLI/hardware/list.py b/SoftLayer/CLI/hardware/list.py
index 734f379d..65a95718 100644
--- a/SoftLayer/CLI/hardware/list.py
+++ b/SoftLayer/CLI/hardware/list.py
@@ -22,7 +22,7 @@
         lambda server: formatting.active_txn(server),
         mask='activeTransaction[id, transactionStatus[name, friendlyName]]'),
     column_helper.Column(
-        'created_by',
+        'owner',
         lambda created_by: utils.lookup(created_by, 'billingItem', 'orderItem', 'order', 'userRecord', 'username'),
         mask='billingItem[id,orderItem[id,order[id,userRecord[username]]]]'),
     column_helper.Column(
@@ -38,6 +38,8 @@
     'backend_ip',
     'datacenter',
     'action',
+    'owner',
+    'tags',
 ]
 
 
@@ -48,6 +50,9 @@
 @click.option('--hostname', '-H', help='Filter by hostname')
 @click.option('--memory', '-m', help='Filter by memory in gigabytes')
 @click.option('--network', '-n', help='Filter by network port speed in Mbps')
+@click.option('--owner', help='Filter by created_by username')
+@click.option('--primary_ip', help='Filter by Primary Ip Address')
+@click.option('--backend_ip', help='Filter by Backend Ip Address')
 @click.option('--search', is_flag=False, flag_value="", default=None,
               help="Use the more flexible Search API to list instances. See `slcli search --types` for list " +
                    "of searchable fields.")
@@ -63,29 +68,41 @@
               default=100,
               show_default=True)
 @environment.pass_env
-def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, search, tag, columns, limit):
+def cli(env, sortby, cpu, domain, datacenter, hostname, memory, network, owner, primary_ip, backend_ip,
+        search, tag, columns, limit):
     """List hardware servers."""
 
     if search is not None:
         object_mask = "mask[resource(SoftLayer_Hardware)]"
         search_manager = SoftLayer.SearchManager(env.client)
-        servers = search_manager.search_hadrware_instances(hostname=hostname, domain=domain, datacenter=datacenter,
-                                                           tags=tag, search_string=search, mask=object_mask)
+        servers = search_manager.search_hadrware_instances(
+            hostname=hostname,
+            domain=domain,
+            datacenter=datacenter,
+            tags=tag,
+            search_string=search,
+            mask=object_mask)
 
     else:
         manager = SoftLayer.HardwareManager(env.client)
-        servers = manager.list_hardware(hostname=hostname,
-                                        domain=domain,
-                                        cpus=cpu,
-                                        memory=memory,
-                                        datacenter=datacenter,
-                                        nic_speed=network,
-                                        tags=tag,
-                                        mask="mask(SoftLayer_Hardware_Server)[%s]" % columns.mask(),
-                                        limit=limit)
+        servers = manager.list_hardware(
+            hostname=hostname,
+            domain=domain,
+            cpus=cpu,
+            memory=memory,
+            datacenter=datacenter,
+            nic_speed=network,
+            tags=tag,
+            owner=owner,
+            public_ip=primary_ip,
+            private_ip=backend_ip,
+            mask="mask(SoftLayer_Hardware_Server)[%s]" % columns.mask(),
+            limit=limit)
 
     table = formatting.Table(columns.columns)
     table.sortby = sortby
+    table.align['created_by'] = 'l'
+    table.align['tags'] = 'l'
 
     for server in servers:
         table.add_row([value or formatting.blank()
diff --git a/SoftLayer/managers/hardware.py b/SoftLayer/managers/hardware.py
index 6564fda0..c8dea24b 100644
--- a/SoftLayer/managers/hardware.py
+++ b/SoftLayer/managers/hardware.py
@@ -121,7 +121,7 @@ def cancel_hardware(self, hardware_id, reason='unneeded', comment='', immediate=
 
     @retry(logger=LOGGER)
     def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
-                      domain=None, datacenter=None, nic_speed=None,
+                      domain=None, datacenter=None, nic_speed=None, owner=None,
                       public_ip=None, private_ip=None, **kwargs):
         """List all hardware (servers and bare metal computing instances).
 
@@ -169,6 +169,7 @@ def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
                               % (','.join(hw_items), ','.join(server_items)))
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
+        _filter['hardware']['id'] = utils.query_filter_orderby()
         if tags:
             _filter['hardware']['tagReferences']['tag']['name'] = {
                 'operation': 'in',
@@ -176,8 +177,7 @@ def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
             }
 
         if cpus:
-            _filter['hardware']['processorPhysicalCoreAmount'] = (
-                utils.query_filter(cpus))
+            _filter['hardware']['processorPhysicalCoreAmount'] = utils.query_filter(cpus)
 
         if memory:
             _filter['hardware']['memoryCapacity'] = utils.query_filter(memory)
@@ -189,20 +189,20 @@ def list_hardware(self, tags=None, cpus=None, memory=None, hostname=None,
             _filter['hardware']['domain'] = utils.query_filter(domain)
 
         if datacenter:
-            _filter['hardware']['datacenter']['name'] = (
-                utils.query_filter(datacenter))
+            _filter['hardware']['datacenter']['name'] = utils.query_filter(datacenter)
 
         if nic_speed:
-            _filter['hardware']['networkComponents']['maxSpeed'] = (
-                utils.query_filter(nic_speed))
+            _filter['hardware']['networkComponents']['maxSpeed'] = utils.query_filter(nic_speed)
 
         if public_ip:
-            _filter['hardware']['primaryIpAddress'] = (
-                utils.query_filter(public_ip))
+            _filter['hardware']['primaryIpAddress'] = utils.query_filter(public_ip)
 
         if private_ip:
-            _filter['hardware']['primaryBackendIpAddress'] = (
-                utils.query_filter(private_ip))
+            _filter['hardware']['primaryBackendIpAddress'] = utils.query_filter(private_ip)
+
+        if owner:
+            _filter['hardware']['billingItem']['orderItem']['order']['userRecord']['username'] = (
+                utils.query_filter(owner))
 
         kwargs['filter'] = _filter.to_dict()
         kwargs['iter'] = True


Test patch:
diff --git a/tests/CLI/modules/hardware/hardware_basic_tests.py b/tests/CLI/modules/hardware/hardware_basic_tests.py
index d7c2ca9b..a5597872 100644
--- a/tests/CLI/modules/hardware/hardware_basic_tests.py
+++ b/tests/CLI/modules/hardware/hardware_basic_tests.py
@@ -169,47 +169,6 @@ def test_detail_drives(self):
         self.assertEqual(output['drives'][0]['Name'], 'Seagate Constellation ES')
         self.assertEqual(output['drives'][0]['Serial #'], 'z1w4sdf')
 
-    def test_list_servers(self):
-        result = self.run_command(['server', 'list', '--tag=openstack'])
-
-        expected = [
-            {
-                'datacenter': 'TEST00',
-                'primary_ip': '172.16.1.100',
-                'hostname': 'hardware-test1',
-                'id': 1000,
-                'backend_ip': '10.1.0.2',
-                'action': 'TXN_NAME',
-            },
-            {
-                'datacenter': 'TEST00',
-                'primary_ip': '172.16.4.94',
-                'hostname': 'hardware-test2',
-                'id': 1001,
-                'backend_ip': '10.1.0.3',
-                'action': None,
-            },
-            {
-                'datacenter': 'TEST00',
-                'primary_ip': '172.16.4.95',
-                'hostname': 'hardware-bad-memory',
-                'id': 1002,
-                'backend_ip': '10.1.0.4',
-                'action': None,
-            },
-            {
-                'action': None,
-                'backend_ip': None,
-                'datacenter': None,
-                'hostname': None,
-                'id': 1003,
-                'primary_ip': None,
-            },
-        ]
-
-        self.assert_no_fail(result)
-        self.assertEqual(expected, json.loads(result.output))
-
     @mock.patch('SoftLayer.CLI.formatting.no_going_back')
     @mock.patch('SoftLayer.HardwareManager.reload')
     def test_server_reload(self, reload_mock, ngb_mock):
@@ -992,17 +951,6 @@ def test_create_credential(self):
                                    '--notes', 'test slcli', '--software', 'system'])
         self.assert_no_fail(result)
 
-    def test_list_hw_search_noargs(self):
-        result = self.run_command(['hw', 'list', '--search'])
-        self.assert_no_fail(result)
-        self.assert_called_with('SoftLayer_Search', 'advancedSearch', args=('_objectType:SoftLayer_Hardware ',))
-
-    def test_list_hw_search_noargs_domain(self):
-        result = self.run_command(['hw', 'list', '--search', '-Dtest'])
-        self.assert_no_fail(result)
-        self.assert_called_with('SoftLayer_Search', 'advancedSearch',
-                                args=('_objectType:SoftLayer_Hardware  domain: *test*',))
-
     @mock.patch('SoftLayer.CLI.formatting.confirm')
     def test_hardware_cancel_no_force(self, confirm_mock):
         confirm_mock.return_value = False
diff --git a/tests/CLI/modules/hardware/hardware_list_tests.py b/tests/CLI/modules/hardware/hardware_list_tests.py
new file mode 100644
index 00000000..1532da60
--- /dev/null
+++ b/tests/CLI/modules/hardware/hardware_list_tests.py
@@ -0,0 +1,93 @@
+"""
+    SoftLayer.tests.CLI.modules.hardware.hardware_list_tests
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    These tests the `slcli hw list` command. Its complex enough to warrant its own file
+
+    :license: MIT, see LICENSE for more details.
+"""
+
+import json
+
+from SoftLayer import testing
+from SoftLayer import utils
+
+
+class HardwareListCLITests(testing.TestCase):
+    def test_list_servers(self):
+        colums = 'datacenter,primary_ip,hostname,id,backend_ip,action'
+        result = self.run_command(['server', 'list', '--tag=openstack', f'--columns={colums}'])
+
+        expected = [
+            {
+                'datacenter': 'TEST00',
+                'primary_ip': '172.16.1.100',
+                'hostname': 'hardware-test1',
+                'id': 1000,
+                'backend_ip': '10.1.0.2',
+                'action': 'TXN_NAME',
+            },
+            {
+                'datacenter': 'TEST00',
+                'primary_ip': '172.16.4.94',
+                'hostname': 'hardware-test2',
+                'id': 1001,
+                'backend_ip': '10.1.0.3',
+                'action': None,
+            },
+            {
+                'datacenter': 'TEST00',
+                'primary_ip': '172.16.4.95',
+                'hostname': 'hardware-bad-memory',
+                'id': 1002,
+                'backend_ip': '10.1.0.4',
+                'action': None,
+            },
+            {
+                'action': None,
+                'backend_ip': None,
+                'datacenter': None,
+                'hostname': None,
+                'id': 1003,
+                'primary_ip': None,
+            },
+        ]
+
+        self.assert_no_fail(result)
+        self.assertEqual(expected, json.loads(result.output))
+
+    def test_list_hw_search_noargs(self):
+        result = self.run_command(['hw', 'list', '--search'])
+        self.assert_no_fail(result)
+        self.assert_called_with('SoftLayer_Search', 'advancedSearch', args=('_objectType:SoftLayer_Hardware ',))
+
+    def test_list_hw_search_noargs_domain(self):
+        result = self.run_command(['hw', 'list', '--search', '-Dtest'])
+        self.assert_no_fail(result)
+        self.assert_called_with('SoftLayer_Search', 'advancedSearch',
+                                args=('_objectType:SoftLayer_Hardware  domain: *test*',))
+
+    def test_list_by_owner(self):
+        result = self.run_command(['hw', 'list', '--owner=testUser'])
+        self.assert_no_fail(result)
+        expectedFilter = utils.NestedDict()
+        expectedFilter['hardware']['id'] = utils.query_filter_orderby()
+        expectedFilter['hardware']['billingItem']['orderItem']['order']['userRecord']['username'] = (
+                utils.query_filter('testUser'))
+        self.assert_called_with('SoftLayer_Account', 'getHardware', filter=expectedFilter)
+
+    def test_list_by_pub_ip(self):
+        result = self.run_command(['hw', 'list', '--primary_ip=1.2.3.4'])
+        self.assert_no_fail(result)
+        expectedFilter = utils.NestedDict()
+        expectedFilter['hardware']['id'] = utils.query_filter_orderby()
+        expectedFilter['hardware']['primaryIpAddress'] = utils.query_filter('1.2.3.4')
+        self.assert_called_with('SoftLayer_Account', 'getHardware', filter=expectedFilter)
+
+    def test_list_by_pri_ip(self):
+        result = self.run_command(['hw', 'list', '--backend_ip=1.2.3.4'])
+        self.assert_no_fail(result)
+        expectedFilter = utils.NestedDict()
+        expectedFilter['hardware']['id'] = utils.query_filter_orderby()
+        expectedFilter['hardware']['primaryBackendIpAddress'] = utils.query_filter('1.2.3.4')
+        self.assert_called_with('SoftLayer_Account', 'getHardware', filter=expectedFilter)
diff --git a/tests/managers/hardware_tests.py b/tests/managers/hardware_tests.py
index c067dd08..12870a43 100644
--- a/tests/managers/hardware_tests.py
+++ b/tests/managers/hardware_tests.py
@@ -57,6 +57,7 @@ def test_list_hardware_with_filters(self):
         self.assertEqual(results, fixtures.SoftLayer_Account.getHardware)
         _filter = {
             'hardware': {
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]},
                 'datacenter': {'name': {'operation': '_= dal05'}},
                 'domain': {'operation': '_= example.com'},
                 'tagReferences': {
@@ -73,8 +74,7 @@ def test_list_hardware_with_filters(self):
                 'networkComponents': {'maxSpeed': {'operation': 100}},
                 'primaryBackendIpAddress': {'operation': '_= 4.3.2.1'}}
         }
-        self.assert_called_with('SoftLayer_Account', 'getHardware',
-                                filter=_filter)
+        self.assert_called_with('SoftLayer_Account', 'getHardware', filter=_filter)
 
     def test_resolve_ids_ip(self):
         _id = self.hardware._get_ids_from_ip('172.16.1.100')