Merge pull request #843 from shiftstack/fix-attachment-race

Fix potential race in interface/volume status update

Author: mandre

internal/controllers/port/actuator.go

@@ -380,9 +380,11 @@ func (actuator portActuator) checkAttachedServer(ctx context.Context, obj orcObj
 	}
 
 	// Find server with matching ID
+	found := false
 	for i := range serverList.Items {
 		server := &serverList.Items[i]
 		if server.Status.ID != nil && *server.Status.ID == osResource.DeviceID {
+			found = true
 			// Check if server is in BUILD status
 			if server.Status.Resource != nil && server.Status.Resource.Status == "BUILD" {
 				log.V(logging.Verbose).Info("Port is attached to server in BUILD status, waiting",
@@ -396,6 +398,23 @@ func (actuator portActuator) checkAttachedServer(ctx context.Context, obj orcObj
 		}
 	}
 
+	// When the port is attached to a device but still reports DOWN,
+	// the Neutron status has not yet transitioned to ACTIVE (e.g.
+	// OVN is still binding the port). serverToPortMapFunc also
+	// detects this and triggers a reconcile, but it races with the
+	// port controller's own status write: the controller may
+	// overwrite Progressing=True with Progressing=False before the
+	// port becomes ACTIVE. Poll here so we keep Progressing=True
+	// until the transition completes. Only do this when we found
+	// the ORC Server object the port is attached to, to avoid
+	// unnecessary polling when the device isn't a tracked server.
+	if found && osResource.Status == PortStatusDown {
+		log.V(logging.Verbose).Info("port needs reconciliation: attached to server but status is DOWN",
+			"port", obj.Name,
+			"status", osResource.Status)
+		return progress.WaitingOnOpenStack(progress.WaitingOnReady, serverBuildPollingPeriod)
+	}
+
 	return nil
 }
 

internal/controllers/volume/actuator.go

@@ -283,10 +283,31 @@ func handleDescriptionUpdate(updateOpts *volumes.UpdateOpts, resource *resourceS
 
 func (actuator volumeActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) {
 	return []resourceReconciler{
+		actuator.checkAttachmentStatus,
 		actuator.updateResource,
 	}, nil
 }
 
+func (volumeActuator) checkAttachmentStatus(ctx context.Context, _ orcObjectPT, osResource *osResourceT) progress.ReconcileStatus {
+	log := ctrl.LoggerFrom(ctx)
+
+	// When the volume has attachments but Cinder still reports
+	// "available" rather than "in-use", the status transition has
+	// not completed yet. serverToVolumeMapFunc also detects this
+	// and triggers a reconcile, but it races with the volume
+	// controller's own status write: the controller may overwrite
+	// Progressing=True with Progressing=False before the volume
+	// becomes in-use. Poll here so we keep Progressing=True until
+	// the transition completes.
+	if len(osResource.Attachments) > 0 && osResource.Status != VolumeStatusInUse {
+		log.V(logging.Verbose).Info("volume needs reconciliation: attached to server but status is not in-use",
+			"status", osResource.Status)
+		return progress.WaitingOnOpenStack(progress.WaitingOnReady, volumeAvailablePollingPeriod)
+	}
+
+	return nil
+}
+
 type volumeHelperFactory struct{}
 
 var _ helperFactory = volumeHelperFactory{}