-
Notifications
You must be signed in to change notification settings - Fork 844
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
schema: add json defs for modules e-install (SC-651) #1366
Conversation
* cc_emit_upstart (removal) * cc_fan * cc_final_message * cc_foo * cc_growpart * cc_grub_dpkg * cc_install_hotplug
7fbf17b
to
cc21bf5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. Inline comments throughout. I don't feel too strongly about the cc_growpart logic, but I think we we are truly going to deprecate some of these values or types, we should per logging deprecation warnings about them in this release.
cloudinit/config/cc_fan.py
Outdated
@@ -18,32 +22,35 @@ | |||
- write ``config_path`` with the contents of the ``config`` key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also document that ubuntu-fan
package will be installed if absent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
MODULE_DESCRIPTION = """\ | ||
This module configures the final message that cloud-init writes. The message is | ||
specified as a jinja template with the following variables set: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also document that this module writes /var/lib/cloud/instance/boot-finished
on exit. systemd units could rely on the presence of this file to trigger other units to start (if they don't want to use cloud-init status --wait
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Pushed next set from comments.
I think I added warnings for everything here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done, thanks for the updates here.
Proposed Commit Message
Additional Context
Removed 'cc_emit_upstart' entirely. According to https://upstart.ubuntu.com/ , the only place we'll see upstart anymore is trusty and RHEL 6, neither of which will ever see a new upstream change