jenkinsci / nomad-plugin

Nomad cloud plugin for Jenkins
https://plugins.jenkins.io/nomad/
MIT License
56 stars 41 forks source link

Apocol/nomad plugin #53

Closed AndreeaPocol closed 5 years ago

AndreeaPocol commented 5 years ago

MR #52 gives users the option to add capabilities, but not to drop them - but the nomad code itself requires both in order to deliver expected behaviour.

If I want to add, say, MKNOD capabilities to my container, I need to add the capability using the Jenkins plugin and also whitelist it in my nomad-client.hcl. That is the expected behaviour. But this will actually give me the following Docker driver error in my nomad agent:

Docker driver doesn't have the following caps whitelisted on this Nomad agent: [list of all basic capabilities minus MKNOD]

because I haven't explicitly dropped all of the other basic Docker-defined capabilities. The reason I need to drop them has to do with how the TweakCapabilities function in the nomad code works. This function (which produces the final list of capabilities desired by the user, i.e. effectiveCaps) defines tweaking as:

  1. starting with the basic Docker-defined capabilities
  2. removing those explicitly dropped by the user
  3. adding any additional capabilities explicitly added by the user

The code then takes the resulting list and validates against the user-defined whitelist.

Since this plugin currently doesn't let users drop capabilities, step 2 doesn't happen and the result of TweakCapabilities is essentially just the entire basic Docker-defined capability list. But, of course, the user hasn't whitelisted the entire basic capability list... They shouldn't have to! So the ensuing validation fails and the result is the aforementioned Docker driver error:

Docker driver doesn't have the following caps whitelisted on this Nomad agent: [list of all basic capabilities minus those whitelisted]

In my opinion, the nomad TweakCapabilities function implementation is not completely intuitive (I would just do step 3 above and validate that against the basic list), but in lieu of changing that, the workaround I've come up with is to add this capability-dropping feature.