[Bug]: Possible race in implicit volume creation (`Utility.getOrCreateVolume()`)

[realrajaryan]

I have done the following

• I have searched the existing issues
• If possible, I've reproduced the issue using the 'main' branch of this project

Steps to reproduce

The current implicit volume creation logic in Utility.getOrCreateVolume() has a race condition in the create or inspect pattern:

do {
    volume = try await ClientVolume.create(...)
} catch let error as VolumeError {
    guard case .volumeAlreadyExists = error else { throw error }
    volume = try await ClientVolume.inspect(parsed.name)
}

Current behavior

1. Container A calls getOrCreateVolume(“vol”) which attempts to create a volume
2. Container B calls getOrCreateVolume(“vol”) which also attempts to create a volume
3. Container A’s create succeeds
4. Container B’s create fails with volumeAlreadyExists
5. Container B calls inspect() to get the volume
6. But between steps 3 and 5, if Container A’s container fails and is cleaned up along with its volume,
Container B’s inspect() will fail with volumeNotFound

Expected behavior

In practice the race window is small. What I added in ContainersService is a precedent that solves similar races using withContainerList() (see ContainersService.swift#L122), where we do atomic check-and-create operations within a single lock context.

Environment

N/A

Relevant log output

N/A

Code of Conduct

• I agree to follow this project's Code of Conduct

[realrajaryan]

We could simply just add an atomic getOrCreateVolume method to VolumesService that performs the check-and-create operation within a single lock

[jglogan]

Idea here is to have a discussion around this for all our managed resources.

There's the problem of coordinating compound operations within the API service which is what the withContainerList() call in ContainersService does.

Get-or-create semantics and atomicity at the client level is a little different.

As you say, we could create a new API. We could extend the create API with an option that returns success if the volume already exists (like mkdir -p).

Or we could decide that races at this level are okay and it's the caller's responsibility to deal with it!

Nothing we have to rush to solve, as you say, the window is small and would typically only be hit in odd automation cases (e.g. chaos testing).