Skip to content

restart update_synapse_for_appservice.sh broken for whatsapp #580

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.

Sign up for GitHub

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

Open
nin0-0 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

restart update_synapse_for_appservice.sh broken for whatsapp #580

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions sources/update_synapse_for_appservice.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
#!/bin/bash

set -eu

app=__APP__
YNH_HELPERS_VERSION=2.1
YNH_APP_ACTION=''
YNH_STDINFO=/dev/stdout
source /usr/share/yunohost/helpers

port_synapse_tls=$(ynh_app_setting_get --key=port_synapse_tls)
service_config_file="/etc/matrix-$app/conf.d/app_service.yaml"
backup_app_service=$(mktemp)

Expand All @@ -17,11 +14,9 @@ cp "$service_config_file" "$backup_app_service"

if [ -n "$(ls "/etc/matrix-$app/app-service/")" ]; then
echo "app_service_config_files:" > "$service_config_file"

for f in "/etc/matrix-$app/app-service/"*; do
echo " - $f" >> "$service_config_file"
done

chown "$app" "/etc/matrix-$app/app-service/"*
chmod 600 "/etc/matrix-$app/app-service/"*
else
Expand All @@ -31,18 +26,23 @@ chown "$app" "$service_config_file"
chmod 600 "$service_config_file"

set +e
ynh_systemctl --service="$app".service --action=restart --wait_until="Synapse now listening on TCP port $port_synapse_tls" --log_path="/var/log/matrix-$app/homeserver.log" --timeout=300

@Josue-T Josue-T Oct 18, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would prefer to keep the same parameter than on the install/upgrade script

synapse_ynh/scripts/install

Line 248 in 5577f5e

ynh_systemctl --service="$app".service --action=restart --wait_until="Synapse now listening on TCP port $port_synapse_tls" --log_path="/var/log/matrix-$app/homeserver.log" --timeout=300

@nin0-0 nin0-0 Oct 18, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these checks in a log file are solid, I've had to echo "somestrings" before because an install was failing when the log messages were changed/not appearing, even when the service was started and active.

@Josue-T Josue-T Oct 19, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok but in this case can you please report the issue so we can fix it everywhere and not just in this script.

@nin0-0 nin0-0 Oct 19, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be I am having a more general logging issue? I haven't had the log message in that log since a week - maybe I'm trying to solve a me-issue only

if any idea, let me know, we can close for now

@Josue-T Josue-T Oct 19, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well at last on my side and on the CI we don't seem to have any issue like this. I mean Yunhost correctly detect when the service started and we don't end on the timeout.

ynh_systemctl --service="$app" --action=restart --log_path=systemd
res=$?
set -e
set +x

if [ $res -eq 0 ]; then
rm "$backup_app_service"
exit 0
else
# Wait a moment and verify service is actually running
sleep 5
if ! systemctl is-active --quiet "$app"; then

@Josue-T Josue-T Oct 18, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that it's more strong, because even if it fail as the service will try to restart depending on when you do the check, it might return that it's active even if it crashed.

@nin0-0 nin0-0 Oct 18, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe need to put is-failed instead of is-active?

is-active PATTERN... Check whether units are active
is-failed PATTERN... Check whether units are failed

@Josue-T Josue-T Oct 19, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well yes maybe, to me we need a strong way to check that the service fully started without crashing at some point and it's from my experience what was doing the current code. Now if this not working as expected maybe we just need to understand why and maybe fix the helper itself.

The idea also of this way we check the startup is that we are sure that synapse fully started when we go out of ynh_systemctl and this time could really depends of servers.

echo "Failed to restart synapse with the new config file. Restore the old config file !!"
cp "$backup_app_service" "$service_config_file"
rm "$backup_app_service"
ynh_systemctl --service="$app".service --action=restart --wait_until="Synapse now listening on TCP port $port_synapse_tls" --log_path="/var/log/matrix-$app/homeserver.log" --timeout=300
exit 1
ynh_systemctl --service="$app" --action=restart --log_path=systemd
sleep 5
if ! systemctl is-active --quiet "$app"; then
echo "ERROR: Synapse failed to start even after restoring config!"
exit 1
fi
fi

rm "$backup_app_service"
exit 0