-
Notifications
You must be signed in to change notification settings - Fork 41
Conversation
Codecov Report
@@ Coverage Diff @@
## main #101 +/- ##
==========================================
+ Coverage 9.03% 63.82% +54.79%
==========================================
Files 39 40 +1
Lines 2667 2737 +70
==========================================
+ Hits 241 1747 +1506
+ Misses 2399 779 -1620
- Partials 27 211 +184
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
1c6a7b4
to
d1a9fa4
Compare
Recent versions of k8s and/or containerd get stuck for about a minute with the failed docker.sock mount which causes problems. This attempts to clean up things faster by explicitly deleting the pods that will never start.
if err != nil { | ||
return err | ||
} | ||
clients[drivers[i].Name][chosenNodeName] = c | ||
clients[drivers[i].Name][builderClients.ChosenNode.NodeName] = builderClients.ChosenNode.BuildKitClient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could run into a race here unless you use sync.Mutex and lock/unlock it around the clients[] hashmap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch! You're right, this was unsafe usage.
// This most likely means another build is running in parallel | ||
// Give it just enough time to finish creating resources then retry | ||
time.Sleep(25 * time.Millisecond * time.Duration(1+rand.Int63n(39))) // 25 - 1000 ms | ||
}); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes me a bit nervous. I get it might take a bit for the builders to simmer down, but just sleeping randomly for 1000 milliseconds feels a bit weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. Let me drop this down to a much smaller pause to get the racing CLIs skewed but not as noticeable to a human.
No description provided.