Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,13 @@ def _perform_flash_operation(
self.logger.error(f"Error running udhcpc: {e}")
raise FlashRetryableError(f"Error running udhcpc: {e}") from e

# make sure that the remote system has the right time without using NTP
# otherwise SSL certificate verification will fail
self.logger.info("Setting the remote DUT time to match the local system time")
current_timestamp = int(time.time())
console.sendline(f"date -s @{current_timestamp}")
console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)
Comment on lines +332 to +337
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling to make time synchronization retryable.

The time synchronization logic correctly addresses the TLS certificate verification issue in labs without NTP. However, unlike the udhcpc block above (lines 322-330), this code lacks error handling. If console.expect() times out or another exception occurs, it won't be wrapped in FlashRetryableError, preventing the retry logic from working as intended.

Apply this diff to add consistent error handling:

             except Exception as e:
                 self.logger.error(f"Error running udhcpc: {e}")
                 raise FlashRetryableError(f"Error running udhcpc: {e}") from e
 
-            # make sure that the remote system has the right time without using NTP
-            # otherwise SSL certificate verification will fail
-            self.logger.info("Setting the remote DUT time to match the local system time")
-            current_timestamp = int(time.time())
-            console.sendline(f"date -s @{current_timestamp}")
-            console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)
+            # make sure that the remote system has the right time without using NTP
+            # otherwise SSL certificate verification will fail
+            try:
+                self.logger.info("Setting the remote DUT time to match the local system time")
+                current_timestamp = int(time.time())
+                console.sendline(f"date -s @{current_timestamp}")
+                console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)
+            except pexpect.TIMEOUT as e:
+                self.logger.error(f"Timeout waiting for date command to complete: {e}")
+                raise FlashRetryableError("Timeout waiting for date command to complete") from e
+            except Exception as e:
+                self.logger.error(f"Error setting remote time: {e}")
+                raise FlashRetryableError(f"Error setting remote time: {e}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# make sure that the remote system has the right time without using NTP
# otherwise SSL certificate verification will fail
self.logger.info("Setting the remote DUT time to match the local system time")
current_timestamp = int(time.time())
console.sendline(f"date -s @{current_timestamp}")
console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)
# make sure that the remote system has the right time without using NTP
# otherwise SSL certificate verification will fail
try:
self.logger.info("Setting the remote DUT time to match the local system time")
current_timestamp = int(time.time())
console.sendline(f"date -s @{current_timestamp}")
console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)
except pexpect.TIMEOUT as e:
self.logger.error(f"Timeout waiting for date command to complete: {e}")
raise FlashRetryableError("Timeout waiting for date command to complete") from e
except Exception as e:
self.logger.error(f"Error setting remote time: {e}")
raise FlashRetryableError(f"Error setting remote time: {e}") from e
🤖 Prompt for AI Agents
In packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py
around lines 332 to 337, the time sync block lacks error handling so exceptions
from console.expect() won't be treated as retryable; wrap the date command and
console.expect call in a try/except, catch Exception, log the exception with
context via self.logger.warning or self.logger.exception, and raise
FlashRetryableError(...) (same pattern and message style used in the udhcpc
block above) so the caller can retry the operation.


stored_cacert = None
if should_download_to_httpd:
self._wait_for_storage_thread(storage_thread, error_queue)
Expand Down Expand Up @@ -383,12 +390,6 @@ def _setup_flasher_ssl(self, console, manifest, cacert_file: str | None) -> str
Raises:
RuntimeError: If there's an error reading the CA certificate file
"""
# make sure that the remote system has the right time without using NTP
# otherwise SSL certificate verification will fail
self.logger.info("Setting the remote DUT time to match the local system time")
current_timestamp = int(time.time())
console.sendline(f"date -s @{current_timestamp}")
console.expect(manifest.spec.login.prompt, timeout=EXPECT_TIMEOUT_DEFAULT)

if cacert_file:
cacert = b""
Expand Down
Loading