Repository navigation
Service update ignores existing log-driver when setting log-opt #1942
Description
as reported in docker/docs#8745 by @Tomcat-Engineering
When updating a service (docker service update), any log-opt specified is ignored unless a log-driver is also set when updating.
this looks to be a combination of something that's by design, and (what I think is) a bug in the docker service update logic; docker service create and update should follow slightly different logic (and I think they currently use the same).
- expected: setting
log-optfor a service that does not have a logging-driver configured isn't supported- reason for this is Docker ignores --log-opt (1.12.6) moby/moby#31515 (comment) that in a swarm setup, all instances of a service should use the same configuration (including logging-driver), so when specifying options, they must be accompanied with the driver
- t.b.d.: creating a service with a log-opt set, but no logging driver silently ignores the option
- should it produce an error? a warning?
- bug: setting
log-optfor a service that has a logging driver configured (when created), but without specifying that driver during update is silently ignored.- when updating a service, the CLI should take the existing logging-driver, and update the options
Taking a full example;
docker service create --log-driver=json-file --log-opt max-file=5 --name loggie nginx:alpine
docker service inspect --format='{{json .Spec.TaskTemplate.LogDriver}}' loggie
{"Name":"json-file","Options":{"max-file":"5"}}Updating without logging-driver set doesn't take effect. IMO this should work, given that the service already had a logging-driver configured. It should still fail (or warn) if no logging-driver was configured yet on the service.
docker service update --log-opt max-file=15 loggie
docker service inspect --format='{{json .Spec.TaskTemplate.LogDriver}}' loggie
{"Name":"json-file","Options":{"max-file":"5"}}Updating with a logging driver specified does;
docker service update --log-driver=json-file --log-opt max-file=15 loggie
docker service inspect --format='{{json .Spec.TaskTemplate.LogDriver}}' loggie
{"Name":"json-file","Options":{"max-file":"15"}}We should fix the update case:
- if a logging-driver is set on the service, keep that driver and set the log-opts
- if no logging-driver is set, and none is specified when updating; produce a warning (or error)
- this validation should be handled daemon-side
- when creating a service; produce a warning (or error) if a log-opt is set, but no log-driver
- this validation should be handled daemon-side
Reacted by Mehdi, Alvise and Federico Bellini
Activity
adeniyistephen commentedon Nov 23, 2020
@thaJeztah Hi, I would love to work on this, Please provide pointers and input on how to work on it. Thanks
thaJeztah commentedon Nov 24, 2020
Yes, feel free to work on this. I think some of the fixes can be handled in the client (but additional fixes may be needed on the daemon side). It basically comes down to the bullets mentioned at the end;
update
docker service update --log-opt foo=barshould;foo=barin the log-opts for the service, but preserve the existing logging driverstep 3. should also (or "ideally" instead?) be handled by the daemon (but this could be done in a follow-up)
There's one case I haven't really thought about what's best; if a logging-driver is specified on
update, and that driver is different than the one it currently has, we should probably reset existing log-driver options (happy to get input on that one though; it's tricky to define what's most expected in that case)thaJeztah commentedon Nov 24, 2020
I guess the ideal would've been if
--log-driverand--log-optwere combined in a single flag, so that they can be set or updated atomically (something like--logging driver=json-file,max-file=3,max-size=20mb), but that would be a bigger change