Add remote file handling #30

b-rowan · 2024-07-25T00:18:14Z

Adds remote firmware handling for files. Based on #29. Closes #28. Closes #29.

Adding independently so they can be merged independently if there are concerns on this PR.

Main concern here is that I don't have a better way to get the hash of the upgrade file (required for swupdate artifact) without just downloading the file when the link is added.

…le name.

easybe

IMHO downloading the artifact when creating a new DB entry to get the hashes is a reasonable approach.

I’ve only skimmed over the changes as reviewing on the phone is not that fun.

.pre-commit-config.yaml

easybe · 2024-07-25T08:07:44Z

goosebit/models.py

+    hw_revision = fields.CharField(max_length=255)
+
+
+class FirmwareUpdate(Model):


I would just call it Firmware as it does not have to be an update, it could technically also be a downgrade (if we would start to allow that) or the initial image.

The rollout model needs to be adjusted as well - instead of hw_model, hw_revision, fw_file it only should reference a firmware.

This then also means work in Manager.get_rollout to query the correct firmware.

The rollout model needs to be adjusted as well - instead of hw_model, hw_revision, fw_file it only should reference a firmware.

This then also means work in Manager.get_rollout to query the correct firmware.

This can get really tricky, if I link Firmware directly then all the stuff that wants to check for rollout that uses the device as a backup need to return Firmware, which means fw_file on Device needs to be replaced with Firmware, but then that gets rid of latest and pinned, which can make things pretty complicated. Let me know what you guys think here.

Yes, it triggers a few follow-up changes. I have a branch where I introduced update_mode on Device (enum with latest, rollout, assigned, and pinned), so that fw_file could be changed to firmware on Device.

Will re-implement those changes on top of this branch to see if it makes sense.

easybe · 2024-07-25T08:14:10Z

Oh, and please note this comment: c553176#r144602289

tsagadar · 2024-07-25T11:56:01Z

I think we should slow down somewhat: as there are no automatic test in the code base, every PR breaks some functionality. Manual testing (what I was doing so far) takes forever. So we better ensure to have tests for the existing functionality before adding / refactoring code.

I created automated API integration tests (focus on the DDI) yesterday - and currently adjust the test code to match this PR. Ideally I could finish these tests (few of them will fail as the code is broken), integrate them into this PR and ensure that everything passes before merging.

tsagadar

Few things I saw while writing tests. Some if them cause my current tests to fail.

tsagadar · 2024-07-25T12:04:32Z

goosebit/models.py

+    hw_revision = fields.CharField(max_length=255)
+
+
+class FirmwareUpdate(Model):


The rollout model needs to be adjusted as well - instead of hw_model, hw_revision, fw_file it only should reference a firmware.

This then also means work in Manager.get_rollout to query the correct firmware.

goosebit/models.py

goosebit/updater/manager.py

.pre-commit-config.yaml

goosebit/api/download.py

tsagadar · 2024-07-25T12:21:21Z

goosebit/db.py

@@ -22,6 +23,10 @@ async def init():
    await command.migrate()
    await command.upgrade(run_in_transaction=True)
    await Tortoise.generate_schemas(safe=True)
+    for update in await FirmwareUpdate.all():


Instead of all() better to filter on db using filter().

Not sure what to filter by here, I suppose maybe uri starts with file?

Thought about using await Firmware.filter(url__startswith='file').all(). Debatable if a good idea, as logic of local is the implemented in two different places. On the upside: with a lot of remote firmware it will be very fast.

goosebit/models.py

tsagadar

I finished a first set of integration test - they can be found at https://github.com/husqvarnagroup/goosebit/tree/mm/add_simple_ddi_integration_tests

Besides the commit adding the tests there is an additional one that makes the tests pass. Most of the things are also annotated in this PR.

Note: poetry run pytest does not pass all the tests. Single execution of tests work (I do it using PyCharm). Will look into this soon (but not today)

tsagadar · 2024-07-25T14:03:21Z

goosebit/updates/swdesc.py

+        swdesc_attrs["version"] = semver.Version.parse(swdesc["software"]["version"])
+        swdesc_attrs["size"] = file.stat().st_size
+        swdesc_attrs["hash"] = sha1_hash_file(file)
+        swdesc_attrs["compatibility"] = [


This parsing does not work for our images. Our swdesc looks (shortened) like the following:

{ "software": { "version": "8.8.1-12-g302f635+189128", "description": "Linux System Firmware (hawkbit) for the GARDENA smart Gateway gardena-sg-mt7688", "smart-gateway-mt7688": { "hardware-compatibility": [ "0.5", "1.0", "1.1.0", "1.2.0", "1.2.1" ], "stable": { "bootslot0": { "images": [ { "filename": "gardena-image-hawkbit-gardena-sg-mt7688.squashfs-xz", "volume": "rootfs1", "sha256": "a7fd6b6b3416efc3a58993280f7335f84826af6a73164624e9cb6b5201499c18", "hook": "check_version_and_leds", "installed-directly": true } ], "bootenv": [ ] }, "bootslot1": { "images": [ { "filename": "gardena-image-hawkbit-gardena-sg-mt7688.squashfs-xz", "volume": "rootfs0", "sha256": "a7fd6b6b3416efc3a58993280f7335f84826af6a73164624e9cb6b5201499c18", "hook": "check_version_and_leds", "installed-directly": true } ], "bootenv": [ ] } } } } }

Is there a standard on how this is supposed to be set? We use loadsync.v1 here, but I'm not sure if there is any standard here...

This is not my area of expertise. Maybe @easybe can provide information once he is back.

goosebit/models.py

goosebit/updater/manager.py

b-rowan · 2024-07-25T21:16:15Z

I'll try to take a look at this later tonight, at a conference this week so I don't have a ton of time, but I'll fix what I can ASAP.

I think we should slow down somewhat: as there are no automatic test in the code base, every PR breaks some functionality.

Agreed, but I really want to get the important core functionality we want figured out ASAP. I'm not a big tests guy personally, I think it's definitely a good idea in this case, I just struggle with the differences between trying to finish important functionality and stabilizing the code/API. There seem to be a lot of cases where the dev API needs to change for important things to happen, so I wasn't sure if we wanted to wait to write tests until after that.

…`Hardware`

b-rowan · 2024-07-26T03:45:13Z

I finished a first set of integration test - they can be found at https://github.com/husqvarnagroup/goosebit/tree/mm/add_simple_ddi_integration_tests

Besides the commit adding the tests there is an additional one that makes the tests pass. Most of the things are also annotated in this PR.

Note: poetry run pytest does not pass all the tests. Single execution of tests work (I do it using PyCharm). Will look into this soon (but not today)

Implemented these, and fixed your issue with pytest not working properly. Just need to add __init__.py files to all directories leading to the tests to make them identify as modules.

b-rowan · 2024-07-26T03:54:53Z

Mostly everything fixed for now, just a couple things I want to clarify on before I change anything else, check comments on unresolved when you get a chance @tsagadar @easybe. Appreciate the pointers, this was a pretty large PR, but I think a pretty important one to deal with early on.

tsagadar · 2024-07-26T05:06:12Z

I think this PR is ready to be merged - great stuff. Follow-up work can be done in a new branch / PR. Will look replacing fw_file with firmware.

UpstreamData and others added 7 commits July 23, 2024 21:47

Add swdesc parser, and basic firmware framework for the database.

3525e10

Handle firmware via DB, and remove tests for handling firmware via fi…

b585270

…le name.

Improve firmware handling for devices.

37066fd

Improve firmware UI.

9c1f39e

Merge branch 'master' into dev_swdesc_parse

c553176

Remove some issues from testing.

742b90f

Add remote file uploads and parsing.

df00f1d

b-rowan mentioned this pull request Jul 25, 2024

Alternate update location support #19

Closed

b-rowan requested a review from easybe July 25, 2024 00:24

easybe reviewed Jul 25, 2024

View reviewed changes

tsagadar requested changes Jul 25, 2024

View reviewed changes

tsagadar reviewed Jul 25, 2024

View reviewed changes

UpstreamData and others added 9 commits July 25, 2024 17:27

Fix typo.

92e5b07

Move generate_chunk out of FirmwareUpdate, and fix version pinning.

dc19e82

Rename get_update_file to get_firmware

7845d55

Change update mode to an enum and re-add firmware None case.

27f1ff0

Use firmware version when checking if device is up to date.

a5d0c5d

Only handle downloading files by ID.

526d6e6

Rename FirmwareUpdate to Firmware and FirmwareCompatibility to …

ac64162

…`Hardware`

Introduce API integration tests

3f48d36

Fix tests, and make them run properly.

d39d7ad

Make latest take device parameter.

f7a24f8

tsagadar approved these changes Jul 26, 2024

View reviewed changes

b-rowan merged commit fa5e0bd into master Jul 26, 2024

b-rowan deleted the dev_remote_files branch August 1, 2024 14:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add remote file handling #30

Add remote file handling #30

b-rowan commented Jul 25, 2024 •

edited

Loading

easybe left a comment

easybe Jul 25, 2024

tsagadar Jul 25, 2024

b-rowan Jul 26, 2024

tsagadar Jul 26, 2024

easybe commented Jul 25, 2024

tsagadar commented Jul 25, 2024

tsagadar left a comment

tsagadar Jul 25, 2024

tsagadar Jul 25, 2024

b-rowan Jul 25, 2024

tsagadar Jul 26, 2024

tsagadar left a comment

tsagadar Jul 25, 2024

b-rowan Jul 25, 2024

tsagadar Jul 26, 2024

b-rowan commented Jul 25, 2024

b-rowan commented Jul 26, 2024

b-rowan commented Jul 26, 2024

tsagadar commented Jul 26, 2024

		hw_revision = fields.CharField(max_length=255)


		class FirmwareUpdate(Model):

Add remote file handling #30

Add remote file handling #30

Conversation

b-rowan commented Jul 25, 2024 • edited Loading

easybe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

easybe commented Jul 25, 2024

tsagadar commented Jul 25, 2024

tsagadar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsagadar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-rowan commented Jul 25, 2024

b-rowan commented Jul 26, 2024

b-rowan commented Jul 26, 2024

tsagadar commented Jul 26, 2024

b-rowan commented Jul 25, 2024 •

edited

Loading