fff-layer3: Make it easier to keep the settings in testmode #9

ChristianD · 2020-12-13T09:44:41+01:00

ChristianD commented

2020-12-13 09:44:41 +01:00

On call -t we write the pid on /tmp/configure-layer3-pid.
If the script exits from user we use trap to run the new function keep_changes()
If the connections to the router lost, the user can run configure-layer3 -k after
reconnect to keep changes manually

Signed-off-by: Christian Dresel freifunk@dresel.systems

On call -t we write the pid on /tmp/configure-layer3-pid. If the script exits from user we use trap to run the new function keep_changes() If the connections to the router lost, the user can run configure-layer3 -k after reconnect to keep changes manually Signed-off-by: Christian Dresel <freifunk@dresel.systems>

adschm reviewed 2020-12-13 13:47:13 +01:00

src/packages/fff/fff-gateway/files/usr/sbin/configuregateway Outdated

						
				@ -69,0 +73,4 @@

						rm /tmp/configuregatewaypid

						exit;

					else

						echo "No configuregateway -t is running"

adschm commented

2020-12-13 13:47:13 +01:00

"configuregateway -t is not running"

ChristianD marked this conversation as resolved

adschm reviewed 2020-12-13 13:47:42 +01:00

src/packages/fff/fff-gateway/files/usr/sbin/configuregateway Outdated

						
				@ -86,6 +104,7 @@ usage() {

					echo "Options:"

					echo "	-c: configure. No commit, no restart!"

					echo "	-t: test changes. Restarts services, waits up to 200s for SIGINT"

					echo "	-k: Keep changes in Testmode"

adschm commented

2020-12-13 13:47:42 +01:00

"test mode"

adschm commented

2020-12-13 13:48:22 +01:00

Äh, und besser "keep changes from test mode"

ChristianD marked this conversation as resolved

adschm added the

layer3

packages/fff

labels 2020-12-13 14:01:37 +01:00

ChristianD force-pushed keepchanges from 9c2d1ad377 to 8566f77725

2020-12-14 12:03:52 +01:00

Compare

adschm added the

feature

label 2020-12-14 14:16:09 +01:00

ChristianD force-pushed keepchanges from 8566f77725 to f8b9f70009

2020-12-16 23:49:47 +01:00

Compare

ChristianD commented

2020-12-16 23:57:08 +01:00

vielleicht noch eine kurze Erklärung was das trap macht, weil es nicht unbedingt was gewöhnliches ist.

Mit trap kann man Signale die von außen an das Script gesendet werden abfangen (z.b. STRG+C oder kill/killall) und im Script noch eine Funktion (in dem Fall keep_changes()) aufrufen bevor es sich bei kill/killall oder STRG+C beendet.

Es gibt dazu ein ganzen haufen verschiedener Signale (wobei für uns nur die oben genannten Situationen interessant sind):

https://de.wikipedia.org/wiki/Signal_(Unix)

Für uns sind folgende Signale wichtig:
SIGINT => Der User bricht das Script mit STRG+C ab
SIGTERM => Der User killt das Programm z.b. mit kill oder killall (was eher selten passiert, aber wir sollten es mit abfangen da früher so gearbeitet wurde)

In beiden Fällen gehen wir davon aus, das der User die Einstellungen behalten will und wir rufen keep_changes() auf.

Anfangs hatte ich noch SIGHUP drinnen, das ist aber falsch. SIGHUP wird an das Script gesendet wenn das Terminal geschlossen wird (darunter fällt auch eine abgebrochene SSH Session), da es aber durchaus passieren kann, das beim Netzwerk rekonfigurieren eine SSH Session abreist, dürfen wir in dem Fall kein keep_changes() aufrufen.

vielleicht noch eine kurze Erklärung was das trap macht, weil es nicht unbedingt was gewöhnliches ist. Mit trap kann man Signale die von außen an das Script gesendet werden abfangen (z.b. STRG+C oder kill/killall) und im Script noch eine Funktion (in dem Fall keep_changes()) aufrufen bevor es sich bei kill/killall oder STRG+C beendet. Es gibt dazu ein ganzen haufen verschiedener Signale (wobei für uns nur die oben genannten Situationen interessant sind): https://de.wikipedia.org/wiki/Signal_(Unix) Für uns sind folgende Signale wichtig: SIGINT => Der User bricht das Script mit STRG+C ab SIGTERM => Der User killt das Programm z.b. mit kill oder killall (was eher selten passiert, aber wir sollten es mit abfangen da früher so gearbeitet wurde) In beiden Fällen gehen wir davon aus, das der User die Einstellungen behalten will und wir rufen keep_changes() auf. Anfangs hatte ich noch SIGHUP drinnen, das ist aber falsch. SIGHUP wird an das Script gesendet wenn das Terminal geschlossen wird (darunter fällt auch eine abgebrochene SSH Session), da es aber durchaus passieren kann, das beim Netzwerk rekonfigurieren eine SSH Session abreist, dürfen wir in dem Fall kein keep_changes() aufrufen.

rohammer reviewed 2020-12-17 01:07:35 +01:00

src/packages/fff/fff-gateway/files/usr/sbin/configuregateway Outdated

						
				@ -76,2 +92,3 @@

					echo "Reverting changes.."

					echo "Reverting changes.."

					# on revert we must delete the file

rohammer commented

2020-12-17 01:07:35 +01:00

... delete the pid-file

ChristianD commented

2020-12-18 11:30:24 +01:00

rebased und @rohammer Anmerkung ausgebessert

ChristianD marked this conversation as resolved

ChristianD force-pushed keepchanges from f8b9f70009 to 94d9f87e0a

2020-12-18 11:29:02 +01:00

Compare

ChristianD commented

2020-12-18 11:30:38 +01:00

rebased und @rohammer Anmerkung ausgebessert

ChristianD changed title from ~~fff-gateway: Make it easier to keep the settings in testmode~~ to fff-layer3: Make it easier to keep the settings in testmode

2020-12-18 11:31:06 +01:00

rohammer approved these changes 2020-12-22 00:30:14 +01:00

rohammer left a comment

Reviewed-by: Robert Langhammer <rlanghammer@web.de>

``` Reviewed-by: Robert Langhammer <rlanghammer@web.de> ```

src/packages/fff/fff-gateway/files/usr/sbin/configuregateway Outdated

						
				@ -73,2 +87,3 @@

					echo "Configuration reloaded. Changes will be reverted in 200s."

					echo "Kill this script to keep changes."

					echo "Kill this script or use configuregateway -k to keep changes."

rohammer commented

2020-12-17 01:09:59 +01:00

Hi,
das klingt so, als ob kill die changes nicht behaelt.
Eigentlich willst du doch sagen:

To keep changes, kill this script or use configuregateway -k.

Ansonsten:

Reviewed-by: Robert Langhammer <rlanghammer@web.de>

Hi, das klingt so, als ob kill die changes nicht behaelt. Eigentlich willst du doch sagen: ``` To keep changes, kill this script or use configuregateway -k. ``` Ansonsten: ``` Reviewed-by: Robert Langhammer <rlanghammer@web.de> ```

ChristianD commented

2020-12-22 13:23:50 +01:00

Also für mich heißt das übersetzt:

"Töte dieses Script ODER nutzte configuregateway -k um die Einstellungen zu behalten"

Finde ich eigentlich passend oder?

Übrigens kp wo du das eben hier im Code kommentiert hast, aber nach meiner Sicht ist hier bereits von configure-layer3 die Rede ;)

Also für mich heißt das übersetzt: "Töte dieses Script ODER nutzte configuregateway -k um die Einstellungen zu behalten" Finde ich eigentlich passend oder? Übrigens kp wo du das eben hier im Code kommentiert hast, aber nach meiner Sicht ist hier bereits von configure-layer3 die Rede ;)

adschm reviewed 2020-12-28 16:45:44 +01:00

src/packages/fff/fff-layer3-config/files/usr/sbin/configure-layer3 Outdated

						
				@ -69,0 +71,4 @@

						echo "Keep changes"

						kill -9 $(cat /tmp/configure-layer3-pid)

						rm /tmp/configure-layer3-pid

						exit;

adschm commented

2020-12-28 16:45:44 +01:00

Why do we need that exit?

ChristianD commented

2020-12-28 17:18:29 +01:00

Bin mir nicht mehr ganz sicher aber ich glaube der Grund war wenn das trap in Zeile 82 die Funktion keep_changes() aufruft, müssen wir es sicher beenden da sonst test_changes() dennoch weiterläuft und am Ende alles revertet was ja in dem Fall nicht gewünscht ist.

adschm commented

2020-12-28 17:25:28 +01:00

Hmm, also exit != return ...

In jedem Fall kann aber der Strichpunkt weg. Und man sollte hierfür einen Kommentar einfügen.

Hmm, also exit != return ... In jedem Fall kann aber der Strichpunkt weg. Und man sollte hierfür einen Kommentar einfügen.

ChristianD commented

2020-12-28 17:31:34 +01:00

Ja exit soll das Script auf jeden Fall sicher beenden weil wir hier dann fertig sind. Kommentar und ; entfernen mach ich gleich

ChristianD marked this conversation as resolved

ChristianD force-pushed keepchanges from 94d9f87e0a to ac6e876581

2020-12-28 17:34:59 +01:00

Compare