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

Restore old behavior of not setting empty hostnames #25

Merged

Conversation

MichaelEischer
Copy link
Contributor

@MichaelEischer MichaelEischer commented Jan 15, 2024

Restore old behavior of not setting empty hostnames

The refactoring in #21
caused hostnames to be set unconditionally compared to the old behavior
of only setting the hostname if it not empty.

When running coreos-cloudinit with datasources that do not provide
metadata such as the file datasource, the refactored code caused the
hostname to always be reset to localhost. This leads to various
problems like preventing k8s nodes from joining their cluster.

This change restores the old behavior by not applying empty hostnames.

Fixes flatcar/Flatcar#1262

How to use

create a file config with content:

#cloud-config

coreos: {}

Then run ./coreos-cloudinit --from-file=config

Testing done

Run the above command. Output without PR:

2024/01/15 14:16:30 Checking availability of "local-file"
2024/01/15 14:16:30 Fetching meta-data from datasource of type "local-file"
2024/01/15 14:16:30 Fetching user-data from datasource of type "local-file"
2024/01/15 14:16:31 Set hostname to 
2024/01/15 14:16:31 Running part "cloud-config.yaml" (cloud-config)
2024/01/15 14:16:31 Updated /etc/environment
2024/01/15 14:16:31 Ensuring runtime unit file "etcd.service" is unmasked
2024/01/15 14:16:31 Ensuring runtime unit file "etcd2.service" is unmasked
2024/01/15 14:16:31 Ensuring runtime unit file "fleet.service" is unmasked
2024/01/15 14:16:31 Ensuring runtime unit file "locksmithd.service" is unmasked

with PR

2024/01/15 14:15:05 Checking availability of "local-file"
2024/01/15 14:15:05 Fetching meta-data from datasource of type "local-file"
2024/01/15 14:15:05 Fetching user-data from datasource of type "local-file"
2024/01/15 14:15:05 Running part "cloud-config.yaml" (cloud-config)
2024/01/15 14:15:05 Ensuring runtime unit file "etcd.service" is unmasked
2024/01/15 14:15:05 Ensuring runtime unit file "etcd2.service" is unmasked
2024/01/15 14:15:05 Ensuring runtime unit file "fleet.service" is unmasked
2024/01/15 14:15:05 Ensuring runtime unit file "locksmithd.service" is unmasked
  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

The refactoring in flatcar#21
caused hostnames to be set unconditionally compared to the old behavior
of only setting the hostname if it not empty.

When running coreos-cloudinit with datasources that do not provide
metadata such as the `file` datasource, the refactored code caused the
hostname to always be reset to `localhost`. This leads to various
problems like preventing k8s nodes from joining their cluster.

This change restores the old behavior by not applying empty hostnames.

Fixes flatcar/Flatcar#1262
@MichaelEischer MichaelEischer force-pushed the do-not-set-empty-hostname branch from b4bd3c5 to 47d54a3 Compare January 15, 2024 14:24
@gabriel-samfira
Copy link
Member

ohh wow. Sorry for the headache!

@jepio jepio merged commit 846cbf9 into flatcar:flatcar-master Jan 15, 2024
1 check passed
@MichaelEischer
Copy link
Contributor Author

MichaelEischer commented Jan 15, 2024

The (previous) CI error (https://github.com/flatcar/coreos-cloudinit/actions/runs/7530122553/job/20496040224?pr=25) is odd, for me locally gofmt (Go 1.21.6) complained about the formatting, but now the CI is unhappy. I've dropped that commit.

@MichaelEischer MichaelEischer deleted the do-not-set-empty-hostname branch January 15, 2024 14:26
@MichaelEischer
Copy link
Contributor Author

Thanks!

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.

Hostname gets reset in Azure VM with alpha 3794.0.0
3 participants