← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~smoser/cloud-init:fix/cii-kvmimage-preserve-original into cloud-init:master

 

What's the use case for the "destroy an image without breaking a snapshot" that introduces the copy of the image.

Some more comments inline as well.

Diff comments:

> diff --git a/tests/cloud_tests/images/nocloudkvm.py b/tests/cloud_tests/images/nocloudkvm.py
> index 1e7962c..8678b07 100644
> --- a/tests/cloud_tests/images/nocloudkvm.py
> +++ b/tests/cloud_tests/images/nocloudkvm.py
> @@ -21,7 +25,13 @@ class NoCloudKVMImage(base.Image):
>          @param img_path: path to the image
>          """
>          self.modified = False
> -        self._img_path = img_path
> +        self._workd = tempfile.mkdtemp(prefix='NoCloudKVMImage')

Every time we init we're going to redo this operation (which I know is light weight w.r.t an empty qcow2 file with backing)
But in the case were we may be iterating on validating the results (similar to the curtin REUSE_TOPDIR) where we don't want
to modify the base/collect data, just how we're comparing it, it would be good to have a check to see if this is needed.

> +        self._orig_img_path = orig_img_path
> +        self._img_path = os.path.join(self._workd,
> +                                      os.path.basename(self._orig_img_path))
> +
> +        c_util.subp(['qemu-img', 'create', '-f', 'qcow2',
> +                    '-b', orig_img_path, self._img_path])
>  
>          super(NoCloudKVMImage, self).__init__(platform, config)
>  
> diff --git a/tests/cloud_tests/platforms/nocloudkvm.py b/tests/cloud_tests/platforms/nocloudkvm.py
> index f1f8187..cf55561 100644
> --- a/tests/cloud_tests/platforms/nocloudkvm.py
> +++ b/tests/cloud_tests/platforms/nocloudkvm.py
> @@ -58,16 +58,14 @@ class NoCloudKVMPlatform(base.Platform):
>          if len(images) != 1:
>              raise Exception('No unique images found')
>  
> -        image = nocloud_kvm_image.NoCloudKVMImage(self, img_conf, images[0])
> -        if img_conf.get('override_templates', False):
> -            image.update_templates(self.config.get('template_overrides', {}),
> -                                   self.config.get('template_files', {}))
> +        image = nocloud_kvm_image.NoCloudKVMImage(self, img_conf,
> +                                                  sorted(images)[0])

A comment here w.r.t what the sorting is getting us.  Especially in light of the

if len(images) != 1:
  raise Exception 

above it which should ensure we only have a single image here

>          return image
>  
> -    def create_image(self, properties, config, features,
> -                     src_img_path, image_desc=None, use_desc=None,
> -                     user_data=None, meta_data=None):
> -        """Create an image
> +    def create_instance(self, properties, config, features,
> +                        src_img_path, image_desc=None, use_desc=None,
> +                        user_data=None, meta_data=None):
> +        """Create an instance
>  
>          @param src_img_path: image path to launch from
>          @param properties: image properties


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/334147
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/cii-kvmimage-preserve-original into cloud-init:master.


References