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

Docs / debugging #4578

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Nov 3, 2023

Context

Followup to PR #4500.

  • add degraded state documentation
  • add content describing how to rerun cloud-init
  • add content describing how to start debugging cloudinit, categorized by failure state
  • relocates various bits of content to more appropriate locations

Fixes #4476.

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

@holmanb holmanb requested a review from s-makin November 3, 2023 20:19
@holmanb holmanb force-pushed the holmanb/docs-add-debugging branch 2 times, most recently from c3a23ad to 18fdf83 Compare November 5, 2023 12:47
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Remove duplicate content that is better described in tutorials.
Add content describing how to re-run cloud-init.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Most of this content isn't useful for debugging
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
- Includes initial steps to take based on class of failure.
- Relocate some FAQ content which is better served here.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Put reference material where it belongs.
Hide implementation details in development docs.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Remove duplicate content that is better described in tutorials.
Add content describing how to re-run cloud-init.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Most of this content isn't useful for debugging
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
- Includes initial steps to take based on class of failure.
- Relocate some FAQ content which is better served here.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Put reference material where it belongs.
Hide implementation details in development docs.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
- Includes initial steps to take based on class of failure.
- Relocate some FAQ content which is better served here.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
Put reference material where it belongs.
Hide implementation details in development docs.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 5, 2023
@holmanb holmanb added the 23.4 label Nov 15, 2023
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.

Overall, a really good change. Thanks!

I left some inline comments. Additionally:
The TOC is messed up. I'm seeing multiple elements of pages being rendered in the TOC for both How-to guides and Explanation. E.g.:
image

Also, I would personally put 'How to debug cloud-init' into How-to guides rather than Development, because debugging is often required even if there's no cloud-init development happening.
I think I would also move Internal Files: data to reference. It might also be useful for debugging, and not just development.

doc/rtd/howto/debugging.rst Outdated Show resolved Hide resolved
doc/rtd/howto/debugging.rst Outdated Show resolved Hide resolved
doc/rtd/howto/debugging.rst Outdated Show resolved Hide resolved
doc/rtd/howto/debugging.rst Outdated Show resolved Hide resolved
doc/rtd/howto/debugging.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/failure_states.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/failure_states.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/failure_states.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/failure_states.rst Show resolved Hide resolved
doc/rtd/explanation/failure_states.rst Outdated Show resolved Hide resolved
@holmanb
Copy link
Member Author

holmanb commented Nov 15, 2023

Thanks for the review @TheRealFalcon!

Also, I would personally put 'How to debug cloud-init' into How-to guides rather than Development, because debugging is often required even if there's no cloud-init development happening.

I agree, which is why it was linked from the How-to section as well. It is linked from both pages, which I guess was an odd choice. I can drop the development one, I guess, though I'm not sure that I am convinced that content being categorically "how to" makes it unfit for a link from the dev docs page. Thoughts?

@TheRealFalcon
Copy link
Member

Ah, I didn't catch that it was linked in both places. I see your point. I'm fine with it but would defer to @s-makin on that one.

holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 16, 2023
Also organize other "how to run cloud-init locally" content better.

Fixes canonicalGH-4608
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 16, 2023
Also organize other "how to run cloud-init locally" content better.

Fixes canonicalGH-4608
holmanb added a commit to holmanb/cloud-init that referenced this pull request Nov 16, 2023
Copy link
Contributor

@s-makin s-makin 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 the changes! It's looking really good and is a huge improvement over what we had before. I've left comments on a couple of the discussions, but otherwise I think your explanations make sense and am happy to accept them as-is. I found a few remaining nits and left suggestions for those, but happy to accept once those are dealt with.

holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 1, 2023
Also organize other "how to run cloud-init locally" content better.

Fixes canonicalGH-4608
holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 1, 2023
@holmanb
Copy link
Member Author

holmanb commented Dec 1, 2023

Thanks @s-makin for the re-review! I think I've addressed everything!

@holmanb
Copy link
Member Author

holmanb commented Dec 1, 2023

@TheRealFalcon Looks like your review is still in a "requested changes" state. I still need to apply / split out the changes in the comments in this PR into logical commits. I'm happy to do that before or after review. I haven't force pushed in case you wanted to compare from when you left off, although 15 files changed, 549 insertions(+), 212 deletions(-) with 3 new files since then so maybe you don't mind if I do that?

@TheRealFalcon
Copy link
Member

@holmanb , structure however you'd like. I'll re-review this one from scratch.

holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 5, 2023
This page described various topics adjacent to debugging, but not
debugging itself. Organize and curate content.

Create new pages describing:

- performance analysis
- testing new cloud-init releases on Ubuntu
- Ubuntu SRU process
holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 5, 2023
Put reference material where it belongs.
Hide implementation details in development docs.
holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 5, 2023
holmanb added a commit to holmanb/cloud-init that referenced this pull request Dec 5, 2023
- New page and content describing debugging for users
- New page and content documenting cloud-init's status
- New page and content documenting cloud-init's exported errors
- New page and content documenting cloud-init's failure states
- New page and content documenting how to re-run cloud-init
- New content documenting how validate user-data
- New content documenting how to use cloud-init with libvirt

Documents canonicalGH-4500
Fixes canonicalGH-4608
@holmanb
Copy link
Member Author

holmanb commented Dec 5, 2023

@TheRealFalcon Ready for review :)

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.

Just what you wanted...more comments on the PR 😉

Since this PR is already very large, I'm ok saving most comments for the future. For the ones that I think really should get fixed before merging, I prepended with fix:

General note:
I don't love the use of jq in our docs. While it's very commonly used by a certain subset of developer, I wouldn't consider it to be a universal Linux tool nor does it come pre-installed on most distros. Is it that bad to show actual output? Could we add emphasis to what we're trying to highlight with styling?

@@ -0,0 +1,46 @@
.. _internal_files:
Copy link
Member

Choose a reason for hiding this comment

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

It's probably worth calling out some files in /run too (outside of what we mention in the log and configuration files page).

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed

doc/rtd/explanation/exported_errors.rst Outdated Show resolved Hide resolved
doc/rtd/explanation/exported_errors.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,74 @@
.. _failure_states:
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 this page could be merged with the 'Exported Errors' page, especially since the exported errors page references recoverable failures without defining them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could be, but while related they are really different concepts. One is about the states of cloud-init, the other is related to extracting information if cloud-init is in one of those states. I'd rather keep them separate for now, but I'll add a link to the page since it isn't defined here - no need to re-define.

doc/rtd/explanation/failure_states.rst Outdated Show resolved Hide resolved
doc/rtd/howto/debugging.rst Show resolved Hide resolved
doc/rtd/howto/ubuntu_test_prerelease.rst Outdated Show resolved Hide resolved
===========================

Afterthe cloud-init team creates an upstream release, cloud-init will
be released in the -proposed APT repository for a
Copy link
Member

Choose a reason for hiding this comment

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

I want to make a stronger case for the "pocket" terminology. Repository is fairly generic term, and can refer to pretty much any place to get software. You could talk about the "Main" or "Universe" repository along with the "-updates" or "-proposed" repository, even though "Universe" and "proposed" aren't interchangeable. "Pocket" is more specific and lets you know exactly what is being referred to. You can't really find it in Debian docs (except the glossary) because it's ubuntu-specific.

Debian defines it here:
https://wiki.debian.org/Glossary#pocket

The Archive Repository Pockets section explains it well here:
https://ubuntu.com/blog/ubuntu-updates-releases-and-repositories-explained#The_archive_repository_pockets

Pocket is found 15 times on the SRU page, vs 1 for repository:
https://wiki.ubuntu.com/StableReleaseUpdates

There's more explanation of it here:
https://wiki.ubuntu.com/SecurityTeam/FAQ#Repositories_and_Updates

It's used in other ubuntu-specific tooling like here (search pocket):
https://manpages.ubuntu.com/manpages/jammy/man1/add-apt-repository.1.html

Since pocket is an established term that is more specific than repository, I think we should be preferring it.

doc/rtd/howto/ubuntu_test_prerelease.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,49 @@
.. _stable_release_updates:
Copy link
Member

Choose a reason for hiding this comment

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

Yes, the new version is great!

@holmanb
Copy link
Member Author

holmanb commented Dec 6, 2023

I don't love the use of jq in our docs. While it's very commonly used by a certain subset of developer, I wouldn't consider it to be a universal Linux tool nor does it come pre-installed on most distros. Is it that bad to show actual output? Could we add emphasis to what we're trying to highlight with styling?

That's fair. It gets really verbose especially on older series that still expect the .v1. to exist, but I'm fine with trying to stick to more commonly available tools as much as possible and not assuming familiarity and availability of non-core tools.

@holmanb
Copy link
Member Author

holmanb commented Dec 6, 2023

I want to make a stronger case for the "pocket" terminology.

Thanks for that. The blog describes it the best I've seen it so far, no objections to this request.

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! We might have a cloud-init record with this one 😄

This page described various topics adjacent to debugging, but not
debugging itself. Organize and curate content.

Create new pages describing:

- performance analysis
- testing new cloud-init releases on Ubuntu
- Ubuntu SRU process
Put reference material where it belongs.
Hide implementation details in development docs.
- New page and content describing debugging for users
- New page and content documenting cloud-init's status
- New page and content documenting cloud-init's exported errors
- New page and content documenting cloud-init's failure states
- New page and content documenting how to re-run cloud-init
- New content documenting how validate user-data
- New content documenting how to use cloud-init with libvirt

Documents canonicalGH-4500
Fixes canonicalGH-4608
@holmanb
Copy link
Member Author

holmanb commented Dec 6, 2023

LGTM! We might have a cloud-init record with this one 😄

Thanks! I just squashed the last couple of commits and will land once tests pass.

@holmanb holmanb merged commit 1d6918b into canonical:main Dec 6, 2023
28 checks passed
holmanb added a commit that referenced this pull request Dec 6, 2023
This page described various topics adjacent to debugging, but not
debugging itself. Organize and curate content.

Create new pages describing:

- performance analysis
- testing new cloud-init releases on Ubuntu
- Ubuntu SRU process
holmanb added a commit that referenced this pull request Dec 6, 2023
Put reference material where it belongs.
Hide implementation details in development docs.
holmanb added a commit that referenced this pull request Dec 6, 2023
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.

[docs]: Content Inappropriately Structured
4 participants