Merge ~oddbloke/cloud-init/+git/cloud-init:clean into cloud-init:master

Proposed by Dan Watkins
Status: Merged
Approved by: Dan Watkins
Approved revision: 5d08d32fad4673515307b811b41e454368a052ab
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~oddbloke/cloud-init/+git/cloud-init:clean
Merge into: cloud-init:master
Diff against target: 29 lines (+4/-2)
2 files modified
cloudinit/cmd/clean.py (+2/-1)
cloudinit/cmd/tests/test_clean.py (+2/-1)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Ryan Harper Approve
Review via email: mp+363936@code.launchpad.net

Commit message

clean: correctly determine the path for excluding seed directory

Previously, init.paths.cloud_dir has a trailing slash, which meant that
"/var/lib/cloud//seed" was being compared to "/var/lib/cloud/seed" and
(of course), never matching.

In this commit, switch to using os.path.join to avoid this case (and
update the tests to catch it in future).

LP: #1818571

To post a comment you must log in.
Revision history for this message
Ryan Harper (raharper) wrote :

I manually applied your change to test_clean and it does trip the check that seed_dir is not removed.

======================================================================
FAIL: cloudinit.cmd.tests.test_clean.TestClean.test_remove_artifacts_removes_artifacts_skipping_seed
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rharper/work/git/cloud-init/cloudinit/cmd/tests/test_clean.py", line 99, in test_remove_artifacts_removes_artifacts_skipping_seed
    'Missing {0} dir'.format(expected_dir))
AssertionError: False is not true : Missing /tmp/ci-TestClean.f5keq5q_/artifacts/seed dir
-------------------- >> begin captured logging << --------------------
cloudinit.util: DEBUG: Testing if a link exists for /tmp/ci-TestClean.f5keq5q_/artifacts/seed
cloudinit.util: DEBUG: Recursively deleting /tmp/ci-TestClean.f5keq5q_/artifacts/seed
cloudinit.util: DEBUG: Testing if a link exists for /tmp/ci-TestClean.f5keq5q_/artifacts/dir2
cloudinit.util: DEBUG: Recursively deleting /tmp/ci-TestClean.f5keq5q_/artifacts/dir2
cloudinit.util: DEBUG: Testing if a link exists for /tmp/ci-TestClean.f5keq5q_/artifacts/dir1
cloudinit.util: DEBUG: Recursively deleting /tmp/ci-TestClean.f5keq5q_/artifacts/dir1
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 8 tests in 0.012s

Approved if CI says yes as well.

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

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

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/cmd/clean.py b/cloudinit/cmd/clean.py
2index 28ee7b8..30e49de 100644
3--- a/cloudinit/cmd/clean.py
4+++ b/cloudinit/cmd/clean.py
5@@ -62,8 +62,9 @@ def remove_artifacts(remove_logs, remove_seed=False):
6
7 if not os.path.isdir(init.paths.cloud_dir):
8 return 0 # Artifacts dir already cleaned
9+ seed_path = os.path.join(init.paths.cloud_dir, 'seed')
10 for path in glob.glob('%s/*' % init.paths.cloud_dir):
11- if path == '%s/seed' % init.paths.cloud_dir and not remove_seed:
12+ if path == seed_path and not remove_seed:
13 continue
14 try:
15 if os.path.isdir(path) and not is_link(path):
16diff --git a/cloudinit/cmd/tests/test_clean.py b/cloudinit/cmd/tests/test_clean.py
17index 15c3294..f092ab3 100644
18--- a/cloudinit/cmd/tests/test_clean.py
19+++ b/cloudinit/cmd/tests/test_clean.py
20@@ -22,7 +22,8 @@ class TestClean(CiTestCase):
21 class FakeInit(object):
22 cfg = {'def_log_file': self.log1,
23 'output': {'all': '|tee -a {0}'.format(self.log2)}}
24- paths = mypaths(cloud_dir=self.artifact_dir)
25+ # Ensure cloud_dir has a trailing slash, to match real behaviour
26+ paths = mypaths(cloud_dir='{}/'.format(self.artifact_dir))
27
28 def __init__(self, ds_deps):
29 pass

Subscribers

People subscribed via source and target branches