-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Handle networks.driver_opts for a service #5125
Handle networks.driver_opts for a service #5125
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5125 +/- ##
==========================================
- Coverage 61.37% 61.35% -0.03%
==========================================
Files 298 295 -3
Lines 20717 20717
==========================================
- Hits 12715 12710 -5
- Misses 7102 7104 +2
- Partials 900 903 +3 |
| "driver_opts": { | ||
| "type": "object", | ||
| "patternProperties": { | ||
| "^.+$": { "type": ["string", "number"] } | ||
| } | ||
| }, |
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.
I put this into schema file v3.12 to get things going ... but perhaps should have created a new version?
(I'm not sure what the version is used for - but this isn't a new API field, just new to the compose spec. So, the new field will be handled by old engines, even if they don't understand specific driver options - it just won't appear in older compose files.)
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.
Yes, we probably should do a new version (but we made the version field optional to automatically pick "latest"
To add a new version, it's good to do in 2 commits;
- 1 commit that makes a copy of the previous version (no changes)
- 1 commit that updates the schema
That way it's easier to see the changes that were introduced in the new version
I think there's a couple of other fields we should add (some of those to be "stubs") so that we're more compatible with the compose-spec; until we have time to work on planning the actual migration to the compose-spec (which may need some more work).
We can discuss tomorrow if you have some time then
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.
Thank you ... I've added a v3.13 and made the changes there.
Signed-off-by: Rob Murray <rob.murray@docker.com>
These are endpoint-specific driver options...
services:
myservice:
image: myimage
networks:
mynet:
driver_opts:
"option1": "value1"
The API has had support for a long time, it's only recently been
added to compose (unreleased right now).
Signed-off-by: Rob Murray <rob.murray@docker.com>
6bad170
to
94f9de5
Compare
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
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


- What I did
Handle networks.driver_opts for a service.
Related to:
These are endpoint-specific driver options...
The API has had support for a long time, it's only recently been added to compose (unreleased right now).
- How I did it
Updated verification schema, add
ServiceNetworkConfig.DriverOpts, passed the opts to swarm, and updated tests.- How to verify it
Updated tests.
With this config:
Ran
docker stack deploy --compose-file compose.yml foo... checked that the option appeared in the container'sinspectoutput, and the sysctl was updated in the container.- Description for the changelog