Merge ~chad.smith/cloud-init:bug/status-fix-done-status into cloud-init:master

Proposed by Chad Smith
Status: Merged
Approved by: Chad Smith
Approved revision: c29199c7d8b0b525f31f1d0eda6f9647e481eeef
Merge reported by: Chad Smith
Merged at revision: 86d2fc7f515f70a5117f00baf701a0bed6310b3e
Proposed branch: ~chad.smith/cloud-init:bug/status-fix-done-status
Merge into: cloud-init:master
Diff against target: 140 lines (+32/-8)
2 files modified
cloudinit/cmd/status.py (+4/-1)
cloudinit/cmd/tests/test_status.py (+28/-7)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+337306@code.launchpad.net

Commit message

cli: fix cloud-init status to report running when before result.json

Fix various corner cases for cloud-init status subcommand. Report
'runnning' under the following conditions:
 - No /run/cloud-init/result.json file exists
 - Any stage in status.json is unfinished
 - status.json reports a non-null stage it is in progress on

LP: #1747965

Description of the change

see commit message

to test:
# provide slow running cloud-config

cat >slow.yaml <EOF
#cloud-config
runcmd:
 - sleep 30
EOF

make deb # in this branch
lxc launch ubuntu-daily:bionic myb1
lxc file push cloud-init*deb myb1/.
lxc exec myb1 'dpkg -i /cloud-init*deb'
lxc config set myb1 user.user-data - < slow.yaml
lxc exec myb1 'cloud-init clean --logs --reboot'
lxc exec myb1 bash
cloud-init --wait # should print dots for < 30 seconds and exit w/ done

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:c29199c7d8b0b525f31f1d0eda6f9647e481eeef
https://jenkins.ubuntu.com/server/job/cloud-init-ci/761/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/761/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

I repeated my test case which is similar to the above test, but without a long running runcmd, just booting a lxd container and this branch works as expected.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py
index d7aaee9..ea79a85 100644
--- a/cloudinit/cmd/status.py
+++ b/cloudinit/cmd/status.py
@@ -105,12 +105,12 @@ def _get_status_details(paths):
105105
106 Values are obtained from parsing paths.run_dir/status.json.106 Values are obtained from parsing paths.run_dir/status.json.
107 """107 """
108
109 status = STATUS_ENABLED_NOT_RUN108 status = STATUS_ENABLED_NOT_RUN
110 status_detail = ''109 status_detail = ''
111 status_v1 = {}110 status_v1 = {}
112111
113 status_file = os.path.join(paths.run_dir, 'status.json')112 status_file = os.path.join(paths.run_dir, 'status.json')
113 result_file = os.path.join(paths.run_dir, 'result.json')
114114
115 (is_disabled, reason) = _is_cloudinit_disabled(115 (is_disabled, reason) = _is_cloudinit_disabled(
116 CLOUDINIT_DISABLED_FILE, paths)116 CLOUDINIT_DISABLED_FILE, paths)
@@ -118,12 +118,15 @@ def _get_status_details(paths):
118 status = STATUS_DISABLED118 status = STATUS_DISABLED
119 status_detail = reason119 status_detail = reason
120 if os.path.exists(status_file):120 if os.path.exists(status_file):
121 if not os.path.exists(result_file):
122 status = STATUS_RUNNING
121 status_v1 = load_json(load_file(status_file)).get('v1', {})123 status_v1 = load_json(load_file(status_file)).get('v1', {})
122 errors = []124 errors = []
123 latest_event = 0125 latest_event = 0
124 for key, value in sorted(status_v1.items()):126 for key, value in sorted(status_v1.items()):
125 if key == 'stage':127 if key == 'stage':
126 if value:128 if value:
129 status = STATUS_RUNNING
127 status_detail = 'Running in stage: {0}'.format(value)130 status_detail = 'Running in stage: {0}'.format(value)
128 elif key == 'datasource':131 elif key == 'datasource':
129 status_detail = value132 status_detail = value
diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py
index a7c0a91..4a5a8c0 100644
--- a/cloudinit/cmd/tests/test_status.py
+++ b/cloudinit/cmd/tests/test_status.py
@@ -7,7 +7,7 @@ from textwrap import dedent
77
8from cloudinit.atomic_helper import write_json8from cloudinit.atomic_helper import write_json
9from cloudinit.cmd import status9from cloudinit.cmd import status
10from cloudinit.util import write_file10from cloudinit.util import ensure_file
11from cloudinit.tests.helpers import CiTestCase, wrap_and_call, mock11from cloudinit.tests.helpers import CiTestCase, wrap_and_call, mock
1212
13mypaths = namedtuple('MyPaths', 'run_dir')13mypaths = namedtuple('MyPaths', 'run_dir')
@@ -36,7 +36,7 @@ class TestStatus(CiTestCase):
3636
37 def test__is_cloudinit_disabled_false_on_sysvinit(self):37 def test__is_cloudinit_disabled_false_on_sysvinit(self):
38 '''When not in an environment using systemd, return False.'''38 '''When not in an environment using systemd, return False.'''
39 write_file(self.disable_file, '') # Create the ignored disable file39 ensure_file(self.disable_file) # Create the ignored disable file
40 (is_disabled, reason) = wrap_and_call(40 (is_disabled, reason) = wrap_and_call(
41 'cloudinit.cmd.status',41 'cloudinit.cmd.status',
42 {'uses_systemd': False},42 {'uses_systemd': False},
@@ -47,7 +47,7 @@ class TestStatus(CiTestCase):
4747
48 def test__is_cloudinit_disabled_true_on_disable_file(self):48 def test__is_cloudinit_disabled_true_on_disable_file(self):
49 '''When using systemd and disable_file is present return disabled.'''49 '''When using systemd and disable_file is present return disabled.'''
50 write_file(self.disable_file, '') # Create observed disable file50 ensure_file(self.disable_file) # Create observed disable file
51 (is_disabled, reason) = wrap_and_call(51 (is_disabled, reason) = wrap_and_call(
52 'cloudinit.cmd.status',52 'cloudinit.cmd.status',
53 {'uses_systemd': True},53 {'uses_systemd': True},
@@ -58,7 +58,7 @@ class TestStatus(CiTestCase):
5858
59 def test__is_cloudinit_disabled_false_on_kernel_cmdline_enable(self):59 def test__is_cloudinit_disabled_false_on_kernel_cmdline_enable(self):
60 '''Not disabled when using systemd and enabled via commandline.'''60 '''Not disabled when using systemd and enabled via commandline.'''
61 write_file(self.disable_file, '') # Create ignored disable file61 ensure_file(self.disable_file) # Create ignored disable file
62 (is_disabled, reason) = wrap_and_call(62 (is_disabled, reason) = wrap_and_call(
63 'cloudinit.cmd.status',63 'cloudinit.cmd.status',
64 {'uses_systemd': True,64 {'uses_systemd': True,
@@ -96,7 +96,7 @@ class TestStatus(CiTestCase):
96 def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self):96 def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self):
97 '''Report enabled when systemd generator creates the enabled file.'''97 '''Report enabled when systemd generator creates the enabled file.'''
98 enabled_file = os.path.join(self.paths.run_dir, 'enabled')98 enabled_file = os.path.join(self.paths.run_dir, 'enabled')
99 write_file(enabled_file, '')99 ensure_file(enabled_file)
100 (is_disabled, reason) = wrap_and_call(100 (is_disabled, reason) = wrap_and_call(
101 'cloudinit.cmd.status',101 'cloudinit.cmd.status',
102 {'uses_systemd': True,102 {'uses_systemd': True,
@@ -149,8 +149,25 @@ class TestStatus(CiTestCase):
149 ''')149 ''')
150 self.assertEqual(expected, m_stdout.getvalue())150 self.assertEqual(expected, m_stdout.getvalue())
151151
152 def test_status_returns_running_on_no_results_json(self):
153 '''Report running when status.json exists but result.json does not.'''
154 result_file = self.tmp_path('result.json', self.new_root)
155 write_json(self.status_file, {})
156 self.assertFalse(
157 os.path.exists(result_file), 'Unexpected result.json found')
158 cmdargs = myargs(long=False, wait=False)
159 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
160 retcode = wrap_and_call(
161 'cloudinit.cmd.status',
162 {'_is_cloudinit_disabled': (False, ''),
163 'Init': {'side_effect': self.init_class}},
164 status.handle_status_args, 'ignored', cmdargs)
165 self.assertEqual(0, retcode)
166 self.assertEqual('status: running\n', m_stdout.getvalue())
167
152 def test_status_returns_running(self):168 def test_status_returns_running(self):
153 '''Report running when status exists with an unfinished stage.'''169 '''Report running when status exists with an unfinished stage.'''
170 ensure_file(self.tmp_path('result.json', self.new_root))
154 write_json(self.status_file,171 write_json(self.status_file,
155 {'v1': {'init': {'start': 1, 'finished': None}}})172 {'v1': {'init': {'start': 1, 'finished': None}}})
156 cmdargs = myargs(long=False, wait=False)173 cmdargs = myargs(long=False, wait=False)
@@ -164,10 +181,11 @@ class TestStatus(CiTestCase):
164 self.assertEqual('status: running\n', m_stdout.getvalue())181 self.assertEqual('status: running\n', m_stdout.getvalue())
165182
166 def test_status_returns_done(self):183 def test_status_returns_done(self):
167 '''Reports done when stage is None and all stages are finished.'''184 '''Report done results.json exists no stages are unfinished.'''
185 ensure_file(self.tmp_path('result.json', self.new_root))
168 write_json(186 write_json(
169 self.status_file,187 self.status_file,
170 {'v1': {'stage': None,188 {'v1': {'stage': None, # No current stage running
171 'datasource': (189 'datasource': (
172 'DataSourceNoCloud [seed=/var/.../seed/nocloud-net]'190 'DataSourceNoCloud [seed=/var/.../seed/nocloud-net]'
173 '[dsmode=net]'),191 '[dsmode=net]'),
@@ -187,6 +205,7 @@ class TestStatus(CiTestCase):
187205
188 def test_status_returns_done_long(self):206 def test_status_returns_done_long(self):
189 '''Long format of done status includes datasource info.'''207 '''Long format of done status includes datasource info.'''
208 ensure_file(self.tmp_path('result.json', self.new_root))
190 write_json(209 write_json(
191 self.status_file,210 self.status_file,
192 {'v1': {'stage': None,211 {'v1': {'stage': None,
@@ -303,6 +322,8 @@ class TestStatus(CiTestCase):
303 write_json(self.status_file, running_json)322 write_json(self.status_file, running_json)
304 elif self.sleep_calls == 3:323 elif self.sleep_calls == 3:
305 write_json(self.status_file, done_json)324 write_json(self.status_file, done_json)
325 result_file = self.tmp_path('result.json', self.new_root)
326 ensure_file(result_file)
306327
307 cmdargs = myargs(long=False, wait=True)328 cmdargs = myargs(long=False, wait=True)
308 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:329 with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:

Subscribers

People subscribed via source and target branches