Improve .deb package creation #88
Conversation
lampkin-diet
commented
Jul 22, 2021
lampkin-diet
commented
- Add systemd service creation after install .deb package
- Add post remove action for .deb package
- Add scripts for building .deb package and tar archive
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
| touch ${{ env.OUTPUT_DIR }}/${{ env.PACKAGE_NAME }}_${{ env.VERSION }}.tar.gz | ||
| tar -czf ${{ env.OUTPUT_DIR }}/${{ env.PACKAGE_NAME }}_${{ env.VERSION }}.tar.gz /home/runner/go/bin/cheqd-noded | ||
| ./build_tar.sh ${{ env.PACKAGE_NAME }} ${{ env.VERSION }} | ||
| working-directory: ./build_tools |
There was a problem hiding this comment.
It seems strange to build that tar inside of the build_tools directory. In generally I try to make scripts to not rely on relative pathing, so that that the working directory isn't an issue. Not sure how hard that would be here with GitHub Actions, but something to think about.
Regardless, It might make more sense to have the build_tar.sh change into whatever directories it needs, and have the tar built in one level up. So it would be sitting in the same directory as build_tools/ sits.
Again... what you have it probably fine, but something to think about for the future, and have less dependency on what directory to run certain scripts from.
build_tools/build_deb.sh
Outdated
| --name "cheqd-node" \ | ||
| --description "cheqd node" \ | ||
| --architecture "${ARCH}" \ | ||
| --pre-install "postinst" \ |
There was a problem hiding this comment.
Shouldn't this be --after-install? In Debian, a preinst script is executed before the archive is extracted to the filesystem.
There was a problem hiding this comment.
For now it's not very significant to run this script after install of before because we don't use cheqd-noded binary while making actions. But yes, you are right, it would be more clearly to run it --after-install
build_tools/postinst
Outdated
| @@ -0,0 +1,79 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
This is a script that will be executed after EVERY install/upgrade of the .deb
So you'll need to make it idempotent
I know you want this script to be runnable as a tar installation, but here is an example postinst script I wrote for a personal project of mine that is idempotent and also managed systemd service, you could probably adapt as necessary: https://github.com/absltkaos/toddleglow/blob/master/debian/toddleglow.postinst
build_tools/postinst
Outdated
| CHEQD_USER_NAME=cheqd | ||
|
|
||
| # Create cheqd user | ||
| useradd -d /home/$CHEQD_USER_NAME -m $CHEQD_USER_NAME |
There was a problem hiding this comment.
This will only succeed the first time the .deb is installed... after which useradd will exit non-zero and you'll have issues.
Also this should be a "system" user, so as not to conflict with people's user management (like an LDAP etc...). Also change the home directory so it is not in /home which is reserved for actual human users.
I suggest wrapping this in an if statement to look something like this:
if ! /usr/bin/getent passwd $CHEQD_USER_NAME > /dev/null 2>&1 ; then
adduser --system ${CHEQD_USER_NAME} --home /var/lib/${CHEQD_USER_NAME}
fi
build_tools/postinst
Outdated
| EOF | ||
|
|
||
| # Add crontab job for daily rotation | ||
| cat <<EOF > /etc/cron.daily/cheqd-node |
There was a problem hiding this comment.
Why do this? Logrotate should already be running via cron.daily and the file is part of the logrotate package
$ dpkg -S /etc/cron.daily/logrotate
logrotate: /etc/cron.daily/logrotate
build_tools/postinst
Outdated
| chmod +x /etc/cron.daily/cheqd-node | ||
|
|
||
| # Restart syslog | ||
| systemctl restart rsyslog |
There was a problem hiding this comment.
Again... what if I don't use rsyslog for log management? I don't think this should be here.
Maybe only do a restart of rsyslog IF this postinst script created /etc/rsyslog.d/cheqd-node.conf in the if statement further up and the rsyslog service is already running.
build_tools/postinst
Outdated
| systemctl restart rsyslog | ||
|
|
||
| # Add systemd script | ||
| cat <<EOF > /etc/systemd/system/cheqd-noded.service |
There was a problem hiding this comment.
This file should be included in the tar file under /lib/systemd/system NOT /etc/systemd/system (which is reserved for admins to override values in the systemd service provided by the package etc..)
Adding it here ties the Admin's hands on being able to customize the service.
build_tools/postinst
Outdated
| [Service] | ||
| Type=simple | ||
| User=cheqd | ||
| ExecStart=/bin/bash -c '/usr/bin/cheqd-noded start' |
There was a problem hiding this comment.
Why does this need to be wrapped in bash -c? Can't it just run directly?
build_tools/postremove
Outdated
| if test -f "/etc/rsyslog.d/cheqd-node.conf"; then | ||
| rm /etc/rsyslog.d/cheqd-node.conf | ||
| # Restart rsyslog daemon | ||
| systemctl restart rsyslog |
There was a problem hiding this comment.
Similar comments on this hard dependency on rsyslog
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
Signed-off-by: Andrew Nikitin <[email protected]>
