Workaround for sendfile size limit

Fixes an issue when the image source is a local file, the image
will be truncated to 2G and fails deployment due to image corruption.

The issue originates from pysendfile, but local testing shows this
also applies to python3.5 os.sendfile().

Co-Authored-By: Kaifeng Wang <kaifeng.w@gmail.com>
Change-Id: I6263a5221fda0509fbd0b6195c079e5e22800a1b
Story: 2004556
Task: 28316

ironic/common/image_service.py

@@ -34,6 +34,10 @@ from ironic.common.i18n import _
 from ironic.common import utils
 
 IMAGE_CHUNK_SIZE = 1024 * 1024  # 1mb
+# NOTE(kaifeng) Image will be truncated to 2GiB by sendfile,
+# we use a large chunk size here for a better performance
+# while keep the chunk size less than the size limit.
+SENDFILE_CHUNK_SIZE = 1024 * 1024 * 1024  # 1Gb
 LOG = log.getLogger(__name__)
 
 
@@ -211,9 +215,17 @@ class FileImageService(BaseImageService):
                 os.link(source_image_path, dest_image_path)
             else:
                 filesize = os.path.getsize(source_image_path)
+                offset = 0
                 with open(source_image_path, 'rb') as input_img:
-                    sendfile.sendfile(image_file.fileno(), input_img.fileno(),
-                                      0, filesize)
+                    while offset < filesize:
+                        # TODO(kaifeng) Use os.sendfile() and remove sendfile
+                        # dependency when python2 support is dropped.
+                        count = min(SENDFILE_CHUNK_SIZE, filesize - offset)
+                        nbytes_out = sendfile.sendfile(image_file.fileno(),
+                                                       input_img.fileno(),
+                                                       offset,
+                                                       count)
+                        offset += nbytes_out
         except Exception as e:
             raise exception.ImageDownloadFailed(image_href=image_href,
                                                 reason=six.text_type(e))

ironic/tests/unit/common/test_image_service.py

@@ -198,7 +198,7 @@ class FileImageServiceTestCase(base.TestCase):
         remove_mock.assert_called_once_with('file')
         link_mock.assert_called_once_with(self.href_path, 'file')
 
-    @mock.patch.object(sendfile, 'sendfile', autospec=True)
+    @mock.patch.object(sendfile, 'sendfile', return_value=42, autospec=True)
     @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True)
     @mock.patch.object(__builtin__, 'open', autospec=True)
     @mock.patch.object(os, 'access', return_value=False, autospec=True)
@@ -220,6 +220,38 @@ class FileImageServiceTestCase(base.TestCase):
         copy_mock.assert_called_once_with(file_mock.fileno(),
                                           input_mock.__enter__().fileno(),
                                           0, 42)
+
+    @mock.patch.object(sendfile, 'sendfile', autospec=True)
+    @mock.patch.object(os.path, 'getsize', return_value=42, autospec=True)
+    @mock.patch.object(__builtin__, 'open', autospec=True)
+    @mock.patch.object(os, 'access', return_value=False, autospec=True)
+    @mock.patch.object(os, 'stat', autospec=True)
+    @mock.patch.object(image_service.FileImageService, 'validate_href',
+                       autospec=True)
+    def test_download_copy_segmented(self, _validate_mock, stat_mock,
+                                     access_mock, open_mock, size_mock,
+                                     copy_mock):
+        # Fake a 3G + 1k image
+        chunk_size = image_service.SENDFILE_CHUNK_SIZE
+        fake_image_size = chunk_size * 3 + 1024
+        fake_chunk_seq = [chunk_size, chunk_size, chunk_size, 1024]
+        _validate_mock.return_value = self.href_path
+        stat_mock.return_value.st_dev = 'dev1'
+        file_mock = mock.MagicMock(spec=file)
+        file_mock.name = 'file'
+        input_mock = mock.MagicMock(spec=file)
+        open_mock.return_value = input_mock
+        size_mock.return_value = fake_image_size
+        copy_mock.side_effect = fake_chunk_seq
+        self.service.download(self.href, file_mock)
+        _validate_mock.assert_called_once_with(mock.ANY, self.href)
+        self.assertEqual(2, stat_mock.call_count)
+        access_mock.assert_called_once_with(self.href_path, os.R_OK | os.W_OK)
+        copy_calls = [mock.call(file_mock.fileno(),
+                                input_mock.__enter__().fileno(),
+                                chunk_size * i,
+                                fake_chunk_seq[i]) for i in range(4)]
+        copy_mock.assert_has_calls(copy_calls)
         size_mock.assert_called_once_with(self.href_path)
 
     @mock.patch.object(os, 'remove', side_effect=OSError, autospec=True)

releasenotes/notes/fix-sendfile-size-cap-d9966a96e2d7db51.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue when the image source is a local file, the image will be
+    truncated to 2G and fails deployment due to image corruption.
+