cloud-init-dev team mailing list archive

[Dan Watkins <daniel.watkins@xxxxxxxxxxxxx>]

Dan Watkins has proposed merging ~daniel-thewatkins/cloud-init/+git/cloud-init:clean into cloud-init:master.

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

Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1818571 in cloud-init: "cloud-init clean removes seed directory even when --seed is not specified"
  https://bugs.launchpad.net/cloud-init/+bug/1818571

For more details, see:
https://code.launchpad.net/~daniel-thewatkins/cloud-init/+git/cloud-init/+merge/363936
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~daniel-thewatkins/cloud-init/+git/cloud-init:clean into cloud-init:master.

diff --git a/cloudinit/cmd/clean.py b/cloudinit/cmd/clean.py
index 28ee7b8..30e49de 100644
--- a/cloudinit/cmd/clean.py
+++ b/cloudinit/cmd/clean.py
@@ -62,8 +62,9 @@ def remove_artifacts(remove_logs, remove_seed=False):
 
     if not os.path.isdir(init.paths.cloud_dir):
         return 0  # Artifacts dir already cleaned
+    seed_path = os.path.join(init.paths.cloud_dir, 'seed')
     for path in glob.glob('%s/*' % init.paths.cloud_dir):
-        if path == '%s/seed' % init.paths.cloud_dir and not remove_seed:
+        if path == seed_path and not remove_seed:
             continue
         try:
             if os.path.isdir(path) and not is_link(path):
diff --git a/cloudinit/cmd/tests/test_clean.py b/cloudinit/cmd/tests/test_clean.py
index 15c3294..f092ab3 100644
--- a/cloudinit/cmd/tests/test_clean.py
+++ b/cloudinit/cmd/tests/test_clean.py
@@ -22,7 +22,8 @@ class TestClean(CiTestCase):
         class FakeInit(object):
             cfg = {'def_log_file': self.log1,
                    'output': {'all': '|tee -a {0}'.format(self.log2)}}
-            paths = mypaths(cloud_dir=self.artifact_dir)
+            # Ensure cloud_dir has a trailing slash, to match real behaviour
+            paths = mypaths(cloud_dir='{}/'.format(self.artifact_dir))
 
             def __init__(self, ds_deps):
                 pass