-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat!: reject any OTA update/rollback request on ecu_info.yaml not properly loaded #465
feat!: reject any OTA update/rollback request on ecu_info.yaml not properly loaded #465
Conversation
…ed, reject any OTA update request on not started
# NOTE: always push OTAStatus change report | ||
if ( | ||
self.load_report(report) | ||
and self._status | ||
and ( | ||
isinstance(report.payload, OTAStatusChangeReport) | ||
or _now > _next_shm_push | ||
) | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, if the OTAStatusChangeReport reported too quickly, the newly coming report will be dropped, resulting in the OTA status not being updated. That is not proper, fixed in this PR.
# load and report booted OTA status | ||
_boot_ctrl_loaded_ota_status = self.boot_controller.get_booted_ota_status() | ||
if not otaclient_cfg.ECU_INFO_LOADED_SUCCESSFULLY: | ||
logger.error( | ||
"ecu_info.yaml file is not loaded properly, will reject any OTA request." | ||
) | ||
logger.error(f"set live_ota_status to {OTAStatus.FAILURE!r}") | ||
self._live_ota_status = OTAStatus.FAILURE | ||
status_report_queue.put_nowait( | ||
StatusReport( | ||
payload=OTAStatusChangeReport( | ||
new_ota_status=OTAStatus.FAILURE, | ||
failure_type=FailureType.UNRECOVERABLE, | ||
failure_reason=f"ecu_info.yaml file is broken or missing, please check {cfg.ECU_INFO_FPATH}. " | ||
"reject any OTA request.", | ||
), | ||
) | ||
) | ||
else: | ||
self._live_ota_status = _boot_ctrl_loaded_ota_status | ||
status_report_queue.put_nowait( | ||
StatusReport( | ||
payload=OTAStatusChangeReport( | ||
new_ota_status=_boot_ctrl_loaded_ota_status, | ||
), | ||
) | ||
) | ||
|
||
self.started = True | ||
logger.info("otaclient started") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If ecu_info.yaml is not loaded properly, we will just set the otaclient as NOT started, any requests coming to otaclient will be rejected when otaclient is not started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that when ecu_info.yaml is broken, only the live_ota_status(in the memory) will be set to FAILURE, the status
file will not be override, as ecu_info.yaml missing/broken is considered a soft failure.
Confirm the FMS's behavior is expected, now this PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but there is no test code. How about to pass invalid/valid ecu_info.yaml and verify the error code?
Let me take some time for implementing the test for it 👍 |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!!
Introduction
This PR implements the requirement of rejecting any OTA update/rollback request when ecu_info.yaml file is broken, and reports the failure_reason as due to ecu_info.yaml is broken.
If ecu_info.yaml is broken and cannot be parsed at startup, otaclient will set the live_ota_status to FAILURE, failure_type to UNRECOVERABLE, with failure_message:
ecu_info.yaml file is broken, please check /boot/ota/ecu_info.yaml. reject any OTA request.
And for every incoming OTA request, otaclient will reject with failure_type.UNRECOVERABLE for every ECUs listed in the request(as otaclient with default ecu_info.yaml doesn't have contact for the sub ECUs).
For multiple ECU environment, due to when falling down to the default ecu_info, we will only have one entry(which is the main ECU) in the available_ecu_ids list, so the situation will be the same as single ECU environment.
Other changes:
BREAKING CHANGE: now otaclient will set itself in FAILURE OTA status and reject any OTA requests when ecu_info.yaml is not properly loaded at startup.
The behavior on FMS console
Tests
https://tier4.atlassian.net/browse/RT4-9345