Skip to content
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

[NEXMANAGE-515][NEXMANAGE-598] Support TiberOS #543

Merged
merged 32 commits into from
Oct 2, 2024

Conversation

yengliong93
Copy link
Contributor

@yengliong93 yengliong93 commented Aug 28, 2024

The PR review is to check for sustainability and correctness. Sustainability is actually more business critical as correctness is largely tested into the code over time. Its useful to keep in mind that SW often outlives the HW it was written for and engineers move from job to job so it is critical that code developed for Intel be supportable across many years. It is up to the submitter and reviewer to look at the code from a perspective of what if we have to debug this 3 years from now after the author is no longer available and defect databases have been lost. Yes, that happens all the time when we are working with time scales of more than 2 years. When reviewing your code it is important to look at it from this perspective.

Author Mandatory (to be filled by PR Author/Submitter)

  • Developer who submits the Pull Request for merge is required to mark the checklist below as applicable for the PR changes submitted.
  • Those checklist items which are not marked are considered as not applicable for the PR change.
  • Items marked with an asterisk suffix are mandatory items to check and if not marked will be treated as non-compliant pull requests by the developers for Inner Source Development Model (ISDM) compliance

PULL DESCRIPTION

This PR updates the INBM and INBC to support TiberOS SOTA.

  • In INBC, the signature and password arguments have been enabled to pass the image signature (SHA256) and password (ORAS JWT token) to INBM. The JWT token will be used to download the new image from the release server using ORAS, while the signature is used to verify the image integrity.
  • In INBM:
    • Added TiberOS as the new OS to be detected.

    • Updated dispatcher to record the tiberos-version in dispatcher_state file. This is used to compare the version of the image after reboot.

    • Added ORAS util for using the ORAS tool to download the image.

    • In TiberOS, INBM uses UpdateTool (UT) to perform the image A/B based update.

      1. PUA issues INBC SOTA download-only. INBM downloads the artifacts from the release server and performs the checking. After that, it calls UT to write the image into partition B.
      2. After the step above, PUA issues INBC SOTA no-download to apply the image changes and reboot the system.
    • The granular log method has been enhanced to record the details of the SOTA in TiberOS.

      • It records the detail status of the SOTA.
      • It records the version of the image if SOTA is successful.
      • It records the failure reason if SOTA fails.

Some opens:

  • Does INBM have to run UT commit command after system reboot?
  • Does INBM have to run UT rollback command if SOTA fails after reboot?
  • Does INBC needs to enable more arguments in order to pass the image information, such as os_image_sha, os_image_url, os_image_id, profile_name and profile_version?

REFERENCES

Reference URL for issue tracking (JIRA/HSD/Github): <URL to be filled>

  • Added label to the Pull Request following the template: ISDM_<Complexity>*
    Note-1: Depending on complexity of code changes, use the suitable word for complexity: Low/Medium/High
    Example: PR for Slim boot loader project with medium complexity can have the label as: ISDM_Medium
  • Added label to the Pull Request for easier discoverability and search
  • RTC or HSD number will be included in final merge. HSD must always be included if available.
  • Changelogs are updated (or N/A if not customer visible)
  • inbm/log_changes.txt are updated for potentially Validation-breaking log changes (or N/A if none)

CODE MAINTAINABILITY

  • Commit Message meets guidelines as indicated in the URL*
  • Every commit is a single defect fix and does not mix feature addition or changes*
  • Added required new tests relevant to the changes
    • PR contains URL links to functional tests executed with the new tests
  • Updated Documentation as relevant to the changes
  • Updated Build steps/commands changes as relevant
  • PR change contains code related to security
  • PR introduces changes that breaks compatibility with other modules (If YES, please provide description)
  • Specific instructions or information for code reviewers (If any):
  • Run 'go fmt' or format-python.sh as applicable.
  • New/modified methods and functions should have type annotations on signatures as applicable
  • New/modified methods must have appropriate doc strings (language dependent)

APPLICATION SPECIFIC

  • Does PR change default config files under /etc? If so, will application still work after an upgrade that leaves /etc alone, like a Mender upgrade?
  • Is cloud UI changed? If so, are cloud definition files updated?

Maintainer Mandatory (to be filled by PR Reviewer/Approving Maintainer)

  • Maintainer who approves the Pull Request for merge is required to mark the checklist below as appropriate for the PR change reviewed as key proof of attestation indicating reasons for merge.
  • Those checklist items which are not marked are considered as not applicable for the PR change.
  • Items marked with an asterisk suffix are mandatory items to check and if not marked will be treated as non-compliant pull requests by the maintainers for ISDM compliance.

QUALITY CHECKS

  • Architectural and Design Fit
  • Quality of code (At least one should be checked as applicable)*
    • Commit Message meets guidelines
    • PR changes adhere to industry practices and standards
    • Error and exception code paths implemented correctly
    • Code reviewed for domain or language specific anti-patterns
    • Code is adequately commented
    • Code copyright is correct
    • Confusing logic is explained in comments
    • Commit comment can be used to design a new test case for the changes
  • Test coverage shows adequate coverage with required CI functional tests pass on all supported platforms*
  • Static code scan report shows zero critical issues*
  • Integration tests are passing

CODE REVIEW IMPACT

  • Summary of Defects Detected in Code Review: <%P1xx,P2xx,P3xx,P4xx%>
    Note P1/P2/P3/P4 denotes severity of defects found (Showstopper/High/Medium/Low) and xx denotes number of defects found

SECURITY CHECKS

Please check if your PR fulfills the following requirements:

  • Follow best practices when handling primitive data types
  • Configure minimal permissions when opening pipes and ports
  • Check contents within input structures are valid before use
  • All forms of input validated
  • Avoid inter-process race conditions
  • Error and exception handling implemented
  • Defend against Canonical Representation Issues - Any paths utilized?
  • Follow 'secure by default' - Any configuration values added
  • Fail safe - Any failure scenarios?
  • Clean up temporary files - Any temporary files being used?

Code must act as a teacher for future developers

This PR updates the INBM to support TiberOS SOTA.

Signed-off-by: yengliong <[email protected]>
@yengliong93 yengliong93 changed the title WIP: [NEXMANAGE-515] Support TiberOS [NEXMANAGE-515][NEXMANAGE-598] Support TiberOS Sep 18, 2024
Copy link
Contributor

@gblewis1 gblewis1 left a comment

Choose a reason for hiding this comment

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

Don't merge yet -- let's hold in a branch as optimization effort for Turtle Creek is pending. We want to make sure we are including INBM in Tiber OS before this is merged.

@yengliong93
Copy link
Contributor Author

Added the fix to address issue in NEXMANAGE-734.

@yengliong93
Copy link
Contributor Author

I want to run the integration test but the docker pull rate limitation encountered.

@yengliong93
Copy link
Contributor Author

Integration test passes.

@gblewis1 gblewis1 merged commit 9a85dd5 into develop Oct 2, 2024
15 checks passed
@gblewis1 gblewis1 deleted the inbm-support-tiberos branch October 2, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants