Repository: softlayer/softlayer-python

Issue:
`vs migrate` has an invalid filter
Filtering off `pendingMigrationFlag` doesn't look to actually work.
```
$> slcli call-api SoftLayer_Account getVirtualGuests --mask="mask[id,hostname,pendingMigrationFlag]" --json-filter='{"virtualGuests":{"pendingMigrationFlag":{"operation":0}}}'
┌──────────────────┬───────────┬──────────────────────┐
│     hostname     │    id     │ pendingMigrationFlag │
├──────────────────┼───────────┼──────────────────────┤
│       adns       │ 100250634 │        False         │
│ cgallo-bnpp-test │ 139920549 │        False         │
│       cp4d       │ 133871998 │        False         │
│    easerverh     │ 132364928 │        False         │
│     haproxy      │ 133986540 │        False         │
│     instana      │ 132690142 │         True         │
│      pxe-1       │ 133926476 │        False         │
│     reserved     │ 102396266 │        False         │
└──────────────────┴───────────┴──────────────────────┘
 slcli call-api SoftLayer_Account getVirtualGuests --mask="mask[id,hostname,pendingMigrationFlag]" --json-filter='{"virtualGuests":{"pendingMigrationFlag":{"operation":1}}}'
[]

$> slcli -v vs migrate
Calling: SoftLayer_Account::getVirtualGuests(id=None, mask='mask[ id, hostname, domain, datacenter, pendingMigrationFlag, powerState, primaryIpAddress,primaryBackendIpAddress, dedicatedHost ]', filter='{'virtualGuests': {'pendingMigrationFlag': {'operation': 1}}}', args=(), limit=100, offset=0)
No guests require migration at this time

```

Discussion:

Solution patch:
diff --git a/.secrets.baseline b/.secrets.baseline
index 9062c20d5..b256d881d 100644
--- a/.secrets.baseline
+++ b/.secrets.baseline
@@ -3,7 +3,7 @@
     "files": "^.secrets.baseline$",
     "lines": null
   },
-  "generated_at": "2024-10-04T22:18:14Z",
+  "generated_at": "2024-10-07T21:05:06Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -112,7 +112,7 @@
         "hashed_secret": "6367c48dd193d56ea7b0baad25b19455e529f5ee",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 121,
+        "line_number": 122,
         "type": "Secret Keyword",
         "verified_result": null
       },
@@ -120,7 +120,7 @@
         "hashed_secret": "df51e37c269aa94d38f93e537bf6e2020b21406c",
         "is_secret": false,
         "is_verified": false,
-        "line_number": 1035,
+        "line_number": 1036,
         "type": "Secret Keyword",
         "verified_result": null
       }
diff --git a/SoftLayer/CLI/virt/migrate.py b/SoftLayer/CLI/virt/migrate.py
index b4a1edef9..c1a1028bf 100644
--- a/SoftLayer/CLI/virt/migrate.py
+++ b/SoftLayer/CLI/virt/migrate.py
@@ -19,7 +19,6 @@ def cli(env, guest, migrate_all, host):
     """Manage VSIs that require migration. Can migrate Dedicated Host VSIs as well."""
 
     vsi = SoftLayer.VSManager(env.client)
-    pending_filter = {'virtualGuests': {'pendingMigrationFlag': {'operation': 1}}}
     dedicated_filter = {'virtualGuests': {'dedicatedHost': {'id': {'operation': 'not null'}}}}
     mask = """mask[
                 id, hostname, domain, datacenter, pendingMigrationFlag, powerState,
@@ -28,21 +27,22 @@ def cli(env, guest, migrate_all, host):
 
     # No options, just print out a list of guests that can be migrated
     if not (guest or migrate_all):
-        require_migration = vsi.list_instances(filter=pending_filter, mask=mask)
+        require_migration = vsi.list_instances(mask=mask)
         require_table = formatting.Table(['id', 'hostname', 'domain', 'datacenter'], title="Require Migration")
 
         for vsi_object in require_migration:
-            require_table.add_row([
-                vsi_object.get('id'),
-                vsi_object.get('hostname'),
-                vsi_object.get('domain'),
-                utils.lookup(vsi_object, 'datacenter', 'name')
-            ])
+            if vsi_object.get('pendingMigrationFlag', False):
+                require_table.add_row([
+                    vsi_object.get('id'),
+                    vsi_object.get('hostname'),
+                    vsi_object.get('domain'),
+                    utils.lookup(vsi_object, 'datacenter', 'name')
+                ])
 
-        if require_migration:
+        if len(require_table.rows) > 0:
             env.fout(require_table)
         else:
-            click.secho("No guests require migration at this time", fg='green')
+            click.secho("No guests require migration at this time.", fg='green')
 
         migrateable = vsi.list_instances(filter=dedicated_filter, mask=mask)
         migrateable_table = formatting.Table(['id', 'hostname', 'domain', 'datacenter', 'Host Name', 'Host Id'],
@@ -56,14 +56,20 @@ def cli(env, guest, migrate_all, host):
                 utils.lookup(vsi_object, 'dedicatedHost', 'name'),
                 utils.lookup(vsi_object, 'dedicatedHost', 'id')
             ])
-        env.fout(migrateable_table)
+        if len(migrateable_table.rows) > 0:
+            env.fout(migrateable_table)
+        else:
+            click.secho("No dedicated guests to migrate.", fg='green')
     # Migrate all guests with pendingMigrationFlag=True
     elif migrate_all:
-        require_migration = vsi.list_instances(filter=pending_filter, mask="mask[id]")
-        if not require_migration:
-            click.secho("No guests require migration at this time", fg='green')
+        require_migration = vsi.list_instances(mask="mask[id,pendingMigrationFlag]")
+        migrated = 0
         for vsi_object in require_migration:
-            migrate(vsi, vsi_object['id'])
+            if vsi_object.get('pendingMigrationFlag', False):
+                migrated = migrated + 1
+                migrate(vsi, vsi_object['id'])
+        if migrated == 0:
+            click.secho("No guests require migration at this time", fg='green')
     # Just migrate based on the options
     else:
         migrate(vsi, guest, host)
diff --git a/SoftLayer/fixtures/SoftLayer_Account.py b/SoftLayer/fixtures/SoftLayer_Account.py
index 96a1a0ee8..a8f1ff71c 100644
--- a/SoftLayer/fixtures/SoftLayer_Account.py
+++ b/SoftLayer/fixtures/SoftLayer_Account.py
@@ -35,7 +35,7 @@
     'globalIdentifier': '1a2b3c-1701',
     'primaryBackendIpAddress': '10.45.19.37',
     'hourlyBillingFlag': False,
-
+    'pendingMigrationFlag': True,
     'billingItem': {
         'id': 6327,
         'recurringFee': 1.54,
@@ -63,6 +63,7 @@
     'globalIdentifier': '05a8ac-6abf0',
     'primaryBackendIpAddress': '10.45.19.35',
     'hourlyBillingFlag': True,
+    'pendingMigrationFlag': True,
     'billingItem': {
         'id': 6327,
         'recurringFee': 1.54,
diff --git a/SoftLayer/managers/vs.py b/SoftLayer/managers/vs.py
index 534ea246e..dddc6310e 100644
--- a/SoftLayer/managers/vs.py
+++ b/SoftLayer/managers/vs.py
@@ -131,6 +131,7 @@ def list_instances(self, hourly=True, monthly=True, tags=None, cpus=None,
                 call = 'getMonthlyVirtualGuests'
 
         _filter = utils.NestedDict(kwargs.get('filter') or {})
+        _filter['virtualGuests']['id'] = utils.query_filter_orderby()
         if tags:
             _filter['virtualGuests']['tagReferences']['tag']['name'] = {
                 'operation': 'in',


Test patch:
diff --git a/tests/managers/vs/vs_tests.py b/tests/managers/vs/vs_tests.py
index a0ac6dae8..7d644630c 100644
--- a/tests/managers/vs/vs_tests.py
+++ b/tests/managers/vs/vs_tests.py
@@ -65,6 +65,7 @@ def test_list_instances_with_filters(self):
 
         _filter = {
             'virtualGuests': {
+                'id': {'operation': 'orderBy', 'options': [{'name': 'sort', 'value': ['ASC']}]},
                 'datacenter': {
                     'name': {'operation': '_= dal05'}},
                 'domain': {'operation': '_= example.com'},
@@ -83,8 +84,7 @@ def test_list_instances_with_filters(self):
                 'transientGuestFlag': {'operation': False},
             }
         }
-        self.assert_called_with('SoftLayer_Account', 'getVirtualGuests',
-                                filter=_filter)
+        self.assert_called_with('SoftLayer_Account', 'getVirtualGuests', filter=_filter)
 
     def test_resolve_ids_ip(self):
         _id = self.vs._get_ids_from_ip('172.16.240.2')