Merge pull request #604 from cdosborn/fix-broken-fixed-ip-add

Make another attempt to fix adding fixed ips

CHANGELOG.md

@@ -28,6 +28,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 ### Added
  - Support multiple hostnames for Atmosphere(1) server ([#602](https://github.com/cyverse/atmosphere/pull/602))
 
+### Fixed
+ - On start/unshelve instances would fail to be reachable because ports added
+   post boot ([#604](https://github.com/cyverse/atmosphere/pull/604))
+
 ## [v32-0](https://github.com/cyverse/atmosphere/compare/v31-1...v32-0) 2018-04-03
 ### Changed
 ### Added

service/instance.py

@@ -470,27 +470,28 @@ def restore_ip_chain(esh_driver, esh_instance, deploy=True,
     start with: task.apply_async()
     """
     from service.tasks.driver import (
-        wait_for_instance, restore_port, add_floating_ip,
+        wait_for_instance, add_fixed_ip, add_floating_ip,
         deploy_init_to, update_metadata
     )
     init_task = wait_for_instance.s(
         esh_instance.id, esh_driver.__class__, esh_driver.provider,
         esh_driver.identity, "active",
         # TODO: DELETEME below.
         no_tasks=True)
-
+    # Step 1: Set metadata to initializing
     metadata = {'tmp_status': 'initializing'}
     metadata_update_task = update_metadata.si(
         esh_driver.__class__, esh_driver.provider, esh_driver.identity,
         esh_instance.id, metadata, replace_metadata=False)
 
-    restore_port_task = restore_port.si(
+    # Step 2: Add fixed
+    fixed_ip_task = add_fixed_ip.si(
         esh_driver.__class__, esh_driver.provider,
         esh_driver.identity, esh_instance.id, core_identity_uuid)
 
     init_task.link(metadata_update_task)
-    metadata_update_task.link(restore_port_task)
-
+    metadata_update_task.link(fixed_ip_task)
+    # Add float and re-deploy OR just add floating IP...
     if deploy:
         core_identity = CoreIdentity.objects.get(uuid=core_identity_uuid)
         deploy_task = deploy_init_to.si(
@@ -500,16 +501,16 @@ def restore_ip_chain(esh_driver, esh_instance, deploy=True,
             esh_instance.id,
             core_identity,
             redeploy=True)
-        restore_port_task.link(deploy_task)
+        fixed_ip_task.link(deploy_task)
     else:
+        logger.info("Skip deployment, Add floating IP only")
         floating_ip_task = add_floating_ip.si(
             esh_driver.__class__,
             esh_driver.provider,
             esh_driver.identity,
             str(core_identity_uuid),
             esh_instance.id)
-        restore_port_task.link(floating_ip_task)
-
+        fixed_ip_task.link(floating_ip_task)
     return init_task
 
 

service/tasks/driver.py

@@ -168,35 +168,61 @@ def _is_instance_ready(instance, status_query,
         return instance.id
     return True
 
-@task(name="restore_port",
+
+@task(name="add_fixed_ip",
       ignore_result=True,
       default_retry_delay=15,
-      max_retries=5,
-      bind=True)
-def restore_port(self,
-                 driverCls,
-                 provider,
-                 identity,
-                 instance_id,
-                 core_identity_uuid=None):
+      max_retries=15)
+def add_fixed_ip(
+        driverCls,
+        provider,
+        identity,
+        instance_id,
+        core_identity_uuid=None):
     from service.instance import _to_network_driver, _get_network_id
     try:
-        driver = get_driver(driverCls, provider, identity)
+        celery_logger.debug("add_fixed_ip task started at %s." % datetime.now())
         core_identity = Identity.objects.get(uuid=core_identity_uuid)
         network_driver = _to_network_driver(core_identity)
+        driver = get_driver(driverCls, provider, identity)
         instance = driver.get_instance(instance_id)
-        assert instance, "Instance {} no longer exists".format(instance_id)
+        if not instance:
+            celery_logger.debug("Instance has been teminated: %s." % instance_id)
+            return None
 
         ports = network_driver.list_ports(device_id=instance.id)
-        for port in ports:
-            network_driver.delete_port(port)
+        # Catch a common scenario that breaks networking
+        assert len(ports) == 1, "Attaching a fixed ip requires a single port"
+        port = ports[0]
+        port_zone = instance_zone = None
+        try:
+            port_zone = port['device_owner'].split(":")[1]
+            instance_zone = instance.extra['availability_zone']
+        except:
+            pass
 
         network_id = _get_network_id(driver, instance)
-        driver._connection.ex_attach_interface(
-            instance.id, network_id=network_id)
+
+        if port_zone and instance_zone and port_zone != instance_zone:
+            # If the port and instance are in different zones, delete the old
+            # port and attach a new one, this only occurs in narrow scenarios
+            # documented in the following ticket:
+            # https://bugs.launchpad.net/nova/+bug/1759924
+            network_driver.delete_port(port)
+            driver._connection.ex_attach_interface(
+                instance.id, network_id=network_id)
+
+        elif not instance._node.private_ips:
+            # Only add fixed ip if the instance doesn't already have one
+            driver._connection.ex_add_fixed_ip(instance, network_id)
+
+        celery_logger.debug("add_fixed_ip task finished at %s." % datetime.now())
     except Exception as exc:
-        celery_logger.exception(exc)
-        self.retry(exc=exc)
+        if "Not Ready" not in str(exc):
+            # Ignore 'normal' errors.
+            celery_logger.exception(exc)
+        add_fixed_ip.retry(exc=exc)
+
 
 def current_openstack_identities():
     identities = Identity.objects.filter(