Entrypoint script does not delete PID file

[FerricFeuerherz]

Hello, please excuse my bad English, but I have found a small bug in the entrypoint.sh file. I run a docker instance on my Raspberry Pi and I had a power shortage after which the container does not want to start any more. It seems that there is a small bug in the entrypoint script. The script never can find any PID file because the variable PID will contain the TCL variable declaration part.

That is why I added the following Line to the entrypoint script and the PID file will be deleted as expected.

PID="pid.$(echo $PID | awk '{gsub("\"", "", $3); print $3}')"

Greetings from Germany, Ferric

[vanosg]

Hi @FerricFeuerherz, sorry it took so long to respond. Do you happen to have a log file of the error you are describing occuring? Thanks!

[FerricFeuerherz]

Hello @vanosg , no problem, I guess you have also have other things to do. Sadly, I don't have the original Log files anymore.

But I reproduced the incident by creating the pid-file after I shutdown the bot. As you can see the container will not start properly, because the pid-file get not removed.

 Eggdrop v1.8.4 (C) 1997 Robey Pointer (C) 2010-2018 Eggheads
 --- Loading eggdrop v1.8.4 (Wed May  5 2021)
 Listening for telnet connections on 0.0.0.0:3355 (all).
 Module loaded: blowfish
 Module loaded: dns
 Module loaded: channels
 Module loaded: server
 Module loaded: ctcp
 Module loaded: irc
 Module loaded: notes            (with lang support)
 Module loaded: console          (with lang support)
 Module loaded: uptime
 Loading dccwhois.tcl...
 Loaded dccwhois.tcl
 Userinfo TCL v1.07 loaded (URL BF GF IRL EMAIL DOB PHONE ICQ).
 use '.help userinfo' for commands.
 Writing channel file...
 Userfile loaded, unpacking...
 USERFILE ALREADY EXISTS (drop the '-m')
 === Rimuru: 1 channels, 4 users.
 I detect Rimuru already running from this directory.
 If this is incorrect, erase the 'pid.Rimuru'

I had outputed the value of the variable PID to the shell and it seems that there is a wrong value stored - or at least not the expected value. I think the issue is the following line of code, because the third element of the string returned by grep is saved.

PID=$(grep "set pidfile" ${CONFIG} |awk '{print $3}')

In this line is checked if the value of PID starts with # but this will never be true, because the # is not saved because of the awk command.

if [[ $PID == \#* ]]; then

That's why I have modified my entryfile as following:

### Remove previous pid file, if present
  PID=$(grep "set pidfile" ${CONFIG})
  if [[ $PID == \#* ]]; then
    PID=$(grep "set botnet-nick" ${CONFIG})
    if [[ $PID == \#* ]]; then
      PID=$(grep "set nick" ${CONFIG})
    fi
  fi

  # I have enclosed most of my strings in ", thats why I have to remove them.
  PID="pid.$(echo $PID | awk '{gsub("\"", "", $3); print $3}')"

  if [ -e "$PID" ]; then
    PID="${PID//\"}"
    echo "Found $PID, removing..."
    rm $PID;
  fi

At least this works for me but I think there is a better solution for this issue.

Best Regards from Germany Ferric

[vanosg]

You commented on this as we were releasing 1.9, things got a little hectic. Which, by the way, is now released as the docker image, so pulling eggdrop:latest will give you 1.9.0. Anyway-

The intent of the pidfile is an OS-agnostic method to determine whether or not Eggdrop is running or not. Eggdrop checks for the existence of the pidfile and, if it is there, won't start because it assumes a copy of Eggdrop is already running from that file. That is intended behavior, we don't want Eggdrop starting if it is already running. So we don't want to always just delete the PID file if it is there, because it is there for a reason. However, in some circumstances, the pid file doesn't get deleted (like in the power loss you described), and that is why is suggests to you to delete the PID file.

I'll look into this a little more and see if there is a tweak that docker could support, but I don't know if this is something we want to incorporate at this time. As the error suggestions, the solution in some cases is just for the user to delete the PID manually and try again. But, we'll certainly look into it, thanks!

[FerricFeuerherz]

Hello @vanosg, thank you for your fast reply. That's what I thought, that you are busy with the main project. I was very excited when I saw that a new Eggdrop version got released, but sadly I haven't found the time to upgrade yet.

But back to the pid-file issue. I am are aware of what a pid-file is for, but I had read the code in entrypoint.sh, and I thought the intention behind the code was to remove the pid-file in any circumstance. That's why I opened the issue in the first place.

The Eggdrop is the first time that I have ever used docker and container in general therefore I am inexperienced in this topic. But I don't think that deleting a file manually somewhere in "/var/lib/docker/overlay2/*" is the right way to get rid of the issue. Another way would be to delete the current container and start another one with the "docker run" command.

But of course this was a very user specific issue and I think most other users will not run into this kind of issue.

Thank you for your effort and that you are taking care of the issue :).

Kind regards Ferric

[eelcohn]

Any update on this issue? I still experience this issue on Eggdrop-docker v1.9.5

[vanosg]

@FerricFeuerherz First, I must apologize, I think I misunderstood the question/issue you were raising. The existing method is indeed an incorrect way to handle checking if PID file is there! You nailed the source and the solution. I hope you have been able to continue using this image in the meantime. I'm merging an updated entrypoint script in shortly using more or less the method you outlined, but let me know if you find any issues with it. I also appreciate you thinking about "s in the field as well, something that I hadn't accounted for. Appreciate any feedback you have to offer once the change goes in

Thanks for the nudge @eelcohn

[vanosg]

Closed by b8e53d8

[raws1]

Hi Team, I am also running into this issue and I see the commit by @vanosg but the fix hasn't been merged into master. It seems like from the comment above you planned to merge it soon so just wanted to ping you and see if that was still planned. Thank you :)

[vanosg]

Hi @raws1 - It has been merged into the develop snapshot, and will be incorporated into the v1.10 image release once that is released, which is going to be "very soon".

We've been saying "very soon" since December, but we really mean it this time :) Thanks, and sorry for the delay on getting version 1.10 out the door

[raws1]

Thank you immensely for your work on this! I don't want to second guess you but wouldn't it make sense to fix the script across all revisions (1.6 - 1.9) of the eggdrop containers since it's part of the docker specific scripts and not an eggdrop code fix?

[vanosg]

It's a great thought. Unfortunately we don't have the time/resources to do extensive testing of older versions, to include docker (yes it's docker, but the unintended interactions can often be surprisingly intense). We do have Dockerfiles for older versions on the GH for legacy/courtesy, but we are only currently supporting 1.9 as an official image on Docker Hub. And again, the hope was to be on 1.10 by now, so our attention has been focused in that direction for the moment.

[vanosg]

ok, Soon has happened! Please check out the RC image for 1.10.0 , and let us know if you have any further issues