Conversation
Signed-off-by: Morlay <[email protected]>
|
|
||
| eg, ctx := errgroup.WithContext(ctx) | ||
| for i, c := range clients { | ||
| if c == nil { |
There was a problem hiding this comment.
Fix looks ok to address the issue, but perhaps we should also look into why this slice contains nil values in the first place, @crazy-max ?
There was a problem hiding this comment.
Multi-node build support and the underlying clients iteration was first introduced while resolving drivers (#37). Client nil check was already there
Lines 283 to 285 in 7d312ea
but was left out to be included in #535.
but perhaps we should also look into why this slice contains nil values in the first place,
Yes agree, might be an issue with the context dialer while creating the buildkit client for docker-container driver. Will take a look.
There was a problem hiding this comment.
Was initially thinking, perhaps the slice is initialized with a length/size (not taking into account that filtering might happen). Haven't looked at the code though (was reading from my phone), but though it wouldn't hurt to leave a comment
|
This does not look correct and iiuc can return cleanly on error cases as skip over the builds. We should first figure out why this happens. Probably the correct solution is to return an error. If skipping over a node is determined to be better it should be in |
5 participants
maybe fix #675
I have another issue
With multi-node builder, but single platform build
buildx build --platform=linux/amd64, will cause the nil issue too