Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start crond with s6 overlay #945

Open
2 of 4 tasks
vladaurosh opened this issue Jan 8, 2025 · 13 comments
Open
2 of 4 tasks

Start crond with s6 overlay #945

vladaurosh opened this issue Jan 8, 2025 · 13 comments
Labels
Feature request➕ New feature or request next release/in dev image🚀 This is coming in the next release or was already released if the issue is Closed.

Comments

@vladaurosh
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing open and closed issues

Is your feature request related to a problem? Please describe

Hi @jokob-sk
Just noticed in https://github.com/jokob-sk/NetAlertX/blob/main/dockerfiles/init.sh#L129 that crond is started in the background.

Describe the solution you'd like

Since we already use s6 overlay I think it would be good idea to move it into https://github.com/jokob-sk/NetAlertX/blob/main/dockerfiles/start.sh

Probably this would do:

# s6 overlay setup
mkdir -p /etc/s6-overlay/s6-rc.d/{SetupOneshot,crond/dependencies.d,php-fpm/dependencies.d,nginx/dependencies.d,$APP_NAME/dependencies.d}
echo "oneshot" > /etc/s6-overlay/s6-rc.d/SetupOneshot/type
echo "longrun" > /etc/s6-overlay/s6-rc.d/crond/type
echo "longrun" > /etc/s6-overlay/s6-rc.d/php-fpm/type
echo "longrun" > /etc/s6-overlay/s6-rc.d/nginx/type
echo "longrun" > /etc/s6-overlay/s6-rc.d/$APP_NAME/type
echo -e "${INSTALL_DIR}/dockerfiles/init.sh" > /etc/s6-overlay/s6-rc.d/SetupOneshot/up
echo -e "#!/bin/execlineb -P\n/usr/sbin/crond -f -d 8" > /etc/s6-overlay/s6-rc.d/crond/run
echo -e "#!/bin/execlineb -P\n/usr/sbin/php-fpm83 -F" > /etc/s6-overlay/s6-rc.d/php-fpm/run
echo -e '#!/bin/execlineb -P\nnginx -g "daemon off;"' > /etc/s6-overlay/s6-rc.d/nginx/run
...
touch /etc/s6-overlay/s6-rc.d/user/contents.d/{SetupOneshot,crond,php-fpm,nginx,$APP_NAME} /etc/s6-overlay/s6-rc.d/{crond,php-fpm,nginx,$APP_NAME}/dependencies.d/SetupOneshot
touch /etc/s6-overlay/s6-rc.d/nginx/dependencies.d/php-fpm
touch /etc/s6-overlay/s6-rc.d/$APP_NAME/dependencies.d/nginx

Also I've noticed:

mkdir -p /etc/s6-overlay/s6-rc.d/{SetupOneshot,php-fpm/dependencies.d,nginx/dependencies.d}
mkdir -p /etc/s6-overlay/s6-rc.d/{SetupOneshot,php-fpm/dependencies.d,nginx/dependencies.d,$APP_NAME/dependencies.d}

First line is not needed so block should be replaced with:
mkdir -p /etc/s6-overlay/s6-rc.d/{SetupOneshot,crond/dependencies.d,php-fpm/dependencies.d,nginx/dependencies.d,$APP_NAME/dependencies.d}

Same goes for:

touch /etc/s6-overlay/s6-rc.d/user/contents.d/{SetupOneshot,php-fpm,nginx} /etc/s6-overlay/s6-rc.d/{php-fpm,nginx}/dependencies.d/SetupOneshot
touch /etc/s6-overlay/s6-rc.d/user/contents.d/{SetupOneshot,php-fpm,nginx,$APP_NAME} /etc/s6-overlay/s6-rc.d/{php-fpm,nginx,$APP_NAME}/dependencies.d/SetupOneshot

First line is not needed, these 2 should be replaced with:
touch /etc/s6-overlay/s6-rc.d/user/contents.d/{SetupOneshot,crond,php-fpm,nginx,$APP_NAME} /etc/s6-overlay/s6-rc.d/{crond,php-fpm,nginx,$APP_NAME}/dependencies.d/SetupOneshot

Describe alternatives you've considered

N/A

Anything else?

N/A

Am I willing to test this? 🧪

  • I will do my best to test this feature on the netlertx-dev image when requested within 48h and report bugs to help deliver a great user experience for everyone and not to break existing installations.

Can I help implement this? 👩‍💻👨‍💻

  • Yes
  • No
@vladaurosh vladaurosh added the Feature request➕ New feature or request label Jan 8, 2025
jokob-sk pushed a commit that referenced this issue Jan 8, 2025
@jokob-sk
Copy link
Owner

jokob-sk commented Jan 8, 2025

Thanks @vladaurosh for the ongoing help 🙏 just pushed these changes into -dev in the above commit

@jokob-sk jokob-sk added the next release/in dev image🚀 This is coming in the next release or was already released if the issue is Closed. label Jan 8, 2025
jokob-sk pushed a commit that referenced this issue Jan 8, 2025
@jokob-sk
Copy link
Owner

jokob-sk commented Jan 8, 2025

Had to roll back the cron s6 implementation approach as it didn't trigger the scripts it was triggering previously - you can test this by trying to restart the app in Maintenance -> Logs -> Restart Server

@vladaurosh
Copy link
Contributor Author

Ok, what script needs to be triggered and what does "Maintenance -> Logs -> Restart Server" restart? The python application?

@jokob-sk
Copy link
Owner

jokob-sk commented Jan 9, 2025

This is the corontab template: https://github.com/jokob-sk/NetAlertX/blob/main/install/crontab

executing this script every minute

/app/back/cron_script.sh:

https://github.com/jokob-sk/NetAlertX/blob/main/back/cron_script.sh

@jokob-sk
Copy link
Owner

jokob-sk commented Jan 9, 2025

I might add more cron dependent scripts in future so that's why its a bit over engineered.

@jokob-sk jokob-sk added In development 🏗 and removed next release/in dev image🚀 This is coming in the next release or was already released if the issue is Closed. labels Jan 9, 2025
@vladaurosh
Copy link
Contributor Author

vladaurosh commented Jan 9, 2025

Ok, so cron runs cron_script.sh every minute and that script checks if "cron_restart_backend" is present in the log file. If so it restarts the application?

Btw, python app is supervised by s6, so if it is killed, s6 will start it immediately.
pkill -f "python " && (python ${INSTALL_DIR}/server > /dev/null 2>&1 &) && echo 'done'
So no need to run it again after pkill.

And what would be the reason to have "cron_restart_backend" in log file?

@jokob-sk
Copy link
Owner

jokob-sk commented Jan 9, 2025

Ok, so cron runs cron_script.sh every minute and that script checks if "cron_restart_backend" is present in the log file. If so it restarts the application?

Yes

Btw, python app is supervised by s6, so if it is killed, s6 will start it immediately.
pkill -f "python " && (python ${INSTALL_DIR}/server > /dev/null 2>&1 &) && echo 'done'
So no need to run it again after pkill.

  • oh, great will remove it with the next commit

image

It's mostly used during development to load up the new version of the code or when a script hangs.

@vladaurosh
Copy link
Contributor Author

I'll try in next couple of days to see why cron doesn't work properly for you when started via s6.

@jokob-sk
Copy link
Owner

jokob-sk commented Jan 9, 2025 via email

@vladaurosh
Copy link
Contributor Author

@jokob-sk
PR for this coming in next couple of hours.
That will fix #965

@jokob-sk
Copy link
Owner

jokob-sk commented Jan 19, 2025 via email

@vladaurosh
Copy link
Contributor Author

Ne errors in the image build, or when container starts. It seems to be the warning thrown by dcron, every second.
Looks like it happens when crond is not started as pid 1 (which is the case here).

vladaurosh added a commit to vladaurosh/Pi.Alert that referenced this issue Jan 19, 2025
@jokob-sk
Copy link
Owner

I assume this is fixed in the dev image - adding the appropriate label - thanks for the help @vladaurosh 🙏

@jokob-sk jokob-sk added next release/in dev image🚀 This is coming in the next release or was already released if the issue is Closed. and removed In development 🏗 labels Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request➕ New feature or request next release/in dev image🚀 This is coming in the next release or was already released if the issue is Closed.
Projects
None yet
Development

No branches or pull requests

2 participants