Don't lazy-load instance.services if the instance is deleted

The 2.16 microversion added the host_status extended
server attribute which relies on the instance.services field.

The primary join in the database for that field is dependent on
the instance not being deleted.

When listing deleted instances at microversion>=2.16, the
compute API attempts to lazy-load the instance.services field
which fails with an InstanceNotFound because the instance
is deleted.

In this case, it's best to just set instance.services to an
empty ServiceList when lazy loading the services field on a
deleted instance since the DB object won't have any value for
the services attribute anyway.

Change-Id: Ic2f239f634f917a5771b0401a5073546c710c036
Closes-Bug: #1548980

nova/objects/instance.py

@@ -897,6 +897,11 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
             self._load_security_groups()
         elif 'flavor' in attrname:
             self._load_flavor()
+        elif attrname == 'services' and self.deleted:
+            # NOTE(mriedem): The join in the data model for instances.services
+            # filters on instances.deleted == 0, so if the instance is deleted
+            # don't attempt to even load services since we'll fail.
+            self.services = objects.ServiceList(self._context)
         else:
             # FIXME(comstud): This should be optimized to only load the attr.
             self._load_generic(attrname)

nova/tests/functional/regressions/test_bug_1548980.py

@@ -83,6 +83,9 @@ class TestServerGet(test.TestCase):
                 break
         else:
             self.fail('Timed out waiting to delete server: %s' % server['id'])
-        self.assertRaises(client.OpenStackApiNotFoundException,
-                          self.admin_api.get_servers,
-                          search_opts={'deleted': 1})
+        servers = self.admin_api.get_servers(search_opts={'deleted': 1})
+        self.assertEqual(1, len(servers))
+        self.assertEqual(server['id'], servers[0]['id'])
+        # host_status is returned in the 2.16 microversion and since the server
+        # is deleted it should be the empty string
+        self.assertEqual(0, len(servers[0]['host_status']))

nova/tests/unit/objects/test_instance.py

@@ -191,6 +191,13 @@ class _TestInstanceObject(object):
             self.assertTrue(inst.obj_attr_is_set(attr))
         self.assertEqual(123, inst.services[0].id)
 
+    def test_lazy_load_services_on_deleted_instance(self):
+        # We should avoid trying to hit the database to reload the instance
+        # and just set the services attribute to an empty list.
+        instance = objects.Instance(self.context, uuid=uuids.instance,
+                                    deleted=True)
+        self.assertEqual(0, len(instance.services))
+
     def test_get_by_id(self):
         self.mox.StubOutWithMock(db, 'instance_get')
         db.instance_get(self.context, 'instid',