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 drop in 3rd party modules #5548

Merged
merged 6 commits into from
Jul 26, 2024

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Jul 25, 2024

Proposed Commit Message

doc: improve drop-in custom modules

Add group of pages for drop-in custom modules and
restructure exitent doc under it.

Add doc for custom datasources and config modules.
    
SC-1836
Fixes GH-4649

Additional Context

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

@github-actions github-actions bot added the documentation This Pull Request changes documentation label Jul 25, 2024
aciba90 added a commit to aciba90/cloud-init that referenced this pull request Jul 25, 2024
aciba90 added a commit to aciba90/cloud-init that referenced this pull request Jul 25, 2024
aciba90 added a commit to aciba90/cloud-init that referenced this pull request Jul 25, 2024
@aciba90 aciba90 force-pushed the doc-drop-in-3rd-party-modules branch from 9a3dd24 to 6e5f22e Compare July 25, 2024 14:04
@blackboxsw blackboxsw self-assigned this Jul 25, 2024
doc/rtd/reference/cli.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/format.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_formats.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_formats.rst Outdated Show resolved Hide resolved
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.

I left some inline comments.

Each new page is currently fairly small. Do we foresee them growing much? If not, I do wonder if we could collapse all of the new pages into a single "custom modules" page. @s-makin or @holmanb (since you initially filed the issue), any strong opinions?

doc/rtd/reference/custom_modules/custom_datasource.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/format.rst Outdated Show resolved Hide resolved
doc/rtd/reference/cli.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_cleaners.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_mergers.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_mergers.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_mergers.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_mergers.rst Outdated Show resolved Hide resolved
Copy link
Member

@holmanb holmanb 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 tackling this @aciba90! This is looking great so far. I think we can add some more content to help fill out these pages some more. See my comments below.

Each new page is currently fairly small. Do we foresee them growing much? If not, I do wonder if we could collapse all of the new pages into a single "custom modules" page. @s-makin or @holmanb (since you initially filed the issue), any strong opinions?

I'd like to see some more content on these pages before we land it, but I really like the structure and organization that this brings to the topics. I'll do a more thorough review tomorrow with some ideas (running out of time now), but as long as we get things properly cross-linked from the appropriate other pages to here, I think that having separate smaller pages that have single focus is a better user experience than large pages that group various things by related topics.

doc/rtd/reference/custom_modules.rst Outdated Show resolved Hide resolved
doc/rtd/reference/custom_modules/custom_cleaners.rst Outdated Show resolved Hide resolved
Custom 3rd-party out-of-tree configuration modules can be added to cloud-init
by:

#. :ref:`Implement a config module<module_creation>` in a Python file with its
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if the referenced page wouldn't make more sense here than in the dev docs. What do you think? There may be some overlap in the intent, but since the intent is that users should be able to do this I think that having it under the "custom_modules" umbrella makes more sense. I think that as far as contributing goes, there are probably more useful things for new contributors to be thinking about than writing custom configuration modules for niche purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it is okay either way because:

That content is in the gray are where the persona is an advanced user that is consuming cloud-init as a user but is giving input to cloud-init that requires develoment knowledge about cloud-init's internal.

The same question applies for the documentation on how to create a new datasource and for #5559.

@aciba90 aciba90 force-pushed the doc-drop-in-3rd-party-modules branch from 6e5f22e to c3a3eb7 Compare July 26, 2024 08:20
@aciba90
Copy link
Contributor Author

aciba90 commented Jul 26, 2024

Thanks for the reviews, much appreciated!

As #5551 is comming and per your recommendations, I have reverted doc/rtd/explanation/format.rst, removed
doc/rtd/reference/custom_modules/custom_formats.rst and linked from custom_modules.rst to doc/rtd/explanation/format.rst#part-handler.

Let me know if something can be further improved. I am happy merging conflicts and / or minimizing the changes surface.

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.

LGTM!

Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thanks @aciba90 I think it's worth pushing forward and tracking the significant iterations as separate issues.

Other inline nits as single diff (other than

diff --git a/doc/rtd/development/datasource_creation.rst b/doc/rtd/development/datasource_creation.rst
index 5f8d2b02d..1b6e525b1 100644
--- a/doc/rtd/development/datasource_creation.rst
+++ b/doc/rtd/development/datasource_creation.rst
@@ -171,7 +171,7 @@ compatibility with the rest of the codebase, and security fixes by the upstream
 development team.
 
 If this is not possible, one can add
-:ref:`custom out-of-tree datasources to cloud-init<custom_datasource>`.
+:ref:`custom out-of-tree datasources<custom_datasource>` to cloud-init.
 
 .. _make-mime: https://cloudinit.readthedocs.io/en/latest/explanation/instancedata.html#storage-locations
 .. _DMI: https://www.dmtf.org/sites/default/files/standards/documents/DSP0005.pdf
diff --git a/doc/rtd/development/module_creation.rst b/doc/rtd/development/module_creation.rst
index f9d6d0364..3e10a1ee0 100644
--- a/doc/rtd/development/module_creation.rst
+++ b/doc/rtd/development/module_creation.rst
@@ -172,7 +172,7 @@ compatibility with the rest of the codebase, and security fixes by the upstream
 development team.
 
 If this is not possible, one can add
-:ref:`custom out-of-tree config module<custom_configuration_module>`
+:ref:`custom out-of-tree config modules<custom_configuration_module>`
 to cloud-init.
 
 .. _MetaSchema: https://github.com/canonical/cloud-init/blob/3bcffacb216d683241cf955e4f7f3e89431c1491/cloudinit/config/schema.py#L58
diff --git a/doc/rtd/reference/cli.rst b/doc/rtd/reference/cli.rst
index ca6d35a7a..eb800b22a 100644
--- a/doc/rtd/reference/cli.rst
+++ b/doc/rtd/reference/cli.rst
@@ -84,7 +84,7 @@ re-run all stages as it did on first boot.
 .. note::
 
    The operations performed by `clean` can be supplemented / customized. See:
-   :ref:`custom_cleaners`.
+   :ref:`custom_clean_scripts`.
 
 .. _cli_collect_logs:
 
diff --git a/doc/rtd/reference/custom_modules.rst b/doc/rtd/reference/custom_modules.rst
index 92c84a339..3145e723b 100644
--- a/doc/rtd/reference/custom_modules.rst
+++ b/doc/rtd/reference/custom_modules.rst
@@ -18,7 +18,7 @@ config via user-data or vendor-data.
 .. toctree::
    :maxdepth: 1
 
-   custom_modules/custom_cleaners.rst
+   custom_modules/custom_clean_scripts.rst
    custom_modules/custom_configuration_module.rst
    custom_modules/custom_datasource.rst
    custom_modules/custom_mergers.rst
diff --git a/doc/rtd/reference/custom_modules/custom_cleaners.rst b/doc/rtd/reference/custom_modules/custom_clean_scripts.rst
similarity index 92%
rename from doc/rtd/reference/custom_modules/custom_cleaners.rst
rename to doc/rtd/reference/custom_modules/custom_clean_scripts.rst
index 17d57d445..955668fb2 100644
--- a/doc/rtd/reference/custom_modules/custom_cleaners.rst
+++ b/doc/rtd/reference/custom_modules/custom_clean_scripts.rst
@@ -1,7 +1,7 @@
-.. _custom_cleaners:
+.. _custom_clean_scripts:
 
-Custom Cleaners
-***************
+Custom Clean Scripts
+********************
 
 Cloud-init provides the directory :file:`/etc/cloud/clean.d/` for third party
 applications which need additional configuration artifact cleanup from

Comment on lines 1 to 3
.. _custom_cleaners:

Custom Cleaners
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make this a bit more descriptive:

Suggested change
.. _custom_cleaners:
Custom Cleaners
.. _custom_clean_scripts:
Custom Clean Scripts

@@ -170,6 +170,8 @@ Datasources included in upstream cloud-init benefit from ongoing maintenance,
compatibility with the rest of the codebase, and security fixes by the upstream
development team.

If this is not possible, one can add
:ref:`custom out-of-tree datasources to cloud-init<custom_datasource>`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: pull cloud-init out of this link text (aligns w/ how we setup the link for custom modules in the other file)

development team.

If this is not possible, one can add
:ref:`custom out-of-tree config module<custom_configuration_module>`
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
:ref:`custom out-of-tree config module<custom_configuration_module>`
:ref:`custom out-of-tree config modules<custom_configuration_module>`

sudo rm -rf /var/log/installer/

The operations performed by `clean` can be supplemented / customized. See:
:ref:`custom_cleaners`.
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
:ref:`custom_cleaners`.
:ref:`custom_clean_scripts`.

.. toctree::
:maxdepth: 1

custom_modules/custom_cleaners.rst
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
custom_modules/custom_cleaners.rst
custom_modules/custom_clean_scripts.rst

Comment on lines 1 to 4
.. _custom_cleaners:

Custom Cleaners
***************
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's make this a bit more descriptive (and probably rename the file too)

Suggested change
.. _custom_cleaners:
Custom Cleaners
***************
.. _custom_cleaner_scripts:
Custom Clean Scripts
******************

Copy link
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

Looks good to me! This is an improvement over what we had before.

@blackboxsw your patch on top of this looks good, if you want to apply now and merge.

@blackboxsw blackboxsw merged commit 914a3a8 into canonical:main Jul 26, 2024
24 checks passed
@aciba90 aciba90 deleted the doc-drop-in-3rd-party-modules branch August 1, 2024 08:23
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Aug 2, 2024
Add group of pages for drop-in custom modules and
restructure existing docs under it.

Add doc for custom datasources and config modules.
    
SC-1836
Fixes canonicalGH-4649
holmanb pushed a commit that referenced this pull request Aug 6, 2024
Add group of pages for drop-in custom modules and
restructure existing docs under it.

Add doc for custom datasources and config modules.
    
SC-1836
Fixes GH-4649
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.

4 participants