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

doc: Update docs on boothooks #5541

Closed
wants to merge 2 commits into from

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Jul 24, 2024

Proposed Commit Message

doc: Update docs on boothooks

Improve explaination on #cloud-boothook for end-users.

Reorder and improve doc/rtd/explanation/format.rst.

SC-1657
Fixes GH-4542

Co-authored-by: Calvin Mwadime <[email protected]>

Additional Context

This PR builds-up on top of #4650.

Test Steps

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Hmmm...I'm currently overhauling the user formats page per #4739 and a separate entry in our spreadsheet.

Is there any way we can just capture the new info here but remove any of the non-boothook related changes or formatting changes?

@s-makin s-makin added the documentation This Pull Request changes documentation label Jul 24, 2024
Copy link
Collaborator

@a-dubs a-dubs left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this @aciba90!! I left a few inline questions, comments, and suggestions.

The file contains a list of URLs, one per line. Each of the URLs will be read
and their content will be passed through this same set of rules, i.e., the
content read from the URL can be gzipped, MIME multi-part, or plain text. If
an error occurs reading a file the remaining files will not be read.
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is meant by "set of rules". and why is it the same for all urls? This might need re-written for dummies like me :P

:ref:`Network boot stage<boot-Network>`, even before ``cc_bootcmd``.

This can be used when something has to be configured very early on boot,
potentially on every boot, with less convenience as ``cc_bootcmd`` but more
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
potentially on every boot, with less convenience as ``cc_bootcmd`` but more
potentially on every boot, with less convenience than ``cc_bootcmd``, but more

flexibility.

.. note::
Boothooks are execute on every boot.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Boothooks are execute on every boot.
Boothooks are executed on every boot.

Boothooks are execute on every boot.
The environment variable ``INSTANCE_ID`` will be set to the current instance
ID. ``INSTANCE_ID`` can be used to implement a `once-per-instance` type of
functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add a final little sentence saying as demonstrated below in [link to "Example of once-per-instance script" section]

@@ -121,6 +193,48 @@ The :command:`make-mime` subcommand takes pairs of (filename, "text/" mime
subtype) separated by a colon (e.g., ``config.yaml:cloud-config``) and emits a
MIME multipart message to :file:`stdout`.

User data format to Mime Content type mapping
Copy link
Collaborator

Choose a reason for hiding this comment

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

should "Mime Content" be lowercase?

Copy link

Choose a reason for hiding this comment

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

or maybe "MIME content"

+----------------------------+----------------------------+
| x-shellscript-per-boot | x-shellscript-per-boot |
+----------------------------+----------------------------+

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think i might just be dense or have overlooked something, but I am unsure of what to do with this information now.

Where do I put Content-Type: text/<content-type>? I just don't know how or where this is used. "content-type" doesn't seem to appear really anywhere else in these docs.


Content-Type: text/<content-type>

Below is a mapping of a specific user data format to the `content-type`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this in an improvement but still i feel like this sentence is a little bit confusing.

Suggested change
Below is a mapping of a specific user data format to the `content-type`
Below is a mapping of each specific user data format to the `content-type`

@aciba90 aciba90 mentioned this pull request Jul 25, 2024
2 tasks
@aciba90
Copy link
Contributor Author

aciba90 commented Jul 25, 2024

Hmmm...I'm currently overhauling the user formats page per #4739 and a separate entry in our spreadsheet.

Is there any way we can just capture the new info here but remove any of the non-boothook related changes or formatting changes?

I have created #5546 with only changes related to boothook, please feel free to merge, integrate it in your PR or whatever, thanks!

Closing this one.

@aciba90 aciba90 closed this Jul 25, 2024
@aciba90 aciba90 deleted the boothook_docs_update branch August 1, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This Pull Request changes documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants