As the guy who first used the term "graph" in this context (because of the git-like graph relationship between layers), I agree that it's confusing and should be deprecated.

As long as this introduces absolutely no interface regression, I am in favor.

According to the docs include in this PR, the milestone of this PR seems 1.14.

However, in 1.13, GraphDriver plugin will be moved to out of experimental.
So, we need to decide whether we rename GraphDriver as well or not.
#28623

I'm +1 for renaming GraphDriver, but I don't think we should call it "DataRootDriver"...
Perhaps --storage-driver and "StorageDriver"?

PTAL @cpuguy83

So, we need to decide whether we rename GraphDriver as well or not.

Oh, I've been wanting to do that for about 2 years: #9758 but we never got around to doing it. There is also already an existing --storage-driver flag for dockerd:

  -s, --storage-driver string                 Storage driver to use

I'm +1 for renaming GraphDriver, but I don't think we should call it "DataRootDriver"...
Perhaps --storage-driver and "StorageDriver"?

I think these are two distinct things: --data-root just specifies where the directory is. Your choice of copy-on-write image/container filesystem storage driver is independent of that. The same directory will contain subdirectories for containers, image, network, swarm, tmp, volumes, and whatever the name of the storage driver is that you choose.

@AkihiroSuda @jlhawn it was attempted twice; #22228 and #23237, but there were issues on Windows. Maybe they are resolved in the newer Windows versions.

SGTM

Ok for me, but I don't think we could ever actually remove --graph/-g

@jlhawn can you keep the old flags around as a hidden flag, in addition to the current one?

Also, the daemon.json should probably support the old name as well (but we can issue a warning, and deprecated that, so you need to add that to the deprecated.md)

can you keep the old flags around as a hidden flag, in addition to the current one?
Also, the daemon.json should probably support the old name as well (but we can issue a warning, and deprecated that, so you need to add that to the deprecated.md)

Is this not already covered by this: https://github.com/docker/docker/pull/28696/files#diff-a348195a1b645d5b477946b1efae728bR173 ? Also the graph option in the config file corresponds to the command line option so you should get a warning for that as well right?

ping @thaJeztah

@jlhawn sorry, yes, you're right about the hidden flag. w.r.t. removing the flag, given that this flag has been around since docker < 1.0, we may need to decide to keep it around, and not actually remove it, but open to suggestions there.

There's a think that needs to be addressed in this PR; you forgot to update the name in the daemon.json file; https://github.com/docker/docker/blob/d6be0e98027611cfb14a3246ca797bee0936e649/daemon/config.go#L99

To allow both the old and the new name to work; possibly we need to have an additional property, that's mapped to the old (graph) name in the daemon.json. If a user uses that option, a deprecation warning should be printed.

In addition; the deprecation warning is currently printed on the commandline. Given that daemon options would usually be set in either a systemd unit file, or through daemon.json, I think we should actually log the error, otherwise people will never see the deprecation message

@thaJeztah I can't quite wrap my head around the code which handles the config file and command line arguments (theres a bunch of stuff which handles resolving conflicts and setting defaults when both are used). Is there an existing example of how to deprecate something from the config file? The only other daemon flag I see which is marked as deprecated is the --restart flag which doesn't have a corresponding config file JSON tag.

I think we should actually log the error, otherwise people will never see the deprecation message

The flag library being used will print the deprecation warning to stderr. What is different about manually logging it (besides the formatting)?

The more I look at it, the more it looks like I might have to "invent" a way of marking and handling deprecated config file options.

Yes, I think this is a first for config-file deprecation. It will probably mean something like;

type CommonConfig struct {
	RootDeprecated     string              `json:"graph,omitempty"`
	Root               string              `json:"data-dir,omitempty"`
}

• if the "old" (graph) is set; print deprecation warning
• if both "old" and "new" are set; produce error and fail

w.r.t. both "flag" and "config file"; the current behavior is that if both a flag and the same option in the config file are set, that an error is shown.

ok, updated after discussing with @vdemeester, and changed to hide the flag instead of printing a deprecation warning, and only log the warning. I think this is the least disruptive approach, and still discourages using the old flag, but happy to change back if we think otherwise

ok, discussed in the maintainers meeting, and there were no objections to this approach, so should be ready to go 😄

Ok then, let's be crazy and merge it 👼

Thanks @jlhawn !

Although deprecated - is this a breaking change? There are lots of tutorials etc on the internet that advise on using -g to specify a different path/disk for the Docker data area.

@alexellis the flags are hidden not removed. It won't break anyone.

These flags were added before Docker 1.0, so will not be removed, only
hidden, to discourage their use.