zuul
/
nodepool
Code Issues Proposed changes

builder: Remove optional extension from DibImageFile 

A DibImageFile represents one dib image-file on disk, so the extension
is required (see prior change
I214581ad80b7740e7ca749b574672d2c33b92474 where we modified callers
who were using this interface).

This fixes a bug by removing code; the pathlib with_suffix replacement
is not safe for image names with a period in them; consider

>>> pathlib.Path('image-v1.2-foo').with_suffix('.vhd')
PosixPath('image-v1.vhd')

We can now simply unconditionally append the extension in
DibImageFile.to_path().

Change-Id: I1bc812ddffacbcc414b8f7f372d9fca78bd87292
This commit is contained in:
Ian Wienand 2021-12-21 16:19:45 +11:00
parent 5704ce75fc
commit 0f257982f4
2 changed files with 6 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
nodepool/builder.py
View File

@ -56,7 +56,7 @@ class DibImageFile(object):
a unique ID, but can be available in multiple formats (with different
extensions).
'''
def __init__(self, image_id, extension=None):
def __init__(self, image_id, extension):
self.image_id = image_id
self.extension = extension
self.md5 = None
@ -79,14 +79,8 @@ class DibImageFile(object):
images.append(image)
return images
def to_path(self, images_dir, with_extension=True):
my_path = Path(images_dir) / self.image_id
if with_extension:
if self.extension is None:
raise exceptions.BuilderError(
'Cannot specify image extension of None'
)
my_path = my_path.with_suffix('.' + self.extension)
def to_path(self, images_dir):
my_path = Path(images_dir) / f'{self.image_id}.{self.extension}'
# Path.with_suffix() will replace an existing suffix, so we create
# new Path objects from strings for the checksum files.
@ -260,7 +254,7 @@ class CleanupWorker(BaseWorker):
manifest_dir = None
for f in files:
filename = f.to_path(images_dir, True)
filename = f.to_path(images_dir)
if not manifest_dir:
path, ext = filename.rsplit('.', 1)
manifest_dir = path + ".d"
@ -1071,7 +1065,7 @@ class UploadWorker(BaseWorker):
self.log.debug("Found image file of type %s for image id: %s" %
(image.extension, image.image_id))
filename = image.to_path(self._config.images_dir, with_extension=True)
filename = image.to_path(self._config.images_dir)
ext_image_name = provider.image_name_format.format(
image_name=image_name, timestamp=str(timestamp)

7
nodepool/tests/unit/test_builder.py
View File

@ -19,7 +19,7 @@ import fixtures
import mock
import time
from nodepool import builder, exceptions, tests
from nodepool import builder, tests
from nodepool.driver.fake import provider as fakeprovider
from nodepool import zk
from nodepool.config import Config
@ -61,11 +61,6 @@ class TestNodepoolBuilderDibImage(tests.BaseTestCase):
'/imagedir/myid1234.qcow2')
self.assertEqual(image.to_path('/imagedir/'),
'/imagedir/myid1234.qcow2')
self.assertEqual(image.to_path('/imagedir/', False),
'/imagedir/myid1234')
image = builder.DibImageFile('myid1234')
self.assertRaises(exceptions.BuilderError, image.to_path, '/imagedir/')
class TestNodepoolBuilderImageInheritance(tests.BaseTestCase):