add exec option to tmpfs

[adshmh]

Added exec option to tmpfs mounts, part of work on moby PR to add exec option to tmpfs mounts
Signed-off-by: Arash Deshmeh adeshmeh@ca.ibm.com

- What I did
Added exec/noexec option to Tmpfs

- How I did it
Modified the API and the agent/exec/dockerapi package. Also added the corresponding unit tests. Also added the exec/noexec option to flagparser.

- How to test it

- Description for the changelog
Added exec option to tmpfs mounts

[thaJeztah]

ping @anshulpundir @dperny PTAL; this is related to a PR in moby; moby/moby#36720

[thaJeztah]

/cc @cpuguy83 @kolyshkin

[dperny]

I'd rather not see a raw_options field in the proto message. I commented on the moby PR this same thing, but it's even more important in swarmkit, I think, to have all of the options structured. I find that when we store data as unstructured strings or maps, we tend to regret it later. I'd rather see it as a boolean.

// allow execution of binaries from the tmpfs
bool exec = 3;

[thaJeztah]

@dperny see moby/moby#36720 (comment) - is this a blocker for you?

[dperny]

@thaJeztah Nah, it's not a blocker.

@adshmh Let's rebase, rebuild protos, and I'll give it one last once-over before we ship.

[thaJeztah]

ping @adshmh could you rebase this PR so that we can move it forward? 🤗

[adshmh]

Sorry for the delay. Rebased.

[thaJeztah]

Whoops; looks like there's an issue with the vendor directory; there's no change in vendor.conf here, but you pulled in a modified file in moby/moby;

diff -r vendor/github.com/docker/docker/api/types/mount/mount.go .vendor.bak/github.com/docker/docker/api/types/mount/mount.go
111a112,114
> 	// Raw options passed to tmpfs mount
> 	RawOptions string `json:",omitempty"`
> 
+ inconsistent dependencies! what you have in vendor.conf does not match with what you have in vendor
make: *** [dep-validate] Error 1
Exited with code 2

[adshmh]

Thanks for the review. Fixed (The modified file from moby/moby was not needed, as conversion will be done on moby/moby not swarmkit).

Latest build failure is from a timeout in running unit tests which does not happen in my local test environment.

[codecov]

Codecov Report

Merging #2647 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #2647     +/-   ##
=========================================
+ Coverage   61.87%   61.97%   +0.1%     
=========================================
  Files         136      136             
  Lines       21926    21933      +7     
=========================================
+ Hits        13566    13594     +28     
+ Misses       6896     6874     -22     
- Partials     1464     1465      +1

[thaJeztah]

Wondering why these changes are here; this could be if you're using a different version of protobuf (see #2503), could you check what version you're using to regenerate?

[adshmh]

Thank you. Fixed. Seems the build uses protoc 3.6.1 now (on both containerized builds and CircleCI).

Perhaps the BUILDING.md file needs an update, as it refers to 3.x or higher versions of protoc:

https://github.com/docker/swarmkit/blob/master/BUILDING.md#regenerating-protobuf-bindings

I think a link to the option of building in a container, showing up at the top of the BUILDING.md, could be useful as well (I can submit a PR for that if it sounds like a good idea)

[thaJeztah]

Ah, yes, that would be a good thing to do. I also think that @dperny was working on a containerised version a while back (so that the version could be "pinned" in that container)

[thaJeztah]

LGTM, thanks!

[thaJeztah]

ping @dperny @anshulpundir PTAL

[denismakogon]

Hi! Are there any estimates on when this is going to be merged? Since moby/moby#36720 heavily depends on this PR. Would be nice to get this merged as soon as possible. Thanks.

[denismakogon]

@dperny thank you.