openstack/metalsmith
Code Issues Proposed changes

Stop requiring root size for whole disk images

Browse Source 
This requirement has been fixed in newer versions of ironic.

Change-Id: I4460755ee66b4aa0b8a651b6bd142c769d669ee2
This commit is contained in:
Dmitry Tantsur 2019-09-26 13:57:18 +02:00
parent 52b777f98e
commit 2d801e2526
6 changed files with 89 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
metalsmith/_provisioner.py
View File

@ -388,7 +388,7 @@ class Provisioner(object):
try:
root_size_gb = _utils.get_root_disk(root_size_gb, node)
image._validate(self.connection)
image._validate(self.connection, root_size_gb)
nics.validate()
@ -405,7 +405,8 @@ class Provisioner(object):
capabilities['boot_option'] = 'netboot' if netboot else 'local'
instance_info = self._clean_instance_info(node.instance_info)
instance_info['root_gb'] = root_size_gb
if root_size_gb is not None:
instance_info['root_gb'] = root_size_gb
instance_info['capabilities'] = capabilities
if hostname:
instance_info['display_name'] = hostname

6
metalsmith/_utils.py
View File

@ -56,9 +56,9 @@ def get_root_disk(root_size_gb, node):
try:
assert int(node.properties['local_gb']) > 0
except KeyError:
raise exceptions.UnknownRootDiskSize(
'No local_gb for node %s and no root partition size '
'specified' % log_res(node))
LOG.debug('No local_gb for node %s and no root partition size '
'specified', log_res(node))
return
except (TypeError, ValueError, AssertionError):
raise exceptions.UnknownRootDiskSize(
'The local_gb for node %(node)s is invalid: '

26
metalsmith/sources.py
View File

@ -34,7 +34,7 @@ LOG = logging.getLogger(__name__)
@six.add_metaclass(abc.ABCMeta)
class _Source(object):
def _validate(self, connection):
def _validate(self, connection, root_size_gb):
"""Validate the source."""
@abc.abstractmethod
@ -53,7 +53,7 @@ class GlanceImage(_Source):
self.image = image
self._image_obj = None
def _validate(self, connection):
def _validate(self, connection, root_size_gb):
if self._image_obj is not None:
return
try:
@ -64,8 +64,13 @@ class GlanceImage(_Source):
'Cannot find image %(image)s: %(error)s' %
{'image': self.image, 'error': exc})
if (root_size_gb is None and
any(getattr(self._image_obj, x, None) is not None
for x in ('kernel_id', 'ramdisk_id'))):
raise exceptions.UnknownRootDiskSize(
'Partition images require root partition size')
def _node_updates(self, connection):
self._validate(connection)
LOG.debug('Image: %s', self._image_obj)
updates = {
@ -108,7 +113,7 @@ class HttpWholeDiskImage(_Source):
self.checksum = checksum
self.checksum_url = checksum_url
def _validate(self, connection):
def _validate(self, connection, root_size_gb):
# TODO(dtantsur): should we validate image URLs here? Ironic will do it
# as well, and images do not have to be accessible from where
# metalsmith is running.
@ -140,7 +145,6 @@ class HttpWholeDiskImage(_Source):
{'fname': fname, 'url': self.checksum_url})
def _node_updates(self, connection):
self._validate(connection)
LOG.debug('Image: %(image)s, checksum %(checksum)s',
{'image': self.url, 'checksum': self.checksum})
return {
@ -170,6 +174,12 @@ class HttpPartitionImage(HttpWholeDiskImage):
self.kernel_url = kernel_url
self.ramdisk_url = ramdisk_url
def _validate(self, connection, root_size_gb):
super(HttpPartitionImage, self)._validate(connection, root_size_gb)
if root_size_gb is None:
raise exceptions.UnknownRootDiskSize(
'Partition images require root partition size')
def _node_updates(self, connection):
updates = super(HttpPartitionImage, self)._node_updates(connection)
updates['kernel'] = self.kernel_url
@ -237,6 +247,12 @@ class FilePartitionImage(FileWholeDiskImage):
self.kernel_location = kernel_location
self.ramdisk_location = ramdisk_location
def _validate(self, connection, root_size_gb):
super(FilePartitionImage, self)._validate(connection, root_size_gb)
if root_size_gb is None:
raise exceptions.UnknownRootDiskSize(
'Partition images require root partition size')
def _node_updates(self, connection):
updates = super(FilePartitionImage, self)._node_updates(connection)
updates['kernel'] = self.kernel_location

57
metalsmith/test/test_sources.py
View File

@ -13,18 +13,49 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import mock
import testtools
from metalsmith import exceptions
from metalsmith import sources
class TestDetect(testtools.TestCase):
def test_glance(self):
def test_glance_whole_disk(self):
source = sources.detect('foobar')
self.assertIsInstance(source, sources.GlanceImage)
self.assertEqual(source.image, 'foobar')
conn = mock.Mock(spec=['image'])
conn.image.find_image.return_value = mock.Mock(
id=42, kernel_id=None, ramdisk_id=None)
source._validate(conn, None)
self.assertEqual({'image_source': 42}, source._node_updates(conn))
def test_glance_partition(self):
source = sources.detect('foobar')
self.assertIsInstance(source, sources.GlanceImage)
self.assertEqual(source.image, 'foobar')
conn = mock.Mock(spec=['image'])
conn.image.find_image.return_value = mock.Mock(
id=42, kernel_id=1, ramdisk_id=2)
source._validate(conn, 9)
self.assertEqual({'image_source': 42, 'kernel': 1, 'ramdisk': 2},
source._node_updates(conn))
def test_glance_partition_missing_root(self):
source = sources.detect('foobar')
self.assertIsInstance(source, sources.GlanceImage)
self.assertEqual(source.image, 'foobar')
conn = mock.Mock(spec=['image'])
conn.image.find_image.return_value = mock.Mock(
id=42, kernel_id=1, ramdisk_id=2)
self.assertRaises(exceptions.UnknownRootDiskSize,
source._validate, conn, None)
def test_glance_invalid_arguments(self):
for kwargs in [{'kernel': 'foo'},
{'ramdisk': 'foo'},
@ -43,6 +74,8 @@ class TestDetect(testtools.TestCase):
self.assertEqual(source.location, 'file:///image')
self.assertEqual(source.checksum, 'abcd')
source._validate(mock.Mock(), None)
def test_file_partition_disk(self):
source = sources.detect('file:///image', checksum='abcd',
kernel='file:///kernel',
@ -53,6 +86,15 @@ class TestDetect(testtools.TestCase):
self.assertEqual(source.kernel_location, 'file:///kernel')
self.assertEqual(source.ramdisk_location, 'file:///ramdisk')
source._validate(mock.Mock(), 9)
def test_file_partition_disk_missing_root(self):
source = sources.detect('file:///image', checksum='abcd',
kernel='file:///kernel',
ramdisk='file:///ramdisk')
self.assertRaises(exceptions.UnknownRootDiskSize,
source._validate, mock.Mock(), None)
def test_file_partition_inconsistency(self):
for kwargs in [{'kernel': 'foo'},
{'ramdisk': 'foo'},
@ -69,12 +111,16 @@ class TestDetect(testtools.TestCase):
self.assertEqual(source.url, 'http:///image')
self.assertEqual(source.checksum, 'abcd')
source._validate(mock.Mock(), None)
def test_https_whole_disk(self):
source = sources.detect('https:///image', checksum='abcd')
self.assertIs(source.__class__, sources.HttpWholeDiskImage)
self.assertEqual(source.url, 'https:///image')
self.assertEqual(source.checksum, 'abcd')
source._validate(mock.Mock(), None)
def test_https_whole_disk_checksum(self):
source = sources.detect('https:///image',
checksum='https://checksum')
@ -92,6 +138,15 @@ class TestDetect(testtools.TestCase):
self.assertEqual(source.kernel_url, 'http:///kernel')
self.assertEqual(source.ramdisk_url, 'http:///ramdisk')
source._validate(mock.Mock(), 9)
def test_http_partition_disk_missing_root(self):
source = sources.detect('http:///image', checksum='abcd',
kernel='http:///kernel',
ramdisk='http:///ramdisk')
self.assertRaises(exceptions.UnknownRootDiskSize,
source._validate, mock.Mock(), None)
def test_https_partition_disk(self):
source = sources.detect('https:///image', checksum='abcd',
# Can mix HTTP and HTTPs

1
playbooks/integration/run.yaml
View File

@ -14,6 +14,7 @@
vars:
metalsmith_image: "{{ metalsmith_whole_disk_image }}"
metalsmith_image_checksum: "{{ metalsmith_whole_disk_checksum | default('') }}"
metalsmith_root_size:
# NOTE(dtantsur): cannot specify swap with whole disk images
metalsmith_swap_size:

5
releasenotes/notes/whole-disk-root-gb-bd8ee3600de9ec8d.yaml Normal file
View File

@ -0,0 +1,5 @@
---
fixes:
- |
No longer requires root size for whole disk images. This requirement has
been removed from ironic.