Retry on conflicts when generating a random image name

[mtrmac]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Eliminate the risk of possibly generating a conflicting image name.

How to verify it

No non-invasive way.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtrmac
To complete the pull request process, please assign umohnani8 after the PR has been reviewed.
You can assign the PR to them by writing /assign @umohnani8 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mtrmac]

Fair point, I should actually write a test for the name collision check at least.

[vrothberg]

I think there is a race between checking if the name is free and creating the image with this specific name. We recently had a similar issue in Podman when assigning random names to containers.

[mtrmac]

Oops, you’re right, this is racy anyway; we would need to pre-allocate the name in a way that persistently records that cross-process, which this code does not do.

So, this would make the race smaller but not actually eliminate it, just adding false sense of security and potentially confusing readers of this code. Let’s not do that.

I don’t see how to truly fix this at this point, so I guess we’ll have to hope the 256-bit ID namespace is large enough :)

[vrothberg]

Maybe we can find a similar solution here. What happened in Podman is loop around CreateContainer and do another round if the error indicates a name conflict with an existing container.

But I am not sure if that would work with images.

[mtrmac]

It probably would work, it’s just so ugly and expensive. (We create the image with a deterministic ID, typically using the config digest; all of that would succeed, only a final Store.SetNames with the conflicting name would fail — and we would tear down the whole image (re-creating squashed layers in the worst case?), and try again.)

[mtrmac]

Strictly speaking the user owns the namespace, so a collision is always possible. Pragmatically, maybe some combination of (container ID, timestamp) could be unique enough. But then the current code uses 256 bits of true randomness (unlike most of Podman, which uses the deterministic PRNG), which is at least as unique.