Worker upstart script ignores /etc/default/thumbor #166

Closed
wichert opened this Issue Mar 8, 2013 · 13 comments

7 participants

@wichert

As a result critical settings are ignored.

I do not understand why there are now two upstart files: thumbor and thumbor-worker. The first does read /etc/default/thumbor, but only to determine of thumbor-worker should be started. What is the rationale for that split? As far as I can see there is no benefit.

@wichert

Ok, I see this is related to the multiple worker thing. I am not convinced the current solution is the right one. I see several problems:

  • the thumbor-worked upstart script ignores ip, conffile and keyfile from /etc/default/thumbor completely which breaks valid configurations.
  • there is an underlying assumption that all thumbor workers use the same configuration and only differ in port number. If the goal is to support multiple thumbors different configurations for each worker should be supported.

I propose a different solution: move configuration files to /etc/thumbor, create a new /etc/thumbor/workers directory and fill that with the equivalent of the current /etc/default/thumbor files. The upstart script can then iterate over those and use separate settings for each worker.

@dhardy92

I don't feel comfortable with upstart yet, it likely I did wrong doing this.

@heynemann
Thumbor (by @globocom) member

Is this still the case @dhardy92 @wichert ?

@heynemann
Thumbor (by @globocom) member

@morpheu what do you think?

@morpheu
Thumbor (by @globocom) member

I've updated thumbor-worker upstart to proper use ip bind defined on thumbor default file (a922636). According with upstart documentation - http://upstart.ubuntu.com/cookbook/#another-instance-example - using 2 upstart conf files is the right way to deal with multiple instances. Calling thumbor upstart or stopping it will also start/stop all instances of thumbor-worker. Right now, I see no point in have different workers using different conf files.

@morpheu morpheu closed this Jan 8, 2014
@semente

Not sure, but this change might broke thumbor when starting multiple instances. I'm on Ubuntu 12.04/precise using the package from PPA (3.15.0-0ubuntu1ppa1~precise).

Sometimes some instances are started but usually none of them are.

In upstart logs I have dozens of "/proc/self/fd/9: 2: .: Can't open /tmp/thumbor-worker-8881".

Also, upstart says that thumbor is running even when no instances are running.

@morpheu
Thumbor (by @globocom) member

@semente, could you paste your /etc/default/thumbor file ?

@morpheu morpheu reopened this Jan 15, 2014
@semente

That is the current, but I also tried with the one that comes with the package including the default /etc/thumbor.conf.

# Location of the configuration file
conffile=/etc/thumbor.conf

# Location of the keyfile which contains the signing secret used in URLs
keyfile=/etc/thumbor.key

# IP address to bind to. Defaults to all IP addresses
ip=127.0.0.1

# TCP port to bind to. Defaults to port 8888.
# multiple instances of thumbor can be started by putting several ports coma separeted
port=8881,8882,8883,8884
@neovatar neovatar pushed a commit that referenced this issue Jan 30, 2014
Thomas Seliger Removed uneeded temporary file code from debian/thumbor-worker.upstar…
…t code. This code prevented thumbor worker from being started and resulting in a "/proc/self/fd/9: 2: .: Can't open /tmp/thumbor-worker-8881" error message. Fixes thumbor/thumbor#166
18d9777
@neovatar

Same error here. The problem is in /etc/init/thumbor-worker.conf:

pre-start script
    [ -r /etc/default/thumbor ] && . /etc/default/thumbor
    if [ "$enabled" = "0" ] && [ "$force" != "1" ] ; then
        logger -is -t "$UPSTART_JOB" "Thumbor is disabled by /etc/default/thumbor, add force=1 to your service command"
        stop
        exit 0
    fi
    exec >"/tmp/${UPSTART_JOB}-${p}"
    echo "ip=${ip}"
end script

script
    . "/tmp/${UPSTART_JOB}-${p}"
    $DAEMON -c "${conffile}" -i "${ip}" -k "${keyfile}" -p "${p}" -l debug
end script

I am not sure what you try to achieve by calling exec >"/tmp/${UPSTART_JOB}-${p}". Actually I don't understand (sorry, if I am missing something here), why we would need that temporary file at all?

@dhardy92

since http://www.markshuttleworth.com/archives/1316 and there is init.d scripts available for debian.
Upstart seams not really a good investment.

@robolson

@dhardy92 Ubuntu 14.04 LTS will still have Upstart and people will be using that release for many years to come. I think if its not too difficult, having a working Upstart script will still be beneficial to Thumbor.

@dhardy92

Actually my point is init.d work also on Ubuntu 14.04
Need only to remove specific things on debian/rules to work every where equally.

@heynemann
Thumbor (by @globocom) member

Is this still the case?

@heynemann heynemann closed this Sep 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment