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

boot.rst: add First Boot Determination section #568

Merged
merged 2 commits into from Sep 16, 2020

Conversation

OddBloke
Copy link
Collaborator

@OddBloke OddBloke commented Sep 7, 2020

LP: #1888858

Copy link
Collaborator

@smoser smoser left a comment

Choose a reason for hiding this comment

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

First, thanks for writing this. Second, 2 general comments, with some in-line:

  1. "first boot", I think really should be consistently "first instance boot" or "first boot of a new instance". cloud-init does identify a "first boot" (per-once) in addition to "first instance boot".
  2. I'd recommend dropping documentation of internal behavior that we do not necessarily wish to support. Instead, provide a cloud-init sub-command that will do what you want.

2 things you could do:

  • cloud-init status could show if manual_cache_clean was set
  • some sub-command could toggle the status of manual_cache_clean.

doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
ID that is present in the system unconditionally. This means that cloud-init
will never detect a first boot when the cache is present, and it follows that
the only way to cause cloud-init to detect a first boot is to manually remove
cloud-init's cache. Internally, this behavior is referred to as ``trust``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd just scrap "Internally....".
This is user-facing documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Scrap the whole sentence, or just "Internally, "?

(If the former: I would like to keep the names of the behaviours, because I think it makes it easier to know where to refer back to when you're reading the later parts of the documentation, and "in trust mode" is more concise than "has manual_cache_clean set to True".)

doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
@OddBloke
Copy link
Collaborator Author

OddBloke commented Sep 8, 2020

Thanks for the review Scott, this is much improved now. I believe I've addressed all your comments except for the one I've left unresolved.

Copy link
Contributor

@lucasmoura lucasmoura left a comment

Choose a reason for hiding this comment

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

I have no problem with the provided configuration, but just a question regarding instance-id. For EC2, isn't it possible for the instance-id to change without requiring cloud-init to reset the setting of the machine ? For example, if we change the region of an instance I think the instance-id will change, but would we like for cloud-init to reset the setting in that scenario ?

@smoser
Copy link
Collaborator

smoser commented Sep 10, 2020 via email

Copy link
Collaborator

@smoser smoser left a comment

Choose a reason for hiding this comment

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

some minor cleanups requested. this does look much better.

doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
doc/rtd/topics/boot.rst Outdated Show resolved Hide resolved
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.

None yet

3 participants