lxd: Resolve the intermittent "file already closed" errors in distributeImage#15674
Merged
tomponline merged 9 commits intocanonical:mainfrom Jun 3, 2025
Merged
lxd: Resolve the intermittent "file already closed" errors in distributeImage#15674tomponline merged 9 commits intocanonical:mainfrom
distributeImage#15674tomponline merged 9 commits intocanonical:mainfrom
Conversation
a98e6e9 to
628063e
Compare
simondeziel
reviewed
May 29, 2025
Member
There was a problem hiding this comment.
I like it but it'd be best to ask someone else to review the Go part. It certainly read better to my untrained eyes :)
tomponline
added 5 commits
June 3, 2025 08:32
…ributeImage Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
… in distributeImage Allows removal of reverter package usage and simplifies file closing logic. With the added benefit that image file handles are closed at the end of each member update. Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
628063e to
d7763b5
Compare
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
…age file distribution Signed-off-by: Thomas Parrott <thomas.parrott@canonical.com>
distributeImagedistributeImage
Member
|
I am really happy that you've been able to put this bug to bed for good (fingers crossed)! Thanks |
Member
|
Sigh, seems that it didn't fully fix the problem: https://github.com/canonical/lxd/actions/runs/15428110615/job/43420377825?pr=15711#step:15:32370 |
Member
Author
Ill take a look. Its a different error than before: |
Member
Author
|
I've identified the issue i think. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
By switching to a per-cluster member local function with defer for closing file handles, rather than bundling them up until the end of the overall function using the reverter (its cleaner too).
Also improves logging and extends image auto update tests to check for correct image file distribution (with comments justifying why LXD has the behaviour it has).