Skip to content

Commit 6e60347

Browse files
authored
Merge pull request #878 from k-orc/bp-release-2.0-e02d662
[release-2.0] Fix potential race in interface/volume status update
2 parents 99cae8d + d083c73 commit 6e60347

2 files changed

Lines changed: 40 additions & 0 deletions

File tree

internal/controllers/port/actuator.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,9 +373,11 @@ func (actuator portActuator) checkAttachedServer(ctx context.Context, obj orcObj
373373
}
374374

375375
// Find server with matching ID
376+
found := false
376377
for i := range serverList.Items {
377378
server := &serverList.Items[i]
378379
if server.Status.ID != nil && *server.Status.ID == osResource.DeviceID {
380+
found = true
379381
// Check if server is in BUILD status
380382
if server.Status.Resource != nil && server.Status.Resource.Status == "BUILD" {
381383
log.V(logging.Verbose).Info("Port is attached to server in BUILD status, waiting",
@@ -389,6 +391,23 @@ func (actuator portActuator) checkAttachedServer(ctx context.Context, obj orcObj
389391
}
390392
}
391393

394+
// When the port is attached to a device but still reports DOWN,
395+
// the Neutron status has not yet transitioned to ACTIVE (e.g.
396+
// OVN is still binding the port). serverToPortMapFunc also
397+
// detects this and triggers a reconcile, but it races with the
398+
// port controller's own status write: the controller may
399+
// overwrite Progressing=True with Progressing=False before the
400+
// port becomes ACTIVE. Poll here so we keep Progressing=True
401+
// until the transition completes. Only do this when we found
402+
// the ORC Server object the port is attached to, to avoid
403+
// unnecessary polling when the device isn't a tracked server.
404+
if found && osResource.Status == PortStatusDown {
405+
log.V(logging.Verbose).Info("port needs reconciliation: attached to server but status is DOWN",
406+
"port", obj.Name,
407+
"status", osResource.Status)
408+
return progress.WaitingOnOpenStack(progress.WaitingOnReady, serverBuildPollingPeriod)
409+
}
410+
392411
return nil
393412
}
394413

internal/controllers/volume/actuator.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,31 @@ func handleDescriptionUpdate(updateOpts *volumes.UpdateOpts, resource *resourceS
287287

288288
func (actuator volumeActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) {
289289
return []resourceReconciler{
290+
actuator.checkAttachmentStatus,
290291
actuator.updateResource,
291292
}, nil
292293
}
293294

295+
func (volumeActuator) checkAttachmentStatus(ctx context.Context, _ orcObjectPT, osResource *osResourceT) progress.ReconcileStatus {
296+
log := ctrl.LoggerFrom(ctx)
297+
298+
// When the volume has attachments but Cinder still reports
299+
// "available" rather than "in-use", the status transition has
300+
// not completed yet. serverToVolumeMapFunc also detects this
301+
// and triggers a reconcile, but it races with the volume
302+
// controller's own status write: the controller may overwrite
303+
// Progressing=True with Progressing=False before the volume
304+
// becomes in-use. Poll here so we keep Progressing=True until
305+
// the transition completes.
306+
if len(osResource.Attachments) > 0 && osResource.Status != VolumeStatusInUse {
307+
log.V(logging.Verbose).Info("volume needs reconciliation: attached to server but status is not in-use",
308+
"status", osResource.Status)
309+
return progress.WaitingOnOpenStack(progress.WaitingOnReady, volumeAvailablePollingPeriod)
310+
}
311+
312+
return nil
313+
}
314+
294315
type volumeHelperFactory struct{}
295316

296317
var _ helperFactory = volumeHelperFactory{}

0 commit comments

Comments
 (0)