WIP: windows container compatibility

[amarruedo]

Hi there!

The following PR tries to solve JENKINS-36776.

So far this is what I have done:

• I've replaced all sh step calls in Docker.groovy with a function that determines if it should use sh or bat
• Changed DockerClient.run() method. Since DockerClient.run() is only used to run the container in a passive mode, I decided to remove user and entrypoint parameters from the method and let DockerClient figure out their values.
• Changed the way environment variables are passed to docker container when doing a docker exec.

With this, I have managed to build our .NET projects in a Windows Server using Jenkins+docker.

I would appreciate to know your thoughts on this changes.

Thank you very much for your attention.

[aidansteele]

This works wonderfully for building .Net projects on Windows just as you said. It's worth noting though that this increases the minimum required version of Docker to 1.13 as that was when API v1.25 was introduced (the env argument to exec requires this).

[amarruedo]

@aidansteele is right, the changes I suggest require docker version 1.13

Should the backwards compatibility be an issue, then I see two possible options:

• Modify the decorator of the WithContainerstep in such a way that for unix environment variable passing it works as it is working right now, and for windows environment variables it works as I suggest (using the -e flag on docker exec). Then, add a warning on the steps start() method, stating that docker version 1.13 is required to work properly on Windows.

• Try to port to windows the way environment variables are passed in unix.

Personally I do prefer first option, basically because it is the easiest and cleanest one. I've been searching how to implement the second possibility and I think that it involves using subprocesses on Windows command line among other things, which right now I don know how to develop properly on the plugin.

I wolud appreciate some feedback just to decide if I should invest time trying to develop the porting, or to know if we are happy with the first possibility

[aidansteele]

I would think/hope that requiring Docker 1.13+ would be acceptable for this significant improvement in functionality. That said, I'm not the maintainer of this project nor associated with the Jenkins project so I don't know how they generally treat this issue.

[amarruedo]

Some popular container distros like CoreOS don't have docker version 1.13 on their stable releases, so I decided to make the changes mentioned previously, where I maintain backwards compatibility for the unix case.

[jglick]

Without the ability to reproduce this environment I cannot maintain such a patch. I am thinking of ways to deprecate (most of) this plugin altogether so users can run docker commands themselves however they prefer.

[aidansteele]

@jglick As a workaround in my Jenkinsfiles I've created a dbat command that essentially wraps bat and decorates the command passed to bat with docker exec ... ${cmd}.

This works well as a replacement for direct calls to bat but the issue arises when I call shared pipeline functions that call bat directly. I guess what I would like is a way to register a "shell call decorator", e.g. something like:

def dockerDecorator(args, orig) {
  args.script = "docker exec <container> ${args.script}"
  return orig(args)
}

withShellDecorator(dockerDecorator) {
  bat "some command to be run inside the container"
  someSharedPipelineFunctionThatCallsBat() // even the bat inside here would be decorated
  sh "as above if this were a unix system"
}

I imagine this might already be possible somehow but my Jenkins-fu is weak. This could serve as a replacement for this plugin. What are your thoughts?

[jglick]

There is a LauncherDecorator system in Jenkins which plugins can implement but it is not set up in a way that a Pipeline script could contribute implementations, due to technicalities of CPS translation and so on. Better to have shared functions allow a callback to replace bat calls.

[rcreasey]

Any update on this getting addressed? Would love to start using this for windows containers.

[MadsAndreasen]

What would it take for this patch to be applied? Is there anything the community can do to help it along? It would be great if windows container could be supported.

[jetersen]

Can be considered closed @oleg-nenashev