Pass the current snapshotter to the buildkit worker#52
Conversation
If buildkit uses a different snapshotter we can't list the images any more because we can't find the snapshot. Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
left one suggestion (for GoDoc), and a question that's probably not an issue
| @@ -78,6 +78,7 @@ type Opt struct { | |||
| DNSConfig config.DNSConfig | |||
| ApparmorProfile string | |||
| UseSnapshotter bool | |||
There was a problem hiding this comment.
Surprised that the linter didn't trip over these not being documented (🤷♂️ )
If we do want to add GoDoc, perhaps we should describe this as the default snapshotter that's used (with in mind that these may be overridable?)
| UseSnapshotter: d.UsesSnapshotter(), | ||
| Snapshotter: d.ImageService().GraphDriverName(), |
There was a problem hiding this comment.
If d.UsesSnapshotter() == false we still set Snapshotter, looks like that can't do any harm, correct?
(related to that; if we would set Snapshotter conditionally, we may not need the UseSnapshotter, as it would be the same as if Snapshotter != "").
There was a problem hiding this comment.
Yeah, the Snapshotter is not used if d.UsesSnapshotter() == false. I agree it's kinda weird passing something that could or not be used. I'm not sure we should have put two different things in the same place (current graph driver vs current snapshotter).
If buildkit uses a different snapshotter we can't list the images any
more because we can't find the snapshot.