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

ug: Add documentation for aktualizr-lite command line interface #734

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

detsch
Copy link
Member

@detsch detsch commented Aug 1, 2024

CLI documentation update for v95.


PR Template and Checklist

Please complete as much as possible to speed up the reviewing process.

Readiness and adding reviewers as appropriate is required.

All PRs should be reviewed by a technical writer/documentation team and a peer.
If effecting customers—which is a majority of content changes—a member of Customer Success must also review.

Readiness

  • Merge (pending reviews)
  • Merge after date or event
  • Draft

Overview

Why merge this PR? What does it solve?

Checklist

  • Run spelling and grammar check, preferably with linter.
  • Avoid changing any header associated with a link/reference.
  • Step through instructions (or ask someone to do so).
  • Review for wordiness
  • Match tone and style of page/section.
  • Run make linkcheck.
  • View HTML in a browser to check rendering.
  • Use semantic newlines.
  • follow best practices for commits.
    • Descriptive title written in the imperative.
    • Include brief overview of QA steps taken.
    • Mention any related issues numbers.
    • End message with sign off/DCO line (-s, --signoff).
    • Sign commit with your gpg key (-S, --gpg-sign).
    • Squash commits if needed.
  • Request PR review by a technical writer and at least one peer.

Comments

Any thing else that a maintainer/reviewer should know.
This could include potential issues, rational for approach, etc.

@doanac
Copy link
Member

doanac commented Aug 1, 2024

.. highlight:: bash
:linenothreshold: 1

Sample bash script illustrating usage of CLI operations and return codes handling::
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this script to the sotactl repo and reference it from this doc.

Copy link
Member Author

@detsch detsch Aug 8, 2024

Choose a reason for hiding this comment

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

I was thinking about having it on a repo an also keep it in the docs, I believe it is simple enough to contribute to the docs context.

- Unable to fetch TUF metadata
- *14*: Failure
- TUF metadata not found in the provided path (offline mode only)
- *15*: Failure
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to ask someone from CS team to check if the CLI commands and return codes as explained here works for them.

source/user-guide/custom-sota-client.rst Outdated Show resolved Hide resolved
source/user-guide/custom-sota-client.rst Outdated Show resolved Hide resolved
source/user-guide/custom-sota-client.rst Outdated Show resolved Hide resolved
@vanmaegima
Copy link
Member

I'm missing a note about stopping aklite daemon before running CLI.
We could cover:

  • manual updates (stop aklite and stop fioconfig)
  • skip daemon start from lmp-device-register
  • skip daemon from the build (we have an example for custom-sota, what would be the alternative for CLI?)

@vanmaegima
Copy link
Member

I haven't reviewed return codes and sample script yet.

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 5c71a19 to 4564d81 Compare August 8, 2024 20:23
@doanac
Copy link
Member

doanac commented Aug 8, 2024

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 4564d81 to 5e568db Compare September 4, 2024 00:41
@doanac
Copy link
Member

doanac commented Sep 4, 2024

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 5e568db to 1548918 Compare September 4, 2024 19:40
@doanac
Copy link
Member

doanac commented Sep 4, 2024

Fetch TUF Metadata and List Updates
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The ``check`` command will refresh the Targets metadata from the OTA server, and present you with a list of available Targets::
Copy link
Contributor

Choose a reason for hiding this comment

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

@kprosise Do you think this "refresh the Targets metadata from the OTA server" means what the command actually does?

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 1548918 to 9eebdc6 Compare September 6, 2024 18:03
@doanac
Copy link
Member

doanac commented Sep 6, 2024

@detsch
Copy link
Member Author

detsch commented Sep 6, 2024

I'm missing a note about stopping aklite daemon before running CLI. We could cover:

  • manual updates (stop aklite and stop fioconfig)

Added note.

  • skip daemon start from lmp-device-register

Added note.

  • skip daemon from the build (we have an example for custom-sota, what would be the alternative for CLI?)

We do need the daemon binary, so the build itself is not changed someone is going to use CLI. Perhaps adding something about not enabling the aklite service by default in the build? Not sure hot that can be done, will have to check it.

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 9eebdc6 to 968cbee Compare September 6, 2024 20:53
@doanac
Copy link
Member

doanac commented Sep 6, 2024

.. note::
An update can only be performed when the original and update Targets are under the same tag.
In case the Target is tagged differently,
it is required to switch tags before running this command.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather recommend to add the device's tag to the Target, switching tags on a device is rather the last resort.

Maybe we should remove this note at all since this is a general rule, not specific to the CLI, is applicable to any types of update.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. Having it here only adds uneccessary complexity to the documentation. The target would not be listed in the list and check commands anyway. Removing the note.

Copy link
Contributor

@mike-sul mike-sul left a comment

Choose a reason for hiding this comment

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

LGTM, just minor comments.

@detsch detsch force-pushed the detsch-document-aklite-cli branch from 968cbee to f227e91 Compare September 9, 2024 23:08
@doanac
Copy link
Member

doanac commented Sep 9, 2024

@detsch detsch force-pushed the detsch-document-aklite-cli branch 2 times, most recently from 0a1f96f to c9b6130 Compare September 10, 2024 17:46
@detsch detsch marked this pull request as ready for review September 10, 2024 17:48
@doanac
Copy link
Member

doanac commented Sep 10, 2024

@detsch
Copy link
Member Author

detsch commented Sep 10, 2024

@kprosise can you take a look? Feel free to do text adjustments and merge if no changes are required from our side. Thanks!

Copy link
Contributor

@kprosise kprosise left a comment

Choose a reason for hiding this comment

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

Made some minor suggestions, but otherwise it LGTM


sudo aktualizr-lite status

Fetch TUF Metadata and List Updates
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Fetch TUF Metadata and List Updates
Fetch :term:`TUF` Metadata and List Updates

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

The ``install`` command installs the Target, which should have been previously pulled.
It yields an error if the specified Target has not been pulled before, and also supports the ``--update-name`` option.

It is necessary to verify the return codes for each command in order to guarantee the correct update process flow,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It is necessary to verify the return codes for each command in order to guarantee the correct update process flow,
It is necessary to verify the return codes for each command to guarantee the correct update process flow,

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

Comment on lines 214 to 216
^^^^^^^^^^
The commands set exit codes (``echo $?``) that can be used by the caller to act accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
^^^^^^^^^^
The commands set exit codes (``echo $?``) that can be used by the caller to act accordingly.
^^^^^^^^^^
The commands set exit codes (``echo $?``) that can be used by the caller to act accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

- *130*: Failure
- Installation failed and rollback operation was not successful

Automating the use of CLI operations
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Automating the use of CLI operations
Automating the use of CLI Operations

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The individual command line interface operations,
specially ``check``, ``pull``, ``install`` and ``run``,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
specially ``check``, ``pull``, ``install`` and ``run``,
especially ``check``, ``pull``, ``install`` and ``run``,

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

Command Line Interface - CLI (Aktualizr-lite Manual Mode)
---------------------------------------------------------

The `aktualizr-lite` executable can be invoked to perform individual operations allowing more control over the update flow.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `aktualizr-lite` executable can be invoked to perform individual operations allowing more control over the update flow.
The ``aktualizr-lite`` executable can be invoked to perform individual operations allowing more control over the update flow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

and/or disabled with ``sudo systemctl disable aktualizr-lite``.

.. note:: If lmp-device-register is used,
the `--start-daemon 0` is recommended
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the `--start-daemon 0` is recommended
Using ``--start-daemon 0`` is recommended

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.


.. note:: If lmp-device-register is used,
the `--start-daemon 0` is recommended
in order to avoid starting aktualizr-lite daemon automatically.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
in order to avoid starting aktualizr-lite daemon automatically.
in order to avoid starting the aktualizr-lite daemon automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.


sudo aktualizr-lite update

To update to a specific build number or target name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To update to a specific build number or target name,
To update to a specific build number or Target name,

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

Only choose to do so mindfully.
For any update, always test before rolling out to production devices.

Aktualizr-lite command line interface also allows the update steps to be performed individually,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Aktualizr-lite command line interface also allows the update steps to be performed individually,
The command line interface also allows the update steps to be performed individually,

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied.

@kprosise
Copy link
Contributor

@detsch, if you could add the suggestions, if they seem appropriate, then squash the commits, I will then go ahead and merge. Thank you!

@detsch detsch force-pushed the detsch-document-aklite-cli branch from c9b6130 to 9d9037d Compare September 12, 2024 12:53
@detsch detsch force-pushed the detsch-document-aklite-cli branch from 9d9037d to 4502850 Compare September 12, 2024 12:55
@detsch
Copy link
Member Author

detsch commented Sep 12, 2024

Thanks @kprosise! Applied all your suggestions, and rebased to the latest main . Feel free to merge when appropriate.

@doanac
Copy link
Member

doanac commented Sep 12, 2024

@doanac
Copy link
Member

doanac commented Sep 12, 2024

@kprosise kprosise merged commit 4e459db into foundriesio:main Sep 12, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants