daemon/containerd: some fixes and enhancements#46320
Conversation
thaJeztah
added
status/2-code-review
area/images
| } | ||
| defer done(ctx) | ||
| defer func() { | ||
| if err := release(ctx); err != nil && !cerrdefs.IsNotFound(err) { |
There was a problem hiding this comment.
If we are going to log this then we shouldn't filter the not found error since this is something that shouldn't happen because this is a lease that we created
There was a problem hiding this comment.
Good point, so I adopted this from some existing code that did the same. My assumption was that the existing code I found did this because these could theoretically be racy; the lease was created with leases.WithExpiration, which I assumed means it could be deleted out-of-band.
If that's the wrong assumption, I'll remove 😅 (let me know!)
There was a problem hiding this comment.
Yeah I would prefer if we logged them regardless of the error
There was a problem hiding this comment.
Alright; let me update 👍
Log a warning if we encounter an error when releasing leases. While it may not have direct consequences, failing to release the lease should be unexpected, so let's make them visible. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Also rename variables that collided with imports. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
c8d_log_release_fails
branch
from
1210b47 to
e10eca3
Compare
|
@rumpl updated; PTAL |
2 participants
daemon/containerd: log errors when releasing leases
Log a warning if we encounter an error when releasing leases. While it
may not have direct consequences, failing to release the lease should be
unexpected, so let's make them visible.
daemon/containerd: newROLayerForImage: remove unused args
Also rename variables that collided with imports.
daemon/containerd: rename some vars that collided with imports
- A picture of a cute animal (not mandatory but encouraged)