fix loading to ignore deleted items

3 files changed, 53 insertions, 35 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 4c27440dc..d85b8512a 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -342,7 +342,7 @@ class CloudController(object):
             if match:
                 db.security_group_rule_destroy(context, rule['id'])
                 self._trigger_refresh_security_group(security_group)
-
+                return True
         raise exception.ApiError("No rule for the specified parameters.")
 
     # TODO(soren): Dupe detection. Adding the same rule twice actually
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index dad544cdb..fee50ec9c 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -29,7 +29,7 @@ from nova.db.sqlalchemy import models
 from nova.db.sqlalchemy.session import get_session
 from sqlalchemy import or_
 from sqlalchemy.exc import IntegrityError
-from sqlalchemy.orm import contains_eager, eagerload, joinedload_all
+from sqlalchemy.orm import contains_eager, joinedload_all
 from sqlalchemy.sql import exists, func
 
 FLAGS = flags.FLAGS
@@ -410,8 +410,17 @@ def instance_destroy(_context, instance_id):
 
 
 def instance_get(context, instance_id):
-    return models.Instance().find(instance_id, deleted=_deleted(context),
-                                    options=eagerload('security_groups'))
+    session = get_session()
+    instance_ref = session.query(models.Instance
+                    ).options(joinedload_all('fixed_ip.floating_ips')
+                    ).options(joinedload_all('security_groups')
+                    ).filter_by(id=instance_id
+                    ).filter_by(deleted=_deleted(context)
+                    ).first()
+    if not instance_ref:
+        raise exception.NotFound('Instance %s not found' % (instance_id))
+
+    return instance_ref
 
 
 def instance_get_all(context):
@@ -942,25 +951,29 @@ def volume_update(_context, volume_id, values):
 ###################
 
 
+INSTANCES_OR = or_(models.Instance.deleted == False,
+                   models.Instance.deleted == None)
+
+
+RULES_OR = or_(models.SecurityGroupIngressRule.deleted == False,
+               models.SecurityGroupIngressRule.deleted == None)
+
+
 def security_group_get_all(_context):
     session = get_session()
     return session.query(models.SecurityGroup
-                 ).join(models.SecurityGroup.rules
-                 ).options(contains_eager(models.SecurityGroup.rules)
-                 ).filter(models.SecurityGroupIngressRule.deleted == False
                  ).filter_by(deleted=False
+                 ).options(joinedload_all('rules')
                  ).all()
 
 
 def security_group_get(_context, security_group_id):
     session = get_session()
     result = session.query(models.SecurityGroup
-                   ).join(models.SecurityGroup.rules
-                   ).options(contains_eager(models.SecurityGroup.rules)
-                   ).filter(models.SecurityGroupIngressRule.deleted == False
                    ).filter_by(deleted=False
                    ).filter_by(id=security_group_id
-                   ).get(security_group_id)
+                   ).options(joinedload_all('rules')
+                   ).first()
     if not result:
         raise exception.NotFound("No secuity group with id %s" %
                                  security_group_id)
@@ -969,41 +982,41 @@ def security_group_get(_context, security_group_id):
 
 def security_group_get_by_name(context, project_id, group_name):
     session = get_session()
-    group_ref = session.query(models.SecurityGroup
-                      ).join(models.SecurityGroup.rules
-                      ).join(models.Instances
-                      ).options(contains_eager(models.SecurityGroup.rules)
-                      ).options(contains_eager(models.SecurityGroup.instances)
-                      ).filter(models.SecurityGroupIngressRule.deleted == False
+    result = session.query(models.SecurityGroup
                       ).filter_by(project_id=project_id
                       ).filter_by(name=group_name
                       ).filter_by(deleted=False
+                      ).options(joinedload_all('rules')
+                      ).options(joinedload_all('instances')
                       ).first()
-    if not group_ref:
+    if not result:
         raise exception.NotFound(
             'No security group named %s for project: %s' \
              % (group_name, project_id))
-    return group_ref
+    return result
 
 
 def security_group_get_by_project(_context, project_id):
     session = get_session()
     return session.query(models.SecurityGroup
-                 ).join(models.SecurityGroup.rules
-                 ).options(contains_eager(models.SecurityGroup.rules)
-                 ).filter(models.SecurityGroupIngressRule.deleted == False
                  ).filter_by(project_id=project_id
                  ).filter_by(deleted=False
+                 ).options(joinedload_all('rules')
+                 ).outerjoin(models.SecurityGroup.rules
+                 ).options(contains_eager(models.SecurityGroup.rules)
+                 ).filter(RULES_OR
                  ).all()
 
 
 def security_group_get_by_instance(_context, instance_id):
     session = get_session()
-    with session.begin():
-        return session.query(models.Instance
-                      ).join(models.Instance.security_groups
-                      ).filter_by(deleted=False
-                      ).all()
+    return session.query(models.SecurityGroup
+                 ).filter_by(deleted=False
+                 ).options(joinedload_all('rules')
+                 ).join(models.SecurityGroup.instances
+                 ).filter_by(id=instance_id
+                 ).filter_by(deleted=False
+                 ).all()
 
 
 def security_group_exists(_context, project_id, group_name):
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index d4caf0b52..b89616ddb 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -340,12 +340,12 @@ class ExportDevice(BASE, NovaBase):
                                                   uselist=False))
 
 
-security_group_instance_association = Table('security_group_instance_association',
-                                            BASE.metadata,
-                                            Column('security_group_id', Integer,
-                                                   ForeignKey('security_group.id')),
-                                            Column('instance_id', Integer,
-                                                   ForeignKey('instances.id')))
+class SecurityGroupInstanceAssociation(BASE, NovaBase):
+    __tablename__ = 'security_group_instance_association'
+    id = Column(Integer, primary_key=True)
+    security_group_id = Column(Integer, ForeignKey('security_group.id'))
+    instance_id = Column(Integer, ForeignKey('instances.id'))
+
 
 class SecurityGroup(BASE, NovaBase):
     """Represents a security group"""
@@ -358,7 +358,11 @@ class SecurityGroup(BASE, NovaBase):
     project_id = Column(String(255))
 
     instances = relationship(Instance,
-                             secondary=security_group_instance_association,
+                             secondary="security_group_instance_association",
+                             secondaryjoin="and_(SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id,"
+                                                 "Instance.id == SecurityGroupInstanceAssociation.instance_id,"
+                                                 "SecurityGroup.deleted == False,"
+                                                 "Instance.deleted == False)",
                              backref='security_groups')
 
     @property
@@ -378,7 +382,8 @@ class SecurityGroupIngressRule(BASE, NovaBase):
     parent_group_id = Column(Integer, ForeignKey('security_group.id'))
     parent_group = relationship("SecurityGroup", backref="rules",
                          foreign_keys=parent_group_id,
-                         primaryjoin=parent_group_id==SecurityGroup.id)
+                         primaryjoin="and_(SecurityGroupIngressRule.parent_group_id == SecurityGroup.id,"
+                                          "SecurityGroupIngressRule.deleted == False)")
 
     protocol = Column(String(5)) # "tcp", "udp", or "icmp"
     from_port = Column(Integer)