Skip to content
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

Add support for capabilities options in services #39173

Merged
merged 1 commit into from
Jun 6, 2019

Conversation

olljanat
Copy link
Contributor

@olljanat olljanat commented May 5, 2019

- What I did
Add support for capabilities options in services

Third step to address #25885 , #24862 and moby/swarmkit#1030

- How I did it

  • Increased API version 1.41
  • Added Capabilities option
  • Bump swarmkit

- How to verify it
Create service with command:

curl --unix-socket /var/run/docker.sock \
  --header "Content-Type: application/json" \
  --request POST \
  --data '{"Name":"test","TaskTemplate":{"ContainerSpec":{"Image":"ollijanatuinen/capsh","Capabilities":["CAP_NET_RAW","CAP_SYS_CHROOT"]}}}' \
  http://localhost/v1.41/services/create

Check log with command: docker logs <container id> which should print capabilities from inside of container.

Update capabilities

curl --unix-socket /var/run/docker.sock \
  --header "Content-Type: application/json" \
  --request POST \
  --data '{"Name":"test","TaskTemplate":{"ContainerSpec":{"Image":"ollijanatuinen/capsh","Capabilities":["CAP_MKNOD"]}}}}' \
  http://localhost/v1.41/services/test/update?version=7139

Reset to default capabilities:

curl --unix-socket /var/run/docker.sock \
  --header "Content-Type: application/json" \
  --request POST \
  --data '{"Name":"test","TaskTemplate":{"ContainerSpec":{"Image":"ollijanatuinen/capsh","Capabilities": null}}}}' \
  http://localhost/v1.41/services/test/update?version=7157

- A picture of a cute animal (not mandatory but encouraged)
image

@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@cefc60c). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #39173   +/-   ##
=========================================
  Coverage          ?   37.05%           
=========================================
  Files             ?      612           
  Lines             ?    45484           
  Branches          ?        0           
=========================================
  Hits              ?    16852           
  Misses            ?    26347           
  Partials          ?     2285

@olljanat
Copy link
Contributor Author

olljanat commented May 13, 2019

(reserved for my derek commands)

@derek derek bot added the rebuild/janky label May 13, 2019
@olljanat olljanat changed the title WIP: Add support for capabilities options in services Add support for capabilities options in services May 13, 2019
@derek derek bot added the rebuild/janky label May 13, 2019
@derek derek bot added the rebuild/janky label May 14, 2019
@derek derek bot added the rebuild/janky label May 14, 2019
@olljanat olljanat force-pushed the 25885-capabilities-swarm branch 3 times, most recently from 997afe0 to 961325c Compare May 15, 2019 13:58
@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 15, 2019
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "25885-capabilities-swarm" git@github.com:olljanat/moby.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354127376
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label May 15, 2019
@derek derek bot added the rebuild/* label May 15, 2019
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Left some comments inline 🤗

vendor/github.com/docker/swarmkit/vendor.conf Outdated Show resolved Hide resolved
api/swagger.yaml Show resolved Hide resolved
api/swagger.yaml Outdated Show resolved Hide resolved
@olljanat olljanat force-pushed the 25885-capabilities-swarm branch 2 times, most recently from cc2c9f3 to 587aef5 Compare May 28, 2019 16:50
Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
@olljanat
Copy link
Contributor Author

@thaJeztah rebased and updated based on your comments. Janky should pass after #39277 is merged.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DigitalTargit
Copy link

How would you use this from a compose-file that you pushed at a swarm using "docker stack deploy" ?

@thaJeztah
Copy link
Member

Compose file changes are not yet implemented, but you'd specify the list of capabilities that a service should get

@olljanat
Copy link
Contributor Author

Yea. That part probably needs still some discussion and actual implementation after that. For example if we will actually support "--privileged" switch like requested on moby/swarmkit#1030 and #24862 or will we up force user to specify exact list of capabilities? (it will be still possible to specify all the capabilities so it more about how we want to implement user interface).

@DigitalTargit
Copy link

DigitalTargit commented May 29, 2019

uggg this question has no business here ... but how do I get this build for testing on Ubuntu 16.04 ...

cause for now I'll write a script that I push into my swarm nodes and "Update capabilities using curl" like your example above ...

@thaJeztah
Copy link
Member

You can compile a static binary with make binary, which builds the daemon and stores it in bundles. You can stop your current daemon and replace it (but definitely only recommended for testing)

Otherwise wait for this to be merged, and it should appear as a nightly build on download.docker.com

@DigitalTargit
Copy link

DigitalTargit commented May 29, 2019

Cool ... I cloned https://github.com/olljanat/moby.git ... and it built. Since you approved the change does this mean this change gets merged tonight and show up in tonights nightly build ? ... and seriously this goes without saying : you know you're one of the most helpful people this industry has ever seen even when the answer is not what people want to hear. Thank you.

@thaJeztah
Copy link
Member

You make me blush! All credits for this one really go to @olljanat!

We generally want two maintainers to review and give their LGTM before we merge; to be sure that a reviewer didn't miss anything (we're human 😁).

That said; anyone can help (and is more than welcome) to review PRs (more eyes never hurt)

This PR is fairly straightforward, so once another maintainer finds time to review, I expect it to be merged soon

@DigitalTargit
Copy link

DigitalTargit commented May 30, 2019

FYI ... I tried it out. I couldn't use it for what I was hoping to solve (using mongo with cifs remote storage). Azure just added NFS today so maybe that's an option but not sure the best driver to use for that.

BUT I did test it this PR (created a swarm with one service) and I do see the capabilities returned in the JSON. 👍 The credit does go to @olljanat

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kolyshkin kolyshkin merged commit 1d5748d into moby:master Jun 6, 2019
@andrewhsu andrewhsu added the kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny label Nov 7, 2019
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
@olljanat olljanat deleted the 25885-capabilities-swarm branch February 17, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/swarm impact/changelog impact/documentation kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants