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

Allow script to continue flashing F/W if connection is terminated. #273

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

Martinski4GitHub
Copy link
Collaborator

@Martinski4GitHub Martinski4GitHub commented Jul 25, 2024

Made changes to allow script to continue with the F/W flash "curl" command after connection is terminated due to stopping Entware services, including TailScale.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Made changes to allow script to continue with the F/W flash "curl" command after connection is terminated due to stopping Entware services, including TailScale.
@ExtremeFiretop
Copy link
Owner

ExtremeFiretop commented Jul 25, 2024

I'm wondering, with this solution, do we even have a need to keep tailscale alive anymore?
Should we walk back/rollback that feature before we get too far along?

@ExtremeFiretop ExtremeFiretop merged commit 4f5cbb9 into ExtremeFiretop:dev Jul 25, 2024
2 checks passed
@jksmurf
Copy link

jksmurf commented Jul 25, 2024

I very much agree with Extremefiiretop here. Tailmon is set up to restart on reboot and force restart when it is not running, so as long as the MerlinAU process completes the FW update without Tailmon interfering, then having one less step where things can go wrong is a good idea..Simple is good.

Tailmon will (should) restart once the router reboots after the FW update, so the user will get access back (as long as the FW itself didn’t introduce other issues, but keeping a tailscale alive won’t fix those).k.

@Martinski4GitHub
Copy link
Collaborator Author

I'm wondering, with this solution, do we even have a need to keep tailscale alive anymore? Should we walk back/rollback that feature before we get too far along?

Currently, the call to stop the Entware services is made for 2 reasons:

1) To free some RAM in the case where the router is found in a "low memory" state when the script is checking for "Required vs Available" RAM to be able to continue with the "pre-flash" checks before doing the actual F/W flash.

2) Right before ejecting the USB-attached drive which is immediately before starting the actual F/W flash.

In case 1) above, we do not want to stop TailScale because it's too early to terminate the connection so we do actually need to keep TailScale "alive" at this point during the "pre-flash" checks.

In case 2) above, we're now stopping all services, no exceptions, so we can eject the USB-attached drive successfully.

@ExtremeFiretop
Copy link
Owner

ExtremeFiretop commented Jul 26, 2024

I'm wondering, with this solution, do we even have a need to keep tailscale alive anymore? Should we walk back/rollback that feature before we get too far along?

Currently, the call to stop the Entware services is made for 2 reasons:

1) To free some RAM in the case where the router is found in a "low memory" state when the script is checking for "Required vs Available" RAM to be able to continue with the "pre-flash" checks before doing the actual F/W flash.

2) Right before ejecting the USB-attached drive which is immediately before starting the actual F/W flash.

In case 1) above, we do not want to stop TailScale because it's too early to terminate the connection so we do actually need to keep TailScale "alive" at this point during the "pre-flash" checks.

In case 2) above, we're now stopping all services, no exceptions, so we can eject the USB-attached drive successfully.

@Martinski4GitHub .

I guess my thought is just remove the first Entware stop and stop everything at once on the second step..
Only leave the second Entware stop after the "trap" is called to ignore hangups.

That way, it's trapped, then we stop Entware services. Up until that point we can keep everything as it was, no?

@Martinski4GitHub
Copy link
Collaborator Author

I'm wondering, with this solution, do we even have a need to keep tailscale alive anymore? Should we walk back/rollback that feature before we get too far along?

Currently, the call to stop the Entware services is made for 2 reasons:
1) To free some RAM in the case where the router is found in a "low memory" state when the script is checking for "Required vs Available" RAM to be able to continue with the "pre-flash" checks before doing the actual F/W flash.
2) Right before ejecting the USB-attached drive which is immediately before starting the actual F/W flash.
In case 1) above, we do not want to stop TailScale because it's too early to terminate the connection so we do actually need to keep TailScale "alive" at this point during the "pre-flash" checks.
In case 2) above, we're now stopping all services, no exceptions, so we can eject the USB-attached drive successfully.

@Martinski4GitHub .

I guess my thought is just remove the first Entware stop and stop everything at once on the second step.. Only leave the second Entware stop after the "trap" is called to ignore hangups.

That way, it's trapped, then we stop Entware services. Up until that point we can keep everything as it was, no?

AFAIR, the main goal of the 1st call to stop Entware services was to try to free as much RAM as possible when the router is found to be below the "Required RAM" threshold. For routers with > 1GB RAM, or people with 1 or 2 services running and a couple of add-ons, having "Low RAM" may be an unlikely scenario, but some people have a lot of Entware services in addition to many add-ons running simultaneously. For such cases, I believe the 1st call is still valid and may still be useful.

P.S.
I have a work meeting in ~10 minutes. I'll take a look at the other messages and respond later in the evening. I expect a busy workday today.

@ExtremeFiretop
Copy link
Owner

ExtremeFiretop commented Jul 26, 2024

I'm wondering, with this solution, do we even have a need to keep tailscale alive anymore? Should we walk back/rollback that feature before we get too far along?

Currently, the call to stop the Entware services is made for 2 reasons:
1) To free some RAM in the case where the router is found in a "low memory" state when the script is checking for "Required vs Available" RAM to be able to continue with the "pre-flash" checks before doing the actual F/W flash.
2) Right before ejecting the USB-attached drive which is immediately before starting the actual F/W flash.
In case 1) above, we do not want to stop TailScale because it's too early to terminate the connection so we do actually need to keep TailScale "alive" at this point during the "pre-flash" checks.
In case 2) above, we're now stopping all services, no exceptions, so we can eject the USB-attached drive successfully.

@Martinski4GitHub .
I guess my thought is just remove the first Entware stop and stop everything at once on the second step.. Only leave the second Entware stop after the "trap" is called to ignore hangups.
That way, it's trapped, then we stop Entware services. Up until that point we can keep everything as it was, no?

AFAIR, the main goal of the 1st call to stop Entware services was to try to free as much RAM as possible when the router is found to be below the "Required RAM" threshold. For routers with > 1GB RAM, or people with 1 or 2 services running and a couple of add-ons, having "Low RAM" may be an unlikely scenario, but some people have a lot of Entware services in addition to many add-ons running simultaneously. For such cases, I believe the 1st call is still valid and may still be useful.

P.S. I have a work meeting in ~10 minutes. I'll take a look at the other messages and respond later in the evening. I expect a busy workday today.

Ah yes, I see now, the first entware stop is at line 2143, this is within the check_memory_and_prompt_reboot function.
NOT within the RunFirmwareUpdateNow function.

Initially I thought you kept the entware stop at line 4873 and just added a "full stop" down below in the same function at line 4885

@ExtremeFiretop
Copy link
Owner

Now in hindsight, I see you actually removed the Entware stop call at line 4873 and only moved it DOWN to line 4885, instead of duplicating it.

@ExtremeFiretop
Copy link
Owner

ExtremeFiretop commented Jul 26, 2024

We can ignore the first "remove the first entware stop" line below i mentioned, I was confused when I wrote that.

I guess my thought is just remove the first Entware stop and stop everything at once on the second step..

However, this still remains of my question:

Only leave the second Entware stop after the "trap" is called to ignore hangups.

That way, it's trapped, then we stop Entware services. Up until that point we can keep everything as it was, no?

Basically, in my head I read you had added a second within the same function, but we do not, we have 1 in each function as expected. But still my question remains, do we need to keep VPN active?

In my mind now, the answer is yes now. But ONLY in a case where the "check_memory_and_prompt_reboot" function runs, since it will stop all Entware services EXCEPT tailscale, at line 2143:

                # Stop Entware services before F/W flash #
                _EntwareServicesHandler_ stop

If we did NOT include this feature, then theres potential for the memory function to randomly run, and boot you from your tailscale connection, before reaching the "no-hang-up" step.

@Martinski4GitHub
Copy link
Collaborator Author

In my mind now, the answer is yes now. But ONLY in a case where the "check_memory_and_prompt_reboot" function runs, since it will stop all Entware services EXCEPT tailscale, at line 2143:

                # Stop Entware services before F/W flash #
                _EntwareServicesHandler_ stop

If we did NOT include this feature, then theres potential for the memory function to randomly run, and boot you from your tailscale connection, before reaching the "no-hang-up" step.

Exactly. When Entware services are stopped due to a "low RAM" check, Tailscale must remain "alive" still so the user can reach the final step just before the actual F/W flash starts.

@ExtremeFiretop
Copy link
Owner

Synced with Gnuton in commit: 80e79ae

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants