Add lease and remove special case#2405
Conversation
Without adding a lease, the containerd GC will remove the view we just created, while we're attempting to use it, leading to the Mount() function erring out. The lease ensures that the view will not be reaped prematurely. Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
|
CC: @AkihiroSuda |
|
The failure seems to be in |
|
The |
|
Thank you, could you try running CI with containerd v1.7.3? Line 36 in b33a58f |
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
|
CI seems happy on 1.7.3 edit: Well, at least the Windows CI 😄 . The failures do not seem related to this change. |
| // When the Unmount fails, RemoveAll will incorrectly delete data from the mounted dir | ||
| defer os.Remove(tempDir) | ||
|
|
||
| // Add a lease of 1 hour to the view so that it is not garbage collected |
There was a problem hiding this comment.
@AkihiroSuda I added a 1 hour lease. Let me know if you prefer a different value.
|
Not sure if relevant, but this one is now consistently failing |
|
I don't think it was triggered by this change. It seems to be a networking issue, which I have seen on ltsc2019, but does not happen on ltsc2022. Does the CI also run on ltsc2022? Are you seeing it there as well? |
|
Currently the Cirrus CI only runs on "windows-2019-core-for-containers" Maybe we should migrate to GitHub Actions? |
This change adds a lease to the view creation. Without a lease, there is a high probability that the newly created view will be removed by the containerd garbage collector shortly after we create the view and before we get a chance to use it.
Depending on when the GC runs during the call to
m.Mount(), we may get a set of different errors (at least on Windows) ranging fromThe system cannot find the path specifiedtoThe parameter is incorrecttoAccess is denied.Adding a lease prevents containerd from removing the view mid-flight.
This change also adds a drive-by fix. The special case for Windows when mounting the view, was removed.
Fixes containerd/containerd#8891