From aa8b0e98b6deeec45c11e60dd522f4fa12a81046 Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Thu, 1 Aug 2019 12:48:06 -0600 Subject: [PATCH] Close resources correctly Under python3 not closing file handles or network resources correctly results in warnings being shown to the end user. This change switches the resource usage to properly close when we're done. Change-Id: Iaf08f7d7b919b154797cf8f10663f349424cd6a2 Related-Bug: #1837393 (cherry picked from commit a06c818e4d926a2d08615f882cd824e2dcf8854e) --- tripleoclient/tests/fakes.py | 16 +-- .../overcloud_image/test_overcloud_image.py | 41 ++++---- tripleoclient/v1/overcloud_image.py | 98 ++++++++++--------- tripleoclient/v1/overcloud_node.py | 1 + tripleoclient/v1/overcloud_plan.py | 1 + 5 files changed, 85 insertions(+), 72 deletions(-) diff --git a/tripleoclient/tests/fakes.py b/tripleoclient/tests/fakes.py index f63cc9245..55495d90c 100644 --- a/tripleoclient/tests/fakes.py +++ b/tripleoclient/tests/fakes.py @@ -43,7 +43,15 @@ class FakeClientManager(object): self.workflow_engine = mock.Mock() -class FakeWebSocket(object): +class FakeHandle(object): + def __enter__(self): + return self + + def __exit__(self, *args): + return + + +class FakeWebSocket(FakeHandle): def wait_for_messages(self, timeout=None): yield { @@ -51,12 +59,6 @@ class FakeWebSocket(object): 'status': 'SUCCESS', } - def __enter__(self): - return self - - def __exit__(self, *args): - return - class FakeClientWrapper(object): diff --git a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py index eef97c338..a22dc2fb9 100644 --- a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py +++ b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py @@ -18,6 +18,7 @@ import os from osc_lib import exceptions import tripleo_common.arch +from tripleoclient.tests.fakes import FakeHandle from tripleoclient.tests.v1.test_plugin import TestPluginV1 from tripleoclient.v1 import overcloud_image @@ -127,7 +128,9 @@ class TestUploadOvercloudImage(TestPluginV1): properties={'kernel_id': 10, 'ramdisk_id': 10, 'hw_architecture': self._arch}, created_at='2015-07-31T14:37:22.000000')) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') + self._file_handle = FakeHandle() + self.cmd._read_image_file_pointer = mock.Mock( + return_value=self._file_handle) self.cmd._check_file_exists = mock.Mock(return_value=True) @mock.patch('osc_lib.utils.find_resource') @@ -290,29 +293,29 @@ class TestUploadOvercloudImage(TestPluginV1): ) self.app.client_manager.image.images.create.assert_has_calls([ mock.call(properties={'hw_architecture': self._arch}, - data=b'IMGDATA', + data=self._file_handle, name='overcloud-full-vmlinuz', disk_format='aki', is_public=True), mock.call(properties={'hw_architecture': self._arch}, - data=b'IMGDATA', + data=self._file_handle, name='overcloud-full-initrd', disk_format='ari', is_public=True), mock.call(properties={'kernel_id': 10, 'ramdisk_id': 10, 'hw_architecture': self._arch}, name='overcloud-full', - data=b'IMGDATA', + data=self._file_handle, container_format='bare', disk_format='qcow2', is_public=True), mock.call(properties={'hw_architecture': self._arch}, - data=b'IMGDATA', + data=self._file_handle, name='bm-deploy-kernel', disk_format='aki', is_public=True), mock.call(properties={'hw_architecture': self._arch}, - data=b'IMGDATA', + data=self._file_handle, name='bm-deploy-ramdisk', disk_format='ari', is_public=True) @@ -340,29 +343,29 @@ class TestUploadOvercloudImage(TestPluginV1): ) self.app.client_manager.image.images.create.assert_has_calls([ mock.call(properties={'hw_architecture': 'x86_64'}, - data=b'IMGDATA', + data=self._file_handle, name='x86_64-overcloud-full-vmlinuz', disk_format='aki', is_public=True), mock.call(properties={'hw_architecture': 'x86_64'}, - data=b'IMGDATA', + data=self._file_handle, name='x86_64-overcloud-full-initrd', disk_format='ari', is_public=True), mock.call(properties={'hw_architecture': 'x86_64', 'kernel_id': 10, 'ramdisk_id': 10}, name='x86_64-overcloud-full', - data=b'IMGDATA', + data=self._file_handle, container_format='bare', disk_format='qcow2', is_public=True), mock.call(properties={'hw_architecture': 'x86_64'}, - data=b'IMGDATA', + data=self._file_handle, name='x86_64-bm-deploy-kernel', disk_format='aki', is_public=True), mock.call(properties={'hw_architecture': 'x86_64'}, - data=b'IMGDATA', + data=self._file_handle, name='x86_64-bm-deploy-ramdisk', disk_format='ari', is_public=True) @@ -479,7 +482,9 @@ class TestUploadOvercloudImageFull(TestPluginV1): mock.Mock(id=10, name='imgname', properties={'hw_architecture': self._arch}, created_at='2015-07-31T14:37:22.000000')) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') + self._file_handle = FakeHandle() + self.cmd._read_image_file_pointer = mock.Mock( + return_value=self._file_handle) self.cmd._check_file_exists = mock.Mock(return_value=True) @mock.patch('os.path.isfile', autospec=True) @@ -670,7 +675,9 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): # call so this way we always create() and get() the same mocked "image" self.app.client_manager.image.images.create.side_effect = self.images self.app.client_manager.image.images.get.side_effect = self.images - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') + self._file_handle = FakeHandle() + self.cmd._read_image_file_pointer = mock.Mock( + return_value=self._file_handle) self.cmd._check_file_exists = mock.Mock(return_value=True) @mock.patch('os.path.isfile', autospec=True) @@ -859,7 +866,9 @@ class TestUploadOnlyExisting(TestPluginV1): mock.Mock(id=10, name='imgname', properties={}, created_at='2015-07-31T14:37:22.000000')) self.cmd._check_file_exists = mock.Mock() - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') + self._file_handle = FakeHandle() + self.cmd._read_image_file_pointer = mock.Mock( + return_value=self._file_handle) @mock.patch('subprocess.check_call', autospec=True) @mock.patch('os.path.isfile', autospec=True) @@ -867,7 +876,6 @@ class TestUploadOnlyExisting(TestPluginV1): self, mock_isfile_call, mock_subprocess_call): self.cmd._image_changed = mock.Mock(return_value=True) self.cmd._image_try_update = mock.Mock(return_value=None) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') parsed_args = self.check_parser( self.cmd, ['--whole-disk', '--image-type=ironic-python-agent'], []) @@ -893,7 +901,6 @@ class TestUploadOnlyExisting(TestPluginV1): self, mock_isfile_call, mock_subprocess_call): self.cmd._image_changed = mock.Mock(return_value=True) self.cmd._image_try_update = mock.Mock(return_value=None) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') parsed_args = self.check_parser( self.cmd, ['--whole-disk', '--image-type=os'], []) @@ -917,7 +924,6 @@ class TestUploadOnlyExisting(TestPluginV1): self, mock_isfile_call, mock_subprocess_call): self.cmd._image_changed = mock.Mock(return_value=True) self.cmd._image_try_update = mock.Mock(return_value=None) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') parsed_args = self.check_parser( self.cmd, ['--image-type=ironic-python-agent'], []) @@ -943,7 +949,6 @@ class TestUploadOnlyExisting(TestPluginV1): self, mock_isfile_call, mock_subprocess_call): self.cmd._image_changed = mock.Mock(return_value=True) self.cmd._image_try_update = mock.Mock(return_value=None) - self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') parsed_args = self.check_parser( self.cmd, ['--image-type=os'], []) diff --git a/tripleoclient/v1/overcloud_image.py b/tripleoclient/v1/overcloud_image.py index 2079ef1cb..7ba92ab78 100644 --- a/tripleoclient/v1/overcloud_image.py +++ b/tripleoclient/v1/overcloud_image.py @@ -372,17 +372,18 @@ class UploadOvercloudImage(command.Command): oc_vmlinuz_file = os.path.join(parsed_args.image_path, image_name + oc_vmlinuz_extension) - kernel = (self._image_try_update(oc_vmlinuz_name, - oc_vmlinuz_file, - parsed_args) or - glance_client_adaptor.upload_image( - name=oc_vmlinuz_name, - is_public=True, - disk_format='aki', - properties=properties, - data=self._read_image_file_pointer( - parsed_args.image_path, oc_vmlinuz_file) - )) + with self._read_image_file_pointer( + parsed_args.image_path, oc_vmlinuz_file) as data: + kernel = (self._image_try_update(oc_vmlinuz_name, + oc_vmlinuz_file, + parsed_args) or + glance_client_adaptor.upload_image( + name=oc_vmlinuz_name, + is_public=True, + disk_format='aki', + properties=properties, + data=data + )) (oc_initrd_name, oc_initrd_extension) = plugin_utils.overcloud_ramdisk( @@ -390,17 +391,18 @@ class UploadOvercloudImage(command.Command): oc_initrd_file = os.path.join(parsed_args.image_path, image_name + oc_initrd_extension) - ramdisk = (self._image_try_update(oc_initrd_name, - oc_initrd_file, - parsed_args) or - glance_client_adaptor.upload_image( - name=oc_initrd_name, - is_public=True, - disk_format='ari', - properties=properties, - data=self._read_image_file_pointer( - parsed_args.image_path, oc_initrd_file) - )) + with self._read_image_file_pointer( + parsed_args.image_path, oc_initrd_file) as data: + ramdisk = (self._image_try_update(oc_initrd_name, + oc_initrd_file, + parsed_args) or + glance_client_adaptor.upload_image( + name=oc_initrd_name, + is_public=True, + disk_format='ari', + properties=properties, + data=data + )) (oc_name, oc_extension) = plugin_utils.overcloud_image( @@ -408,20 +410,21 @@ class UploadOvercloudImage(command.Command): oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) - overcloud_image = (self._image_try_update(oc_name, oc_file, - parsed_args) or - glance_client_adaptor.upload_image( - name=oc_name, - is_public=True, - disk_format='qcow2', - container_format='bare', - properties=dict( - {'kernel_id': kernel.id, - 'ramdisk_id': ramdisk.id}, - **properties), - data=self._read_image_file_pointer( - parsed_args.image_path, oc_file) - )) + with self._read_image_file_pointer( + parsed_args.image_path, oc_file) as data: + overcloud_image = (self._image_try_update(oc_name, oc_file, + parsed_args) or + glance_client_adaptor.upload_image( + name=oc_name, + is_public=True, + disk_format='qcow2', + container_format='bare', + properties=dict( + {'kernel_id': kernel.id, + 'ramdisk_id': ramdisk.id}, + **properties), + data=data + )) img_kernel_id = glance_client_adaptor.get_image_property( overcloud_image, 'kernel_id') @@ -442,17 +445,18 @@ class UploadOvercloudImage(command.Command): oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) - overcloud_image = (self._image_try_update(oc_name, oc_file, - parsed_args) or - glance_client_adaptor.upload_image( - name=oc_name, - is_public=True, - disk_format='qcow2', - container_format='bare', - properties=properties, - data=self._read_image_file_pointer( - parsed_args.image_path, oc_file) - )) + with self._read_image_file_pointer( + parsed_args.image_path, oc_file) as data: + overcloud_image = (self._image_try_update(oc_name, oc_file, + parsed_args) or + glance_client_adaptor.upload_image( + name=oc_name, + is_public=True, + disk_format='qcow2', + container_format='bare', + properties=properties, + data=data + )) self.log.debug("uploading bm images to glance") diff --git a/tripleoclient/v1/overcloud_node.py b/tripleoclient/v1/overcloud_node.py index 457661071..2164a1d8c 100644 --- a/tripleoclient/v1/overcloud_node.py +++ b/tripleoclient/v1/overcloud_node.py @@ -262,6 +262,7 @@ class ImportNode(command.Command): self.log.debug("take_action(%s)" % parsed_args) nodes_config = oooutils.parse_env_file(parsed_args.env_file) + parsed_args.env_file.close() if parsed_args.validate_only: return baremetal.validate_nodes(self.app.client_manager, diff --git a/tripleoclient/v1/overcloud_plan.py b/tripleoclient/v1/overcloud_plan.py index d69b309d4..8b29376fe 100644 --- a/tripleoclient/v1/overcloud_plan.py +++ b/tripleoclient/v1/overcloud_plan.py @@ -202,6 +202,7 @@ class ExportPlan(command.Command): ) f = request.urlopen(tempurl) tarball_contents = f.read() + f.close() with open(outfile, 'wb') as f: f.write(tarball_contents)