openshift-s2i / s2i-wildfly

Source-to-Image template for WildFly applications
http://wildfly.org/
Other
73 stars 145 forks source link

Wildfly doesn't gracefully terminate? #118

Closed bparees closed 7 years ago

bparees commented 7 years ago

Doesn't seem to react on SIGTERM...

from: https://github.com/openshift/origin/issues/12946

bparees commented 7 years ago

@smarterclayton doesn't sound promising: http://lists.jboss.org/pipermail/wildfly-dev/2016-April/004867.html

I suppose we could trap the sigterm and then invoke the cli to shut it down.

smarterclayton commented 7 years ago

Every image that we ship that runs on kubernetes should react to sigterm. If we can't do it in the framework itself, I think we should at least make an attempt in the scripts. Reacting to SIGTERM properly is pretty fundamental.

smarterclayton commented 7 years ago

I'd question whether anything that doesn't react to SIGTERM should be running on Kubernetes (except as a first step). Alternatively - implement preStop

smarterclayton commented 7 years ago

The problem with preStop is that it won't be automatic whenever the image is used, only if you put it in a template.

jorgemoralespou commented 7 years ago

The thread doesn't look promising at all when it also switches to graceful startup. The server can accept requests before the application is deployed as well.

El 14 feb. 2017 5:00, "Ben Parees" notifications@github.com escribió:

@smarterclayton https://github.com/smarterclayton doesn't sound promising: http://lists.jboss.org/pipermail/wildfly-dev/2016-April/004867.html

I suppose we could trap the sigterm and then invoke the cli to shut it down.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openshift-s2i/s2i-wildfly/issues/118#issuecomment-279601835, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEyDnxz51_3H7AtOEB-gfPGii8n_GtCks5rcSbmgaJpZM4MACDA .

bparees commented 7 years ago

The thread doesn't look promising at all when it also switches to graceful startup. The server can accept requests before the application is deployed as well.

@jorgemoralespou in theory that can be managed via an appropriate readiness probe though.

jorgemoralespou commented 7 years ago

@bparees I realized that soon afterwards. I've even talked about that at a conference. Shame on me.

jorgemoralespou commented 7 years ago

@bparees have you seen this in the jboss-dockerfiles wildfly image? https://github.com/jboss-dockerfiles/wildfly/pull/16/files

bparees commented 7 years ago

@jorgemoralespou thanks, i hadn't... but based on the thread above i wonder what that env variable actually does.

bparees commented 7 years ago

@jorgemoralespou all that flag seems to do is setup a bunch of shell traps to forward signals to the JVM that's being daemonized, i don't think it actually changes how the JVM is going to handle the TERM signal when it gets it.

   if [ "x$LAUNCH_JBOSS_IN_BACKGROUND" = "x" ]; then
      # Execute the JVM in the foreground
      eval \"$JAVA\" -D\"[Standalone]\" $JAVA_OPTS \
         \"-Dorg.jboss.boot.log.file="$JBOSS_LOG_DIR"/server.log\" \
         \"-Dlogging.configuration=file:"$JBOSS_CONFIG_DIR"/logging.properties\" \
         -jar \""$JBOSS_HOME"/jboss-modules.jar\" \
         $MODULE_OPTS \
         -mp \""${JBOSS_MODULEPATH}"\" \
         org.jboss.as.standalone \
         -Djboss.home.dir=\""$JBOSS_HOME"\" \
         -Djboss.server.base.dir=\""$JBOSS_BASE_DIR"\" \
         "$SERVER_OPTS"
      JBOSS_STATUS=$?
   else
      # Execute the JVM in the background
      eval \"$JAVA\" -D\"[Standalone]\" $JAVA_OPTS \
         \"-Dorg.jboss.boot.log.file="$JBOSS_LOG_DIR"/server.log\" \
         \"-Dlogging.configuration=file:"$JBOSS_CONFIG_DIR"/logging.properties\" \
         -jar \""$JBOSS_HOME"/jboss-modules.jar\" \
         $MODULE_OPTS \
         -mp \""${JBOSS_MODULEPATH}"\" \
         org.jboss.as.standalone \
         -Djboss.home.dir=\""$JBOSS_HOME"\" \
         -Djboss.server.base.dir=\""$JBOSS_BASE_DIR"\" \
         "$SERVER_OPTS" "&"
      JBOSS_PID=$!
      # Trap common signals and relay them to the jboss process
      trap "kill -HUP  $JBOSS_PID" HUP
      trap "kill -TERM $JBOSS_PID" INT
      trap "kill -QUIT $JBOSS_PID" QUIT
      trap "kill -PIPE $JBOSS_PID" PIPE
      trap "kill -TERM $JBOSS_PID" TERM
      if [ "x$JBOSS_PIDFILE" != "x" ]; then
        echo $JBOSS_PID > $JBOSS_PIDFILE
      fi
jorgemoralespou commented 7 years ago

I agree, but I wonder why the upstream jboss dockerfiles have enabled such feature. And knowing these are the same people that do EAP, I would just test it.

bparees commented 7 years ago

maybe we should just ask @goldmann

bparees commented 7 years ago

@goldmann or @rcernich can you offer any advice/insight here?

bparees commented 7 years ago

@goldmann @rcernich bump, can you offer any insight on how to get wildfly (or eap) to handle sigterm gracefully, or if it's even possible?

rcernich commented 7 years ago

I believe we used a prestop hook: https://github.com/jboss-openshift/application-templates/blob/master/eap/eap70-basic-s2i.json#L302-L312

bparees commented 7 years ago

thanks @rcernich

as @smarterclayton said, the problem w/ that approach is it only works if the user knows to put it in their deploymentconfig (or uses that template). A wrapper script that launches jboss and catches sigterm (and then invokes that same logic) is probably the right answer, though it's definitely messier. Alternatively maybe we should just open an issue against wildfly and eap that they should handle sigterm cleanly.

jorgemoralespou commented 7 years ago

@bparees why alternatively and not complementary. You can provide a solution for what's current (with the workaround you mention) and then hope the project will handle it in the future. ;-)

bparees commented 7 years ago

Because I don't have unlimited development resources at my disposal. So hacking up a workaround that will (hopefully) be short-lived while the right fix is created isn't the best use of them.

Ben Parees | OpenShift

On Jul 7, 2017 03:28, "Jorge Morales Pou" notifications@github.com wrote:

@bparees https://github.com/bparees why alternatively and not complementary. You can provide a solution for what's current (with the workaround you mention) and then hope the project will handle it in the future. ;-)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openshift-s2i/s2i-wildfly/issues/118#issuecomment-313608647, or mute the thread https://github.com/notifications/unsubscribe-auth/AEvl3ujkhD9fTDWamPLNaNzWn_YWSsQcks5sLd3_gaJpZM4MACDA .

knrc commented 7 years ago

Both EAP 7.0 and EAP 7.1 respond to TERM signals in our images, the key is to set the LAUNCH_JBOSS_IN_BACKGROUND environment variable. I would assume wildfly does something similar but haven't checked.

knrc commented 7 years ago

BTW the TERM behaviour is different from the clean shutdown so the only way to currently ensure they are the same would be to trap the signal in a wrapper script and then issue the CLI command instead of passing the signal through to the JVM.

bparees commented 7 years ago

opened this as a wildfly RFE: https://issues.jboss.org/browse/WFLY-9062

knrc commented 7 years ago

FYI I also opened a JIRA on our side to investigate the wrapper as an alternative to the prestop hook, https://issues.jboss.org/browse/CLOUD-1864

bramvonk commented 6 years ago

We (I and @bpasson) added the line:

trap "kill -TERM $JBOSS_PID" SIGTERM

to the traps in the standalone.sh, and set the LAUNCH_JBOSS_IN_BACKGROUND environment variable. This way, the SIGTERM was correctly trapped and sent to the java process, and wildfly shut down as expected.