cc_set_hostname: ignore /var/lib/cloud/data/set-hostname if it's empty#1967
Merged
Conversation
If the file exists but is empty, do nothing. Otherwise cloud-init will crash because it does not handle the empty file. RHBZ: 2140893 Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
aciba90
suggested changes
Jan 18, 2023
aciba90
left a comment
Contributor
There was a problem hiding this comment.
Thanks, @esposem, for the fix.
It looks reasonable to be defensive in the case that the artifact file is empty.
Do you happen to know why is that file empty in the first place? AFAICT it should be only written by cc_set_hostname with valid non-empty json content.
Could we please cover this case with a unit test? Something in the lines of:
diff --git a/tests/unittests/config/test_cc_set_hostname.py b/tests/unittests/config/test_cc_set_hostname.py
index 3d1d86eef..57f531200 100644
--- a/tests/unittests/config/test_cc_set_hostname.py
+++ b/tests/unittests/config/test_cc_set_hostname.py
@@ -2,6 +2,7 @@
import logging
import os
+from pathlib import Path
import shutil
import tempfile
from io import BytesIO
@@ -242,5 +243,21 @@ class TestHostname(t_help.FilesystemMockingTestCase):
str(ctx_mgr.exception),
)
+ def test_ignore_empty_previous_artifact_file(self):
+ cfg = {
+ "hostname": "blah",
+ "fqdn": "blah.blah.blah.yahoo.com",
+ }
+ distro = self._fetch_distro("debian")
+ paths = helpers.Paths({"cloud_dir": self.tmp})
+ ds = None
+ cc = cloud.Cloud(ds, paths, {}, distro, None)
+ self.patchUtils(self.tmp)
+ prev_fn = Path(cc.get_cpath("data")) / "set-hostname"
+ prev_fn.touch()
+ cc_set_hostname.handle("cc_set_hostname", cfg, cc, LOG, [])
+ contents = util.load_file("/etc/hostname")
+ self.assertEqual("blah", contents.strip())
+
# vi: ts=4 expandtab
Contributor
Author
|
Hi @aciba90, thanks for the review! No honestly we don't know why is the file empty, but our QA managed to reproduce the bug. I'll add your test in my PR, thanks! |
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Commit Message
If the file exists but is empty, do nothing.
Otherwise cloud-init will crash because it does not handle the empty file.
RHBZ: 2140893
Signed-off-by: Emanuele Giuseppe Esposito eesposit@redhat.com
Test Steps
Checklist: