Consequently these kinds of fixes can be applied to the other images. I just wanted to run this PR by you all.

It appears that moving the host into the Dockerfile has made this not work when used with Compose.

This used to work...

server:
  image: gocd/gocd-server
  ports:
    - "8153:8153"
agent:
  image: gocd-agent
  links:
    - server:go-server

After applying your changes the agent wants to connect to localhost:8153 (or whatever I put in the Dockerfile of course) and not what's provided by Compose.

Any suggestions?

Ah yes, the GO_SERVER var should default to go-server. I'll fix

That does make mine work, but I'm still not sure hard coding it is the "right thing".

Interested in other opinions (that we're not likely to get on a holiday weekend)

The ENV directive sets the default value for the environment variable and it can be overridden at runtime with "docker run -e GO_SERVER=foo". Since the docs state that that default will be used I think this is a good solution.

Any other thoughts? Is this good to merge?

@arvindsv maintains this the most and he's out of the office this week. He'll be back early next week. Sorry for the delay!

What's the best way to add to the agent's PATH environment? I haven't been able to follow the logic from the /sbin/my_init to determine how I can change it

@dkastner I take it you disagree with http://phusion.github.io/baseimage-docker/ on why you should use an init system in your Docker container? Would you care to explain why?
Would also care to explain why your builds, run by gocd-agent, should require root privileges?

@dmg3 if you really need to override PATH, you can do it at runtime with docker run -e PATH=/usr/bin:/my/new/path or you can modify it in a custom Dockerfile/image build:

FROM gocd/gocd-agent

ENV PATH=/usr/bin:/my/new/path

@FredrikWendt

re: Phusion's philosophy:

Phusion came up with its baseimage while docker was still in its early stages and they were operating under the sort of assumptions you make when you have a "snowflake" server that needs to have maximal uptime. I'll address each of their reasons for their baseimage:

chron

We don't need chron. If we did, it'd be better to build a specialized image that installs chron and is only in charge of running chron, not your app and a bunch of other processes in addition to chron.

syslog

AFAIK We don't need it.

ssh

Needing to ssh into a container is an antipattern. The recommended practice is to not auto-rm containers when they die, allowing you to inspect them with docker exec and docker cp to investigate crashes.

runit/daemon monitoring

This isn't needed with docker's --restart flag and/or platforms like ECS or Kubernetes.

zombie processes

This shouldn't be a problem if the agent properly waits for the command processes it executes.

re: root privileges.

It's not that anything should require root privileges, it's just that with docker everything can run as root because it's all secured within LXC's user namespace. In other words, the root user here is "jailed" and unable to affect the host OS. If there is a security vulnerability in the agent itself, the container will be compromised, but the effects will be limited.

For any of these concerns, we don't need to make gocd-agent responsible. Users can build custom images using this barebones image which is much smaller than the phusion baseimage.

@dkastner Thanks! That's the piece I was missing!

On Sun, Oct 18, 2015 at 9:04 AM, Derek Kastner [email protected]
wrote:

@dmg3 https://github.com/dmg3 if you really need to override PATH, you
can do it at runtime with docker run -e PATH=/usr/bin:/my/new/path or you
can modify it in a custom Dockerfile/image build:

FROM gocd/gocd-agent
ENV PATH=/usr/bin:/my/new/path

—
Reply to this email directly or view it on GitHub
#13 (comment).

I think @dkastner approach is very good. I also believe other images (server, dev, build-installer) should also favor using debian or java official image, instead of phusion/baseimage.

Please also see jpetazzo's article on why thin images are better: https://jpetazzo.github.io/2014/06/23/docker-ssh-considered-evil/

This change is very useful, using it for my purposes.

I just notice one problem with this image: zombie process and reaping, more here:
http://blog.phusion.nl/2015/01/20/docker-and-the-pid-1-zombie-reaping-problem/

I noticed that folks from Jenkins, solved that using a tool called tini, the discussion is here: jenkinsci/docker#54 (comment)

I am working on solving this, still WIP here https://github.com/labianchin/gocd-docker/blob/no-daemon2/gocd-agent/Dockerfile. When it is done, I will create a PR.

Other thing that would be nice is to check the SHA1 sum from the downloaded deb package.

As far as I understand, the only reason a process could become a zombie is if:

• Someone manually sends a kill -9 to a process that Go is running that happens to have subprocesses
• Go sends a SIGKILL to the task it is running
• The OOM killer times out when trying to SIGTERM a process and sends it SIGKILL

Are there any other instances?

The first source of zombies seems like a "user error" problem - in that if you go around kill -9ing things, I'm inclined to say you have no business expecting the system to clean up after you.

I believe the second case could be solved by Go itself. If Go needs to kill subprocesses, it should be assigning subprocesses to process groups and sending kill signals to the entire group.

The tini solution seems to be a good short-term fix, but the PR_SET_CHILD_SUBREAPER setting it supports could theoretically be handled by Go itself (and would be a great addition on any platform), although I'm not familiar with java enough to know if it can support those kernel features.

Great idea - This is almost exactly like the my version. Which I created for the exact same reasons @dkastner mentions. I just used the gocd zip file download instead of the installer to have all the files conveniently in one place and more control over other stuff like the java version. I also dockerized the server in the same manner.

I'd suggest adding agent.auto.register.hostname ($AGENT_HOSTNAME) to the autoregister.properties file (available since v15.2) in go-agent-start.sh in the same manner as $AGENT_RESOURCES and $AGENT_ENVIRONMENTS. In my version I actauly add $AGENT_HOSTNAME-$(hostname) so I also have the real hostname of the (docker)machine so it can easily be traced.

You might also wish to make some compatibility with running from zip install. The only two things that need to be considered (in go-agent-start.sh) are:
a. $AGENT_CONFIG_DIR (in my version if it doesn't exist then I set it to the current working dir/config (and create the directory).
b. location of (i execute from current dir - exec ./agent.sh)

btw - prior to version 15.3.0, the agent.sh script starts the agent with eval "$CMD" (not exec) - so it does not become the root process nor receive signals.

I'm trying to incorporate some of the changes in this PR into #27. Feedback welcome.

Hi @arvindsv. Also take a look on @tanob images tanob/gocd-docker. He is using it for a couple of months already in his project.

Thanks, @labianchin. I will.

Closing in favour of #43