Skip to content
This repository has been archived by the owner on Jul 9, 2020. It is now read-only.

Fixes #29141 - CloudInit is compatible w/ Kickstart def user data #691

Merged

Conversation

ofedoren
Copy link
Member

No description provided.

@ofedoren
Copy link
Member Author

@lzap, I heard you've got some experience with CloudInit, could you please review those changes?

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

Other than that, ok.

@ofedoren
Copy link
Member Author

@lzap, done. Extracted into ntp and yum_proxy snippets.

echo "updating system time"
yum -y install ntpdate
/usr/sbin/ntpdate -sub <%= host_param('ntp-server') || '0.fedora.pool.ntp.org' %>
/usr/sbin/hwclock --systohc
Copy link
Member

Choose a reason for hiding this comment

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

One more thing once you on that. In the default kickstart we have this:

  use_ntp = host_param_true?('use-ntp') || (is_fedora && os_major < 16) || (rhel_compatible && os_major <= 7)

<% if use_ntp -%>
ntp
-chrony
<% else -%>
chrony
-ntp
<% end -%>

Esssentially, on older systems we do install ntp but on modern ones we do default to chrony, unless user opts-into the traditional ntp. Can you respect the very same in the snippet? Copy the check over and install either ntp or chrony. I'd also enable start the service as well: systemctl enable --now service.

Copy link
Member

Choose a reason for hiding this comment

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

Beware that ntpdate will not work if chrony is installed, use chronyc -a makestep instead.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I don't like the fedora default pool, I'd probably just provide it an ntp-server if there is any, otherwise i'd keep the default server list that has been installed with the package.

@ofedoren ofedoren force-pushed the bug-29141-cloud-init-kick-def-ud-compat branch from 6e066ea to 44546d8 Compare March 2, 2020 15:35
@ofedoren
Copy link
Member Author

ofedoren commented Mar 2, 2020

@lzap, updated.

I've tried to address your comments. Is it better now?

@ofedoren
Copy link
Member Author

ofedoren commented Apr 2, 2020

@lzap, ping :)

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I haven't tested this end to end, will do during test week tho.

@lzap lzap merged commit 6ee9672 into theforeman:develop Apr 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants