This repository was archived by the owner on Jan 23, 2026. It is now read-only.
flasher-driver: add support for using fls as flasher#735
Merged
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughAdds a selectable flash method (default "fls") with optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Driver as FlashDriver
participant Target as TargetHost
participant FLS as FLS Installer/Runner
participant Monitor as ProgressMonitor
User->>Driver: flash(path, method="fls", fls_version="X")
activate Driver
alt method == "fls"
Driver->>Driver: _perform_flash_operation(method="fls", fls_version="X")
Driver->>FLS: (optional) download & install fls version X
FLS-->>Driver: fls installed / available
Driver->>Driver: _cmdline_tls_args()
Driver->>Target: run `fls from-url [image_url]` (with TLS/headers)
activate Target
loop stream progress
Target->>Monitor: stdout lines
Monitor->>User: print progress updates
Monitor->>Monitor: detect completion marker / EOF
end
Target-->>Driver: process exit
deactivate Target
else method == "shell"
Driver->>Driver: _flash_with_progress() (existing shell-based flow)
end
Driver-->>User: flash result
deactivate Driver
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py(12 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-14T16:05:14.577Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 339
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:349-349
Timestamp: 2025-03-14T16:05:14.577Z
Learning: The `_upload_artifact` method in `BaseFlasherClient` class should call `write_from_path` with the calculated filename parameter: `storage.write_from_path(filename, path, operator=operator)` instead of `storage.write_from_path(path, operator=operator)`.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (2)
packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/client.py (1)
pexpect(29-37)examples/soc-pytest/jumpstarter_example_soc_pytest/test_on_rpi4.py (1)
console(20-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: build
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: e2e
🔇 Additional comments (6)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (6)
15-15: LGTM - pexpect import is necessary.The
pexpectimport is required for the new error handling (pexpect.TIMEOUTat line 325) and FLS progress monitoring (pexpect.TIMEOUTandpexpect.EOFat lines 503, 525).
322-330: Good enhancement - explicit retryable error handling for network startup.The enhanced error handling for
udhcpcproperly wraps timeout and other exceptions as retryable errors, which aligns well with the retry logic at the higher level.
410-425: Good refactor - generalized TLS argument builder.Renaming
_curl_tls_argsto_cmdline_tls_argsbetter reflects its use for both curl-based and FLS-based flashing methods. The implementation remains correct.
487-487: Verify FLS command-line argument compatibility.The code assumes FLS accepts the same TLS and header argument formats as curl (e.g.,
-k,--cacert,-H). Please confirm that FLS supports these exact argument formats, or adjust the argument construction accordingly.Additionally, document what the
-i 1.0and-nflags mean for FLS, as they're not immediately clear.
497-532: Verify FLS outputs expected completion markers.The progress monitor reuses
_check_completion_markers(line 517), which looks for"FLASH_COMPLETE"and"FLASH_FAILED"strings. Please verify that the FLS tool actually outputs these markers. If FLS uses different completion indicators, the monitoring logic may not detect completion correctly or may wait indefinitely.If FLS uses different markers, consider:
- Adding FLS-specific marker detection, or
- Documenting the expected FLS output format, or
- Making FLS output these markers for compatibility.
1131-1142: CLI options properly defined with validation.The CLI options for
--methodand--fls-versionare well-defined with appropriate types and choices. Theclick.Choiceconstraint ensures only valid methods are accepted via CLI.Note: The default value inconsistency with the method signature (line 101) was flagged separately.
mangelajo
commented
Nov 5, 2025
mangelajo
force-pushed
the
fls-support
branch
2 times, most recently
from
1fa098b to
827e367
Compare
Member
Author
|
this is how it looks with this patch: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (1)
412-427: Fix insecure TLS flag: FLS expects--insecure-tls, not-k.FLS supports
--insecure-tlsfor cert verification (equivalent to curl -k) and--cacertfor CA bundles. The code at line 420 generates-kbut should generate--insecure-tlsto be compatible with FLS. The--cacertflag is correct.
♻️ Duplicate comments (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (1)
478-491: Handle missing FLS when no version specified.When
fls_version == ""(the API default), the code skips downloading FLS and directly executes theflscommand at line 491. If FLS isn't pre-installed in the flasher image, this will fail with an unclear error.Consider checking if FLS exists before attempting to use it:
if fls_version != "": self.logger.info(f"Downloading FLS version {fls_version} from GitHub releases") # Download fls binary to the target device (until it is available on the target device) fls_url = ( f"https://github.com/jumpstarter-dev/fls/releases/download/{fls_version}/" f"fls-aarch64-linux" ) console.sendline(f"curl -L {fls_url} -o /sbin/fls") console.expect(prompt, timeout=EXPECT_TIMEOUT_DEFAULT) console.sendline("chmod +x /sbin/fls") console.expect(prompt, timeout=EXPECT_TIMEOUT_DEFAULT) + else: + # Verify FLS is available when no version specified + console.sendline("which fls") + console.expect(prompt, timeout=EXPECT_TIMEOUT_DEFAULT) + console.sendline("echo $?") + console.expect(prompt, timeout=EXPECT_TIMEOUT_DEFAULT) + exit_code = int(console.before.decode(errors="ignore").strip().splitlines()[-1]) + if exit_code != 0: + raise FlashNonRetryableError( + "FLS not found. Either specify fls_version to download, or ensure FLS is pre-installed in the flasher image." + ) # Flash the imageBased on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py(12 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:100-101
Timestamp: 2025-11-05T13:31:39.263Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the `flash()` method and its CLI command intentionally have different defaults for `fls_version`: the method defaults to `""` (empty string) while the CLI defaults to `"0.1.5"`. This is by design to provide CLI users with a convenient default while keeping the programmatic API ready for when FLS is included in the flasher images.
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:476-486
Timestamp: 2025-11-05T13:33:24.716Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the FLS flasher download in _flash_with_fls() hardcodes "aarch64-linux" architecture because the flasher always targets ARM devices currently.
📚 Learning: 2025-11-05T13:31:39.263Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:100-101
Timestamp: 2025-11-05T13:31:39.263Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the `flash()` method and its CLI command intentionally have different defaults for `fls_version`: the method defaults to `""` (empty string) while the CLI defaults to `"0.1.5"`. This is by design to provide CLI users with a convenient default while keeping the programmatic API ready for when FLS is included in the flasher images.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-11-05T13:33:24.716Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:476-486
Timestamp: 2025-11-05T13:33:24.716Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the FLS flasher download in _flash_with_fls() hardcodes "aarch64-linux" architecture because the flasher always targets ARM devices currently.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-09-24T16:13:14.244Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 644
File: packages/jumpstarter-driver-flashers/oci_bundles/aarch64-itb/build_fits.sh:16-18
Timestamp: 2025-09-24T16:13:14.244Z
Learning: The build script packages/jumpstarter-driver-flashers/oci_bundles/aarch64-itb/build_fits.sh is designed to run specifically on ARM containers to pick up the Fedora ARM kernel, so hardcoded aarch64 package references are intentional and correct.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-03-14T16:05:14.577Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 339
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:349-349
Timestamp: 2025-03-14T16:05:14.577Z
Learning: The `_upload_artifact` method in `BaseFlasherClient` class should call `write_from_path` with the calculated filename parameter: `storage.write_from_path(filename, path, operator=operator)` instead of `storage.write_from_path(path, operator=operator)`.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (2)
packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/client.py (1)
pexpect(29-37)examples/soc-pytest/jumpstarter_example_soc_pytest/test_on_rpi4.py (1)
console(20-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
🔇 Additional comments (8)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (8)
100-101: LGTM!The different defaults for
fls_versionbetween the method (empty string) and CLI (line 1144: "0.1.5") are intentional per the design—providing CLI convenience while keeping the API ready for when FLS is baked into images.Based on learnings.
322-330: LGTM! Good error handling improvement.Wrapping udhcpc failures in
FlashRetryableErrorenables the retry mechanism to handle network setup issues gracefully. The specific handling ofpexpect.TIMEOUTand the fallbackExceptioncatch are both appropriate.
341-365: LGTM! Method validation properly implemented.The branching logic correctly validates the method parameter, raising
ArgumentErrorfor invalid values. This addresses the concern about programmatic calls bypassing CLI validation.
537-584: LGTM! Refactoring to use renamed TLS helper.The update correctly uses the renamed
_cmdline_tls_argsmethod. The refactoring maintains consistency across both flash methods.
1135-1146: LGTM! CLI options properly configured.The
--methodand--fls-versionoptions are correctly defined with appropriate defaults and validation. The TODO comment at line 1144 clearly documents the plan to change the default once FLS is baked into images.Based on learnings.
1148-1163: LGTM! CLI function signature updated.The function signature correctly accepts the new
methodandfls_versionparameters from the CLI options.
1174-1186: LGTM! Parameters properly forwarded.The new
methodandfls_versionparameters are correctly passed through from the CLI to theflash()method, completing the integration.
501-536: No changes needed—FLS completion detection is properly implemented.The code correctly handles FLS completion via two mechanisms:
Primary (prompt-based): Lines 524-526 detect completion when
console.match_index == 0, meaning the prompt matched. Since FLS is a standard CLI tool, it returns to the prompt upon completion, triggering this detection path.Fallback (marker-based): Line 521 checks for shell-specific markers ("FLASH_COMPLETE"/"FLASH_FAILED"), which are generated by the shell-based method (line 574:
echo "F""LASH_COMPLETE") but not emitted by FLS. This check is harmless and won't interfere.The timeout loop (line 507) ensures output is continuously accumulated with 1-second intervals until the prompt is matched or an exception occurs. The
_flash_with_flsand_flash_with_progressmethods appropriately use different completion mechanisms suited to each approach.
mangelajo
force-pushed
the
fls-support
branch
from
827e367 to
d43fa8b
Compare
bennyz
reviewed
Nov 5, 2025
for now we will download it by default from github in runtime, once we are happy with a version, we can bake it into the flasher image.
mangelajo
force-pushed
the
fls-support
branch
from
d43fa8b to
4c0c286
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (1)
478-496: Consider adding a check for pre-installed FLS when version is empty.When
fls_version == ""(the programmatic default), the code assumes FLS is pre-installed at/sbin/fls. If it's not present, the command at line 498 will fail with a less clear error message.Consider adding a pre-check when
fls_version == "":if fls_version == "": # Verify FLS is installed console.sendline("command -v fls") console.expect(prompt, timeout=EXPECT_TIMEOUT_DEFAULT) fls_check = console.before.decode(errors="ignore").strip() if "/fls" not in fls_check: raise FlashNonRetryableError( "FLS binary not found. Either include FLS in the flasher image or specify --fls-version." )This would provide clearer feedback when FLS is missing rather than failing at command execution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py(12 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:100-101
Timestamp: 2025-11-05T13:31:39.263Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the `flash()` method and its CLI command intentionally have different defaults for `fls_version`: the method defaults to `""` (empty string) while the CLI defaults to `"0.1.5"`. This is by design to provide CLI users with a convenient default while keeping the programmatic API ready for when FLS is included in the flasher images.
📚 Learning: 2025-11-05T13:31:39.263Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:100-101
Timestamp: 2025-11-05T13:31:39.263Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the `flash()` method and its CLI command intentionally have different defaults for `fls_version`: the method defaults to `""` (empty string) while the CLI defaults to `"0.1.5"`. This is by design to provide CLI users with a convenient default while keeping the programmatic API ready for when FLS is included in the flasher images.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-11-05T13:33:24.716Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:476-486
Timestamp: 2025-11-05T13:33:24.716Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, the FLS flasher download in _flash_with_fls() hardcodes "aarch64-linux" architecture because the flasher always targets ARM devices currently.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-11-05T13:45:58.087Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 735
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:15-15
Timestamp: 2025-11-05T13:45:58.087Z
Learning: In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py, pexpect is intentionally used as a transitive dependency through the jumpstarter-driver-pyserial package. The flashers package does not declare pexpect as a direct dependency because the pyserial driver package is intended to control the pexpect version.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-09-24T16:13:14.244Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 644
File: packages/jumpstarter-driver-flashers/oci_bundles/aarch64-itb/build_fits.sh:16-18
Timestamp: 2025-09-24T16:13:14.244Z
Learning: The build script packages/jumpstarter-driver-flashers/oci_bundles/aarch64-itb/build_fits.sh is designed to run specifically on ARM containers to pick up the Fedora ARM kernel, so hardcoded aarch64 package references are intentional and correct.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-01-29T11:52:43.554Z
Learnt from: bennyz
Repo: jumpstarter-dev/jumpstarter PR: 241
File: packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py:52-60
Timestamp: 2025-01-29T11:52:43.554Z
Learning: The TFTP driver (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py) handles all low-level concerns like path validation, error handling, and checksum computation. The client (packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/client.py) should remain simple as it delegates these responsibilities to the driver.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
📚 Learning: 2025-03-14T16:05:14.577Z
Learnt from: mangelajo
Repo: jumpstarter-dev/jumpstarter PR: 339
File: packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py:349-349
Timestamp: 2025-03-14T16:05:14.577Z
Learning: The `_upload_artifact` method in `BaseFlasherClient` class should call `write_from_path` with the calculated filename parameter: `storage.write_from_path(filename, path, operator=operator)` instead of `storage.write_from_path(path, operator=operator)`.
Applied to files:
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
🧬 Code graph analysis (1)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (2)
packages/jumpstarter-driver-pyserial/jumpstarter_driver_pyserial/client.py (1)
pexpect(29-37)packages/jumpstarter/jumpstarter/common/exceptions.py (1)
ArgumentError(54-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: e2e
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
🔇 Additional comments (2)
packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py (2)
322-330: LGTM! Enhanced error handling for network setup.Good defensive improvement wrapping the udhcpc call with specific timeout and general exception handling, properly classified as retryable errors.
508-550: LGTM! Well-structured progress monitoring with comprehensive error detection.The incremental output tracking, panic detection, and exit code verification provide robust monitoring of the FLS operation. The logic correctly handles both normal completion (prompt match) and timeout cases for continuous reading.
bennyz
approved these changes
Nov 5, 2025
|
Successfully created backport PR for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
for now we will download it by default from github in runtime, once we are happy with a version, we can bake it into the flasher image.
Summary by CodeRabbit
New Features
Bug Fixes