luci-app-attendedsysupgrade: use remote revision for version code

[dannil]

With 8860ca069c04e3dbd3f107f1526f2901f1085902 the check against the version_code parameter was implemented but was incorrectly set to the current revision instead of the latest available remote revision.

This resulted in not being able to upgrade due to the version_code parameter never matched the newer remote revision on the sysupgrade server.

To clarify the problem, r26637-05aec66d53 is the current running revision, and r26679-70088a7e4c is the latest remote revision (which we want to upgrade to) at time of writing this.

See https://github.com/openwrt/luci/pull/7163#issuecomment-2176678529

Without fix:

With fix:

• [X] This PR is not from my main or master branch :poop:, but a separate branch :white_check_mark:
• [X] Each commit has a valid :black_nib: Signed-off-by: <[email protected]> row (via git commit --signoff)
• [X] Each commit and PR title has a valid :memo: <package name>: title first line subject for packages
• [ ] Incremented :up: any PKG_VERSION in the Makefile
• [X] Tested on: (x86/64, SNAPSHOT r26637-05aec66d53, Firefox) :white_check_mark:
• [X] ( Preferred ) Mention: @ the original code author for feedback @aparcar
• [ ] ( Preferred ) Screenshot or mp4 of changes:
• [ ] ( Optional ) Closes: e.g. openwrt/luci#issue-number
• [ ] ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
• [X] Description: (describe the changes proposed in this PR)

[dannil]

~~We should if possible wait for @aparcar to verify this but seeing how firmware selector works as described in https://github.com/openwrt/luci/pull/7163#issuecomment-2176770355 I'm very sure this is how it's supposed to be implemented, with the latest remote revision being supplied as the version_code parameter.~~

Due to that the LuCI app supports advanced mode by both reinstalling the current revision and the latest available revision, the code path is a bit more complex than simply using the remote_revision variable. @systemcrash @aparcar feel free to revert 8860ca069c04e3dbd3f107f1526f2901f1085902 on master if needed; it'll reintroduce the non-deterministic behavior but I think it's better to allow users to at least be able to use the LuCI app, which isn't possible right now.

[aparcar]

Could you create a PR to revert this? And also is anyone interested in taking over maintainer ship of the app?

[dannil]

Could you create a PR to revert this?

https://github.com/openwrt/luci/pull/7168

[systemcrash]

So what's the consensus on this? It goes on with some modification?

[dannil]

So what's the consensus on this? It goes on with some modification?

That's the goal, the gist is that the LuCI app has the ability to flash the current running version again (it's part of the "Advanced options"), which breaks if you are running a SNAPSHOT and supply the version_code parameter to the API as you can't re-flash a SNAPSHOT since the image builders for that version are long gone by then. There needs to be some code to remove the version_code parameter only if not running on a stable branch (23.05, 22.03 etc.) if this case happens without breaking existing functionality.

[systemcrash]

Got it. OK, is that achievable?

Snapshot just needs to take current snapshot but there's no trivial way to do so?

[dannil]

Got it. OK, is that achievable?

Snapshot just needs to take current snapshot but there's no trivial way to do so?

It should be, I just haven't had time to look into into (work ya know :P). The one thing I can't wrap my head around right now is if the LuCI app actually supports to re-flash the existing SNAPSHOT version you have installed, which I think is no (due to the above reasons in the previous comment). However, it's still presented as a valid option at the moment to do this, which, if this is kept, will break when adding the version_code parameter as the image builders for SNAPSHOT:s doesn't accept "old" version codes.

So, my guess right now (I haven't had time to investigate it tho) is that the behavior that the LuCI app allowed you to re-flash the same installed SNAPSHOT has never been a valid configuration, and it would then instead just flash the newest available image from the image builders instead of the one you actually requested. Adding the version_code parameter exposed this existing, invalid configuration, which is why it wasn't as trivial to solve that I thought it would be, as the correct way to fix this then would be to disallow the re-flash of the already installed SNAPSHOT version by hiding it in the dropdown or any similar solution.

[systemcrash]

Is this changed any by your new PR #7203 ?

[dannil]

Is this changed any by your new PR #7203 ?

No, it's two completely different issues. In #7203, all the existing code already used the versioned endpoints except for /api/overview, so the /api/v1/build endpoint that accepts the version_code parameter wasn't touched at all in that PR.