-
Notifications
You must be signed in to change notification settings - Fork 18.9k
ImageService: rename GraphDriverName to StorageDriver, remove daemon.graphdriver #43982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ImageService: rename GraphDriverName to StorageDriver, remove daemon.graphdriver #43982
Conversation
Make the function name more generic, as it's no longer used only for graphdrivers but also for snapshotters. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Make the ImageService the source of truth for the storage-driver that's used. Signed-off-by: Sebastiaan van Stijn <[email protected]>
| // StorageDriver returns the name of the default storage-driver (snapshotter) | ||
| // used by the ImageService. | ||
| func (i *ImageService) StorageDriver() string { | ||
| return "" |
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.
currently always empty, but will be propagated once rumpl#40 is upstreamed
It was only used as an intermediate variable to store what's returned by layerstore.DriverName() / ImageService.StorageDriver() Signed-off-by: Sebastiaan van Stijn <[email protected]>
a4be51d to
d2276ff
Compare
| // As layerstore initialization may set the driver | ||
| d.graphDriver = layerStore.DriverName() | ||
|
|
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.
With this code removed, what happens if the layer store sets the driver? Is it moved to the image service storage driver?
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.
For the old (graph driver) image store, ImageService.StorageDriver() is just a proxy for layerstore.DriverName(), so the layerstore remains the source of truth;
Lines 178 to 180 in 6f8ea5b
| func (i *ImageService) GraphDriverName() string { | |
| return i.layerStore.DriverName() | |
| } |
And for snapshotters, it will use the snapshotter name (see https://github.com/moby/moby/pull/43982/files#r948993572 above)
rumpl
left a comment
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.
LGTM
ImageService: rename GraphDriverName to StorageDriver
Make the function name more generic, as it's no longer used only for graphdrivers but also for snapshotters.
daemon: info: fillDriverInfo() get driver-name from ImageService
Make the ImageService the source of truth for the storage-driver that's used.
daemon: remove daemon.graphdriver
It was only used as an intermediate variable to store what's returned
by layerstore.DriverName() / ImageService.StorageDriver()
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)