From c23bb174858f82c01a428877c7f4c9933ac0fee4 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Wed, 16 Mar 2022 18:01:38 +0000 Subject: [PATCH 1/9] BUG 1473527: module ssh-authkey-fingerprints fails Input/output error: /dev/console --- cloudinit/util.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 10a7ca71234..7855b72111c 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -29,7 +29,7 @@ import stat import string import subprocess -import sys +import import time from base64 import b64decode, b64encode from errno import ENOENT @@ -393,14 +393,23 @@ def multi_log( sys.stderr.write(text) if console: conpath = "/dev/console" + writing_to_console_failed = False if os.path.exists(conpath): - with open(conpath, "w") as wfh: - wfh.write(text) - wfh.flush() - elif fallback_to_stdout: - # A container may lack /dev/console (arguably a container bug). If - # it does not exist, then write output to stdout. this will result - # in duplicate stderr and stdout messages if stderr was True. + try: + with open(conpath, "w") as wfh: + wfh.write(text) + wfh.flush() + except OSError: + writing_to_console_failed = True + + if fallback_to_stdout or writing_to_console_failed: + # A container may lack /dev/console (arguably a container bug). + # Additionally, /dev/console may not be writable to on a VM (again + # likely a VM bug). + # + # If either of these is the case, then write output to stdout. + # This will result in duplicate stderr and stdout messages if + # stderr was True. # # even though upstart or systemd might have set up output to go to # /dev/console, the user may have configured elsewhere via From 11813f06a24a3fe104049f5faa13872fde28a09e Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Thu, 17 Mar 2022 11:47:25 +0000 Subject: [PATCH 2/9] Add to contributors list, fix tox doc, make code compile --- cloudinit/util.py | 11 ++++++----- tools/.github-cla-signers | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 7855b72111c..3f78bb22164 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -29,7 +29,7 @@ import stat import string import subprocess -import +import sys import time from base64 import b64decode, b64encode from errno import ENOENT @@ -393,16 +393,17 @@ def multi_log( sys.stderr.write(text) if console: conpath = "/dev/console" - writing_to_console_failed = False + writing_to_console_worked = False if os.path.exists(conpath): try: with open(conpath, "w") as wfh: wfh.write(text) wfh.flush() + writing_to_console_worked = True except OSError: - writing_to_console_failed = True - - if fallback_to_stdout or writing_to_console_failed: + pass + + if fallback_to_stdout and not writing_to_console_worked: # A container may lack /dev/console (arguably a container bug). # Additionally, /dev/console may not be writable to on a VM (again # likely a VM bug). diff --git a/tools/.github-cla-signers b/tools/.github-cla-signers index 5f05dba907f..3410815608f 100644 --- a/tools/.github-cla-signers +++ b/tools/.github-cla-signers @@ -6,6 +6,7 @@ Aman306 andgein andrewbogott andrewlukoshko +andrew-lee-metaswitch antonyc aswinrajamannar beantaxi From fa94c62eb0797e4260bd2901fb1d5f07c2c95dd5 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Fri, 18 Mar 2022 14:45:03 +0000 Subject: [PATCH 3/9] Add log --- cloudinit/util.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 3f78bb22164..6ee0ad13b57 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -401,12 +401,15 @@ def multi_log( wfh.flush() writing_to_console_worked = True except OSError: - pass + console_error = "Failed to write to /dev/console" + sys.stdout.write(console_error) + if log: + log.log(logging.WARNING, console_error) if fallback_to_stdout and not writing_to_console_worked: # A container may lack /dev/console (arguably a container bug). # Additionally, /dev/console may not be writable to on a VM (again - # likely a VM bug). + # likely a VM bug or virtualization bug). # # If either of these is the case, then write output to stdout. # This will result in duplicate stderr and stdout messages if From 7d2c74768dad9ba19f8c165c8d6ead1c177238d3 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Fri, 25 Mar 2022 11:53:53 +0000 Subject: [PATCH 4/9] Add UTs --- cloudinit/util.py | 10 +++++++--- tests/unittests/test_util.py | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 6ee0ad13b57..1ba17a1d4d0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -381,6 +381,12 @@ def find_modules(root_dir) -> dict: return entries +def write_to_console(conpath, text): + with open(conpath, "w") as wfh: + wfh.write(text) + wfh.flush() + + def multi_log( text, console=True, @@ -396,9 +402,7 @@ def multi_log( writing_to_console_worked = False if os.path.exists(conpath): try: - with open(conpath, "w") as wfh: - wfh.write(text) - wfh.flush() + write_to_console(conpath, text) writing_to_console_worked = True except OSError: console_error = "Failed to write to /dev/console" diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 3f3079b04e7..363661bb325 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1982,6 +1982,30 @@ def test_logs_dont_go_to_stdout_if_fallback_to_stdout_is_false(self): util.multi_log("something", fallback_to_stdout=False) self.assertEqual("", self.stdout.getvalue()) + @mock.patch( + "cloudinit.util.write_to_console", + mock.Mock(side_effect=OSError("Failed to write to console")), + ) + def test_logs_do_go_to_stdout_if_writing_to_console_fails_and_fallback_true(self): + self._createConsole(self.root) + util.multi_log("something", fallback_to_stdout=True) + self.assertEqual( + "Failed to write to /dev/consolesomething", self.stdout.getvalue() + ) + + @mock.patch( + "cloudinit.util.write_to_console", + mock.Mock(side_effect=OSError("Failed to write to console")), + ) + def test_logs_go_nowhere_if_writing_to_console_fails_and_fallback_false( + self, + ): + self._createConsole(self.root) + util.multi_log("something", fallback_to_stdout=False) + self.assertEqual( + "Failed to write to /dev/console", self.stdout.getvalue() + ) + def test_logs_go_to_log_if_given(self): log = mock.MagicMock() logged_string = "something very important" From ae9e52fceb2af235c5740fd5296282ad5346f109 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Fri, 25 Mar 2022 15:10:26 +0000 Subject: [PATCH 5/9] Black --- tests/unittests/test_util.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 363661bb325..2238f2c4ce1 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1986,7 +1986,9 @@ def test_logs_dont_go_to_stdout_if_fallback_to_stdout_is_false(self): "cloudinit.util.write_to_console", mock.Mock(side_effect=OSError("Failed to write to console")), ) - def test_logs_do_go_to_stdout_if_writing_to_console_fails_and_fallback_true(self): + def test_logs_do_go_to_stdout_if_writing_to_console_fails_and_fallback_true( + self, + ): self._createConsole(self.root) util.multi_log("something", fallback_to_stdout=True) self.assertEqual( From b16cd6b6528add48699ac1193f337327ccf0e9b5 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Mon, 4 Apr 2022 11:16:11 +0100 Subject: [PATCH 6/9] Fix flake8 linter error --- tests/unittests/test_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 2238f2c4ce1..24e75cbade3 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1986,7 +1986,7 @@ def test_logs_dont_go_to_stdout_if_fallback_to_stdout_is_false(self): "cloudinit.util.write_to_console", mock.Mock(side_effect=OSError("Failed to write to console")), ) - def test_logs_do_go_to_stdout_if_writing_to_console_fails_and_fallback_true( + def test_logs_go_to_stdout_if_writing_to_console_fails_and_fallback_true( self, ): self._createConsole(self.root) From 388d7fddd892214a5eeed748cc255f94a74a1a2b Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Tue, 5 Apr 2022 09:07:30 +0100 Subject: [PATCH 7/9] Update cloudinit/util.py Co-authored-by: James Falcon --- cloudinit/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 1ba17a1d4d0..9656d5ae3b6 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -406,7 +406,7 @@ def multi_log( writing_to_console_worked = True except OSError: console_error = "Failed to write to /dev/console" - sys.stdout.write(console_error) + sys.stdout.write(f"{console_error}\n") if log: log.log(logging.WARNING, console_error) From 2ee1d34bf10dcfbca0d7b608995dedf4b9c32f4d Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 7 Apr 2022 14:15:51 -0500 Subject: [PATCH 8/9] unit test update after last update --- tests/unittests/test_util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 24e75cbade3..a8731c01f58 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -1992,7 +1992,8 @@ def test_logs_go_to_stdout_if_writing_to_console_fails_and_fallback_true( self._createConsole(self.root) util.multi_log("something", fallback_to_stdout=True) self.assertEqual( - "Failed to write to /dev/consolesomething", self.stdout.getvalue() + "Failed to write to /dev/console\nsomething", + self.stdout.getvalue(), ) @mock.patch( @@ -2005,7 +2006,7 @@ def test_logs_go_nowhere_if_writing_to_console_fails_and_fallback_false( self._createConsole(self.root) util.multi_log("something", fallback_to_stdout=False) self.assertEqual( - "Failed to write to /dev/console", self.stdout.getvalue() + "Failed to write to /dev/console\n", self.stdout.getvalue() ) def test_logs_go_to_log_if_given(self): From d16f60f334a2adb2c8ba152e9739f1a6eef14844 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 7 Apr 2022 15:33:44 -0500 Subject: [PATCH 9/9] formatting --- cloudinit/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 9656d5ae3b6..2864e18b35d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2078,7 +2078,7 @@ def write_file( omode="wb", preserve_mode=False, *, - ensure_dir_exists=True + ensure_dir_exists=True, ): """ Writes a file with the given content and sets the file mode as specified.