daemon: require storage-driver to be set if the driver is deprecated #43378
Conversation
|
@tianon @cpuguy83 @tonistiigi @samuelkarp @corhere PTAL; let me know if you agree with this approach This makes the deprecated drivers opt-in, even if they were previously used. Before this we would "silently" continue to use them (apart from printing a warning in logs and in The warnings will still be printed in both places after this (and if the user opted-in to use the deprecated driver) |
daemon/graphdriver/driver.go
Outdated
| @@ -220,34 +217,45 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err | ||
| logrus.Errorf("[graphdriver] prior storage driver %s failed: %s", name, err) | ||
| return nil, err | ||
| } | ||
| if isDeprecated(name) { | ||
| err = errors.Errorf("prior storage driver %s is deprecated and will be removed in the next release; update the the daemon configuration and explicitly choose this storage driver to continue using it", name) |
There was a problem hiding this comment.
I know this isn't complicated to set, but if a user isn't familiar with it already it's a bit esoteric to figure out exactly where to set this -- do we have a good document we could have this error point users to when their daemon won't start?
I guess the opposite argument is that we don't want to encourage these users to continue using their current storage driver (that's kind of the whole point of this error), so perhaps making them do some basic work to figure out how is appropriate?
There was a problem hiding this comment.
So, erm, yes, I actually went looking for a good location (and was thinking to add an easy-to-type /go/<something> URL for that, e.g. https://docs.docker.com/go/storage-driver); the point was that when I looked at https://docs.docker.com/storage/storagedriver/select-storage-driver/, I noticed there's a wall of text about storage drivers, but that page itself doesn't show how to configure which driver to use. For that, you'd have to go to the docs for one of the individual drivers, e.g. https://docs.docker.com/storage/storagedriver/overlayfs-driver/, which outlines how to set the daemon.json etc.
Perhaps I should still consider that though. I wanted to have "something" up at least (so that it would be included in the next release), but I guess that's not there yet, so I could still tweak it (either in this PR, or as a follow-up once I have the documentation a bit improved)
There was a problem hiding this comment.
There's a balancing act to be had w.r.t. actionability of the error message. The kinds of users who we most want to pay attention to the deprecation error are also the kinds of users who are most likely [citation needed] to pay no attention to what the error says and do whatever needs to be done to Just Make The Error Go Away Already. If the error message is directly actionable (i.e. "add "storage-driver": "overlay" to your daemon.json") they will act on that and forget about the important part. On the other extreme, if the user has to dig around for documentation on how to update the daemon configuration and explicitly choose a storage driver, because the error message does not contain a doc link or links to documentation which is not directly actionable, they're not going to bother to dig around and learn anything. They'll just paste the error message into Google and follow by rote whatever StackOverflow answer or Joe Blogspam's post says to do to get their daemon booting again—with the deprecated driver. Then they'll inevitably get angry when the deprecated driver is removed and they're left high and dry "because I never got any advance warning!"
IMO we need to control the messaging from the get-go: that is, link from the error message to good and actionable official documentation which details both how to migrate from overlay/aufs/devicemapper to overlay2 and how to opt into continuing to use the deprecated drivers. Make the path of least resistance lead to our messaging and we'll have a better chance at getting through to the people who most need it, and keep them away from third-party sources of information which are likely to gloss over the consequences of continuing to use the deprecated drivers.
There was a problem hiding this comment.
Yes, it is more convenient to let users jump to a clear actionable document
There was a problem hiding this comment.
I opened a PR in the documentation repository to add a (placeholder) https://docs.docker.com/go/storage-driver/ URL docker/docs#14435
We can update that redirect to point to a specific section once we have added more details for that; I'll update this PR to include that URL in the deprecation message(s).
480d343 to
548ca84
Compare
- use pkg/errors for errors and fix error-capitalisation - remove one redundant call to logDeprecatedWarning() (we're already skipping deprecated drivers in that loop). - rename `list` to `priorityList` for readability. - remove redundant "skip" for the vfs storage driver, as it's already excluded by `scanPriorDrivers()` - change one debug log to an "info", so that the daemon logs contain the driver that was configured, and include "multiple prior states found" error in the daemon logs, to assist in debugging failed daemon starts. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, we only printed a warning if a storage driver was deprecated. The
intent was to continue supporting these drivers, to allow users to migrate
to a different storage driver.
This patch changes the behavior; if the user has no storage driver specified
in the daemon configuration (so if we try to detect the previous storage
driver based on what's present in /var/lib/docker), we now produce an error,
informing the user that the storage driver is deprecated (and to be removed),
as well as instructing them to change the daemon configuration to explicitly
select the storage driver (to allow them to migrate).
This should make the deprecation more visible; this will be disruptive, but
it's better to have the failure happening *now* (while the drivers are still
there), than for users to discover the storage driver is no longer there
(which would require them to *downgrade* the daemon in order to migrate
to a different driver).
With this change, `docker info` includes a link in the warnings that:
/ # docker info
Client:
Context: default
Debug Mode: false
Server:
...
Live Restore Enabled: false
WARNING: The overlay storage-driver is deprecated, and will be removed in a future release.
Refer to the documentation for more information: https://docs.docker.com/go/storage-driver/
When starting the daemon without a storage driver configured explicitly, but
previous state was using a deprecated driver, the error is both logged and
printed:
...
ERRO[2022-03-25T14:14:06.032014013Z] [graphdriver] prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information
...
failed to start daemon: error initializing graphdriver: prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information
When starting the daemon and explicitly configuring it with a deprecated storage
driver:
WARN[2022-03-25T14:15:59.042335412Z] [graphdriver] WARNING: the overlay storage-driver is deprecated and will be removed in a future release; visit https://docs.docker.com/go/storage-driver/ for more information
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
548ca84 to
3853eb5
Compare
|
@corhere @tao12345666333 @tianon I updated the warnings and errors to include a link to the documentation (will have a look at making the docs more useful w.r.t migrating and configuring, but I guess that doesn't have to block this change itself) |
|
Thanks all! Let me bring this one in |
relates to:
daemon: graphdriver: some minor cleanup
listtopriorityListfor readability.scanPriorDrivers()daemon: require storage-driver to be set if the driver is deprecated
Previously, we only printed a warning if a storage driver was deprecated. The
intent was to continue supporting these drivers, to allow users to migrate
to a different storage driver.
This patch changes the behavior; if the user has no storage driver specified
in the daemon configuration (so if we try to detect the previous storage
driver based on what's present in /var/lib/docker), we now produce an error,
informing the user that the storage driver is deprecated (and to be removed),
as well as instructing them to change the daemon configuration to explicitly
select the storage driver (to allow them to migrate).
This should make the deprecation more visible; this will be disruptive, but
it's better to have the failure happening now (while the drivers are still
there), than for users to discover the storage driver is no longer there
(which would require them to downgrade the daemon in order to migrate
to a different driver).
With this change,
docker infoincludes a link in the warnings that:When starting the daemon without a storage driver configured explicitly, but
previous state was using a deprecated driver, the error is both logged and
printed:
When starting the daemon and explicitly configuring it with a deprecated storage
driver:
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)