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

chore(docs): update new datasource developer docs #5715

Closed
wants to merge 1 commit into from

Conversation

blackboxsw
Copy link
Collaborator

@blackboxsw blackboxsw commented Sep 18, 2024

Adding a bit more context to new datasource developer docs having mentioned a few of these details in review suggestions on new datasource additions for WSL, CloudCIX and Aeza.net.

Proposed Commit Message

    chore(docs): update new datasource developer docs
    
    Use links instead of :file: for repo-local files to ensure CI
    validates linkcheck on those files in the event that upstream changes
    their names or location in the future.
    
    Reorder ds-identify docs above python DataSource module as the
    python module needs to reflect the same ds-identify detection logic
    in a ds_detect module.
    
    Add that ds-identify does rely on identification alternatives to DMI such as
    attached device labels, kernel command line and virt type.
    Add example reference to simple NWCS datasource commit for ds-identify docs.

Additional Context

Test Steps

tox -e doc
xdg-open doc/rtd_html/development/datasource_creation.html

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>)

@github-actions github-actions bot added the documentation This Pull Request changes documentation label Sep 18, 2024
@blackboxsw blackboxsw force-pushed the docs-ds-development branch 2 times, most recently from 1909fb9 to 77ec8c1 Compare September 18, 2024 18:20
Use links instead of :file: for repo-local files to ensure CI
validates linkcheck on those files in the event that upstream changes
their names or location in the future.

Reorder ds-identify docs above python DataSource module as the
python module needs to reflect the same ds-identify detection logic
in a ds_detect module.

Add that ds-identify does rely on identification alternatives to DMI such as
attached device labels, kernel command line and virt type.
Add example reference to simple NWCS datasource commit for ds-identify docs.
Update ``ds-identify``
----------------------

In ``systemd`` systems, ``ds-identify`` is used to detect which datasource
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added mention of Alpine and BSD too as this is broader than just systemd nowadays

Use one of the simpler datasources such as ``DataSourceHetzner`` as a guiding
template for style and expectations. The DataSource module should implement a
``ds_detect`` method validates the same identification conditions defined
in ds-identify and returns ``True`` when met. This allows cloud-init to support
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@blackboxsw add netv2 discussion here. from Alberto

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not see changes covering this. Maybe they got mangled by some git operation?

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.

One inline nit, but LGTM

Update ``ds-identify``
----------------------

In ``systemd``, Alpine and BSD environments, ``ds-identify`` runs in early boot
Copy link
Member

Choose a reason for hiding this comment

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

I think we can skip the bit about systemd, alpine and bsd environments. I would be very surprised if a new datasource doesn't support any OS that uses ds-identify.

Copy link
Contributor

@aciba90 aciba90 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 improving these docs!


Each cloud platform must positively identify itself to the guest. This allows
the guest to make educated decisions based on the platform on which it is
running. On the x86 and arm64 architectures, many clouds identify themselves
through `DMI`_ data. For example, Oracle's public cloud provides the string
``'OracleCloud.com'`` in the DMI chassis-asset field.
``'OracleCloud.com'`` in the DMI chassis-asset field. Some platforms present
attached devices with well known filesystem label, kernel command line flags or
Copy link
Contributor

Choose a reason for hiding this comment

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

Either

Suggested change
attached devices with well known filesystem label, kernel command line flags or
attached devices with a well known filesystem label, kernel command line flags or

or

Suggested change
attached devices with well known filesystem label, kernel command line flags or
attached devices with well known filesystem labels, kernel command line flags or

right?

to detect which datasource should be enabled, or if ``cloud-init`` should run
at all. You'll need to add early identification support for the platform via a
``dscheck_<CloudPlatform>`` function in `tools/ds-identify`_.
For example, see `NWCS support`_
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
For example, see `NWCS support`_
For example, see `NWCS support`_.

Add datasource module cloudinit/sources/DataSource<CloudPlatform>.py
--------------------------------------------------------------------

We suggest you start by copying one of the simpler datasources
such as ``DataSourceHetzner``.
Use one of the simpler datasources such as ``DataSourceHetzner`` as a guiding
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
Use one of the simpler datasources such as ``DataSourceHetzner`` as a guiding
Use one of the simpler datasources, such as ``DataSourceHetzner``, as a guiding

Use one of the simpler datasources such as ``DataSourceHetzner`` as a guiding
template for style and expectations. The DataSource module should implement a
``ds_detect`` method validates the same identification conditions defined
in ds-identify and returns ``True`` when met. This allows cloud-init to support
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not see changes covering this. Maybe they got mangled by some git operation?

Copy link

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

@github-actions github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Oct 10, 2024
@github-actions github-actions bot closed this Oct 17, 2024
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 stale-pr Pull request is stale; will be auto-closed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants