Skip to content

Quick Patch for improved safety#45

Merged
ExtremeFiretop merged 15 commits intomainfrom
dev
Dec 20, 2023
Merged

Quick Patch for improved safety#45
ExtremeFiretop merged 15 commits intomainfrom
dev

Conversation

@ExtremeFiretop
Copy link
Owner

No description provided.

ExtremeFiretop and others added 13 commits December 18, 2023 21:37
Added a background child process to simply wait for 6 minutes while the F/W Update is in progress. If after 6 minutes the curl process still exists, the background task kills it; otherwise does nothing & returns. This is our "escape hatch" to handle the rare possibility that the curl command never completes the F/W Update. If the F/W update completes normally, the background task will simply do nothing and may get terminated if the router reboots itself.
Added code to check if the router is reporting a new F/W Update is available so that we can modify the notification settings accordingly.
Added code to restart the WebGUI just before our own login is initiated to begin the actual F/W Update process. This ensures that nobody else is logged in at that moment so that we can do process without any interruptions.
@Martinski4GitHub
Copy link
Collaborator

Things are looking very good & much more polished than ever. Certainly production-ready.

Copy link
Collaborator

@Martinski4GitHub Martinski4GitHub left a comment

Choose a reason for hiding this comment

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

All good to go.

@ExtremeFiretop ExtremeFiretop merged commit fc6db9c into main Dec 20, 2023
@ExtremeFiretop
Copy link
Owner Author

All good to go.

I had turned on auto-merge but also turned on requiring signed commits and that was blocking the auto-merge.
I've disabled the need for signed commits for now. As soon as I did it successfully merged.

@ExtremeFiretop
Copy link
Owner Author

Things are looking very good & much more polished than ever. Certainly production-ready.

Just in time!

I see GT-AXE11000_3004_388.6_alpha1 is now available, we will get to watch it update to 388.6 once alpha/beta phases are passed :)

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Dec 21, 2023

All good to go.

BTW @Martinski4GitHub , I can't think of any other PRs we need for now, so until we think of more I'm not sure when the next time we shall speak. I just wanted to wish you a happy Christmas or happy holidays. Add a happy new year with that as well!

Working on this project with you has not only improved my skills and understanding, but was honestly fun to do and workaround/troubleshoot the issues as we found them. In my ordinal post on the forums this is truly what I pictured when I asked for help, and I think you knew that reading my comments while I was being slaughtered there. lol

I will be forever thankful to have someone else to bounce ideas off, this is not just 'my' design; it's 'our' design hands down.
I'm sure it's not over, we will surely find more models with exceptions, or specific tweaks that need to be made to maintain the script. But man has the result so far been A+ work if I do say so myself ;)

@ExtremeFiretop
Copy link
Owner Author

AND let me just add, to a project people thought was literally IMPOSSIBLE lol.

@ExtremeFiretop
Copy link
Owner Author

I'm also starting to think in the new year, after we test on the latest release (388.6 once it's ready.
That I'll make a post in the forums add-in section to get a bit of visibility, let me know if your for/against that :)

@ExtremeFiretop
Copy link
Owner Author

Actually, maybe not, because this is twice now that I've tested an upgrade and everytime I do my USB comes back as: "SDA(1)" instead of "SDA"

image

image

Post update I always seem to need to reset the USB and start over for some reason else the script gives me this:

image

I could change the directory to the new: "SDA(1)" but that's not what I want to use lol!

@Martinski4GitHub
Copy link
Collaborator

All good to go.

BTW @Martinski4GitHub , I can't think of any other PRs we need for now, so until we think of more I'm not sure when the next time we shall speak. I just wanted to wish you a happy Christmas or happy holidays. Add a happy new year with that as well!

Working on this project with you has not only improved my skills and understanding, but was honestly fun to do and workaround/troubleshoot the issues as we found them. In my ordinal post on the forums this is truly what I pictured when I asked for help, and I think you knew that reading my comments while I was being slaughtered there. lol

I will be forever thankful to have someone else to bounce ideas off, this is not just 'my' design; it's 'our' design hands down. I'm sure it's not over, we will surely find more models with exceptions, or specific tweaks that need to be made to maintain the script. But man has the result so far been A+ work if I do say so myself ;)

Thank you very much. Happy Holidays to you & your family and a very Happy New Year!!

It has certainly been a fun, positive experience & collaboration for me as well as we worked together to find solutions to issues or made improvements to have a more polished script with a user-friendly interface. I like your approach to the project & the enthusiasm to make this a kind of "partnership" without any of the drama, cynicism & negativity I have found in other projects.

Off we go to a better 2024. Cheers Mate!!

@Martinski4GitHub
Copy link
Collaborator

Martinski4GitHub commented Dec 22, 2023

I'm also starting to think in the new year, after we test on the latest release (388.6 once it's ready. That I'll make a post in the forums add-in section to get a bit of visibility, let me know if your for/against that :)

I have no problems at all with that. Generally in the past, the SNB Forums folks have shown hostility toward any "Automatic F/W Updates" script so they may not be the most friendly audience; OTOH, once some people see the final product, they may warm to the idea and perhaps at least try it to see how it can work for them.

@Martinski4GitHub
Copy link
Collaborator

Martinski4GitHub commented Dec 22, 2023

Actually, maybe not, because this is twice now that I've tested an upgrade and everytime I do my USB comes back as: "SDA(1)" instead of "SDA"

Does your USB drive partition have a volume name assigned to it?

If not, that would be the solution to these problems. Years ago, I saw something like this happen with some USB flash drives, but once I gave a volume name to each drive partition and used that name as part of the mount point paths (e.g. /tmp/mount/MyAsusUSB/) in my scripts, I've never had those issues.

P.S. Ah, just realized that the volume name is SDA. Strange that it would not take that as a unique name for the mount point path and create the "SDA(1)" instead as if the original was already taken by another drive.

What is the output of the following commands on that router?
blkid
grep "/dev/sd.* /tmp/mnt/.*" /proc/mounts | awk -F ' ' '{print $1,$2,$3}'

@ExtremeFiretop
Copy link
Owner Author

Actually, maybe not, because this is twice now that I've tested an upgrade and everytime I do my USB comes back as: "SDA(1)" instead of "SDA"

Does your USB drive partition have a volume name assigned to it?

If not, that would be the solution to these problems. Years ago, I saw something like this happen with some USB flash drives, but once I gave a volume name to each drive partition and used that name as part of the mount point paths (e.g. /tmp/mount/MyAsusUSB/) in my scripts, I've never had those issues.

P.S. Ah, just realized that the volume name is SDA. Strange that it would not take that as a unique name for the mount point path and create the "SDA(1)" instead as if the original was already taken by another drive.

What is the output of the following command on that router? grep "/dev/sd.* /tmp/mnt/.*" /proc/mounts | awk -F ' ' '{print $1,$2,$3}'

Output is: /dev/sda /tmp/mnt/SDA tfat

image

So far to what I can tell, my logging solution is keeping the USB busy and blocking the unmount command. Might even be part of the reason my AX88U_Pro needed a manual reboot at the end? Not sure...

But when I run the script with the actual flash commands commented out and throw an unmount in there, it fails telling me the device is still busy, when I remove my logging that problem goes away:

image

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Dec 22, 2023

Basically after the reboot it comes back online with SDA and SDA(1) as found here:

image

So far the solution is to remove the logging to the USB path, and/or add something like this:

# Check if USBMountPoint is set
if [ -z "$USBMountPoint" ]; then
    printf "USB mount point is not set. Skipping unmount."
else
        # Unmount the USB drive
        Say "Unmounting the USB drive at $USBMountPoint..."
        umount "$USBMountPoint"

        # Check if the umount command was successful
        if [ $? -eq 0 ]; then
            Say "USB drive successfully unmounted."

            # Check if the mount point directory exists
            if [ -d "$USBMountPoint" ]; then
                Say "Removing the mount point directory..."
                rm -r "$USBMountPoint"

                # Check if the rm command was successful
                if [ $? -eq 0 ]; then
                    Say "Mount point directory successfully removed."
                else
                    Say "Failed to remove the mount point directory."
                fi
            else
                Say "Mount point directory does not exist."
            fi
        else
            Say "Failed to unmount the USB drive."
        fi
    fi

With this above, post reboot, the SDA mount point is gone so it re-mounts it fresh and new

@ExtremeFiretop
Copy link
Owner Author

The reason I add the code to remove the mount point is due to the fact that even when I unmount it successfully, the folder remains in the mnt directory as found below (And yes I am refreshing WinSCP):

image

(And yes I am refreshing WinSCP)

@ExtremeFiretop
Copy link
Owner Author

What is the output of the following commands on that router? blkid

I forgot one of your outputs:

image

@Martinski4GitHub
Copy link
Collaborator

So far the solution is to remove the logging to the USB path, and/or add something like this:

What about moving up the following lines and put them right before the curl command for the actual F/W Update:
} 2>&1 | tee -a "$LOG_FILE"
#END: Redirect both stdout and stderr to log file #
This should release any file handle on the USB drive used for the logfile, so when the F/W Update process tries to unmount the USB drive it should work.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Dec 22, 2023

So far the solution is to remove the logging to the USB path, and/or add something like this:

What about moving up the following lines and put them right before the curl command for the actual F/W Update: } 2>&1 | tee -a "$LOG_FILE" #END: Redirect both stdout and stderr to log file # This should release any file handle on the USB drive used for the logfile, so when the F/W Update process tries to unmount the USB drive it should work.

Naturally we lose the logging of the curl but I was thinking of testing that as well and seeing the result, it was on my to-do list lol!

@ExtremeFiretop
Copy link
Owner Author

My remaining question is if just moving the logging alone is enough or if I really do need to remove the mount point as well.

It's odd to me that the SDA folder still exists after I unmount. Makes me think just moving the logging isn't enough, that may unblock the USB from being unmounted, but if the mnt point still exists it will still re-mount as SDA(1)

@Martinski4GitHub
Copy link
Collaborator

Naturally we lose the logging of the curl but I was thinking of testing that as well and seeing the result, it was on my to-do list lol!

Yes, but we could handle that separately by logging into the system logfile.
Quick example:


if nohup curl "${routerURLstr}/upgrade.cgi"
.....
.....
then
Say "${REDct}ERROR${NOct}: F/W Update successful."
else
Say "${REDct}ERROR${NOct}: F/W Update failed."
fi


@Martinski4GitHub
Copy link
Collaborator

My remaining question is if just moving the logging alone is enough or if I really do need to remove the mount point as well.

It's odd to me that the SDA folder still exists after I unmount. Makes me think just moving the logging isn't enough, that may unblock the USB from being unmounted, but if the mnt point still exists it will still re-mount as SDA(1)

I have seen the "unmount" calls take quite a few seconds, depending on what else might be running from it (e.g. Entware services), SMB, NFS, etc. So put some wait after it (e.g. sleep 10) to test how it works on your system.

For example, the F/W Update process tries to stop all Entware services & user space processes to minimize the chances of the USB being "busy" at the time of the unmount, and still takes a few seconds.

@ExtremeFiretop
Copy link
Owner Author

My remaining question is if just moving the logging alone is enough or if I really do need to remove the mount point as well.
It's odd to me that the SDA folder still exists after I unmount. Makes me think just moving the logging isn't enough, that may unblock the USB from being unmounted, but if the mnt point still exists it will still re-mount as SDA(1)

I have seen the "unmount" calls take quite a few seconds, depending on what else might be running from it (e.g. Entware services), SMB, NFS, etc. So put some wait after it (e.g. sleep 10) to test how it works on your system.

For example, the F/W Update process tries to stop all Entware services & user space processes to minimize the chances of the USB being "busy" at the time of the unmount, and still takes a few seconds.

If I move the logging up, the script freezes whereever the logging ends lol.

In this case right behind the continue prompt it freezes at "OK"

    printf "${GRNct}**IMPORTANT**:${NOct}\nThe firmware flash is about to start.\n"
    printf "Press Enter to stop now, or type ${GRNct}Y${NOct} to continue.\n"
    printf "Once started, the flashing process CANNOT be interrupted.\n"
    if ! _WaitForYESorNO_ "Continue"
    then _DoCleanUp_ 1 "$keepZIPfile" ; return 1 ; fi
	
	} 2>&1 | tee -a "$LOG_FILE"
    #END: Redirect both stdout and stderr to log file #

@ExtremeFiretop
Copy link
Owner Author

Yeah wherever I move the logging it freezes, I'm not sure whats going on, only seems to work behind the curl lol?

@Martinski4GitHub
Copy link
Collaborator

If I move the logging up, the script freezes whereever the logging ends lol.

In this case right behind the continue prompt it freezes at "OK"

    printf "${GRNct}**IMPORTANT**:${NOct}\nThe firmware flash is about to start.\n"
    printf "Press Enter to stop now, or type ${GRNct}Y${NOct} to continue.\n"
    printf "Once started, the flashing process CANNOT be interrupted.\n"
    if ! _WaitForYESorNO_ "Continue"
    then _DoCleanUp_ 1 "$keepZIPfile" ; return 1 ; fi
	
	} 2>&1 | tee -a "$LOG_FILE"
    #END: Redirect both stdout and stderr to log file #

That's strange. I'm about to have dinner in a few minutes, but I can try it later in the evening on my own router. ATM, I can't think of anything that would freeze the script at that point.

@ExtremeFiretop
Copy link
Owner Author

If I move the logging up, the script freezes whereever the logging ends lol.
In this case right behind the continue prompt it freezes at "OK"

    printf "${GRNct}**IMPORTANT**:${NOct}\nThe firmware flash is about to start.\n"
    printf "Press Enter to stop now, or type ${GRNct}Y${NOct} to continue.\n"
    printf "Once started, the flashing process CANNOT be interrupted.\n"
    if ! _WaitForYESorNO_ "Continue"
    then _DoCleanUp_ 1 "$keepZIPfile" ; return 1 ; fi
	
	} 2>&1 | tee -a "$LOG_FILE"
    #END: Redirect both stdout and stderr to log file #

That's strange. I'm about to have dinner in a few minutes, but I can try it later in the evening on my own router. ATM, I can't think of anything that would freeze the script at that point.

It's extremely strange I'm hoping you can recreate the issue. No matter where I move the end block for the logging, that's where the script freezes.

@ExtremeFiretop
Copy link
Owner Author

We might need to take a look at my logging solution again. I can't unmount and delete the mount point with it where it is, and if I move it anywhere else, it freezes the script Lol.

I'm Canadian so: Coincé entre le marteau et l'enclume

@Martinski4GitHub
Copy link
Collaborator

It's extremely strange I'm hoping you can recreate the issue. No matter where I move the end block for the logging, that's where the script freezes.

OK, I know what the problem was. As expected, everything inside the curly braces is executed in a child process so any results stored in variables while running the child process are not passed back to the parent process. As a consequence of that, variables used in the "curl" command (e.g. "$routerURLstr" "$firmware_file") to finally do the F/W Update don't have the expected values, and the curl command simply just hangs.

I already have a fixed for this and testing is going well. I'll submit a PR soon.

@Martinski4GitHub
Copy link
Collaborator

I'm Canadian so: Coincé entre le marteau et l'enclume

Ah yes, "caught between a rock and a hard place" or as my Grandma would say "entre la espada y la pared." My grandparents were Hispanic and I learned to speak their language when I was little since my Grandma would babysit me, my brother & sister, and later on when we were in school, our Grandpa would pick us up and both would talk to us in Spanish. Our Mother also taught us Spanish, and we took classes in high school & college so we are fairly fluent in conversation.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Dec 22, 2023

It's extremely strange I'm hoping you can recreate the issue. No matter where I move the end block for the logging, that's where the script freezes.

OK, I know what the problem was. As expected, everything inside the curly braces is executed in a child process so any results stored in variables while running the child process are not passed back to the parent process. As a consequence of that, variables used in the "curl" command (e.g. "$routerURLstr" "$firmware_file") to finally do the F/W Update don't have the expected values, and the curl command simply just hangs.

I already have a fixed for this and testing is going well. I'll submit a PR soon.

Just doesn't explain why in my examples I had commented out/deleted the entire curl commands and it was still freezing at the logging end block location.

@Martinski4GitHub
Copy link
Collaborator

It's extremely strange I'm hoping you can recreate the issue. No matter where I move the end block for the logging, that's where the script freezes.

OK, I know what the problem was. As expected, everything inside the curly braces is executed in a child process so any results stored in variables while running the child process are not passed back to the parent process. As a consequence of that, variables used in the "curl" command (e.g. "$routerURLstr" "$firmware_file") to finally do the F/W Update don't have the expected values, and the curl command simply just hangs.
I already have a fixed for this and testing is going well. I'll submit a PR soon.

Just doesn't explain why in my examples I had commented out/deleted the entire curl commands and it was still freezing at the logging end block location.

Did you comment out both curl commands at the same time?
During my testing & troubleshooting, I saw the "hangs" happen every time at the curl commands, and after checking the command arguments I found that variables were not defined because they were in the scope of the parent process (i.e. no longer in the scope of the child process).

@ExtremeFiretop
Copy link
Owner Author

It's extremely strange I'm hoping you can recreate the issue. No matter where I move the end block for the logging, that's where the script freezes.

OK, I know what the problem was. As expected, everything inside the curly braces is executed in a child process so any results stored in variables while running the child process are not passed back to the parent process. As a consequence of that, variables used in the "curl" command (e.g. "$routerURLstr" "$firmware_file") to finally do the F/W Update don't have the expected values, and the curl command simply just hangs.
I already have a fixed for this and testing is going well. I'll submit a PR soon.

Just doesn't explain why in my examples I had commented out/deleted the entire curl commands and it was still freezing at the logging end block location.

Did you comment out both curl commands at the same time? During my testing & troubleshooting, I saw the "hangs" happen every time at the curl commands, and after checking the command arguments I found that variables were not defined because they were in the scope of the parent process (i.e. no longer in the scope of the child process).

I think so? But it's hard to remember now lol.

Maybe I didn't and your right in that regard.

Either way I like the way the UI updates and scrolls without my old method of logging. It really is a choice of preference. If we really intend to only run this in the background the better logging is better, but if we intend to run this more in the user menu, then this minimal logging solution is better and looks better to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants