zuul/nodepool
Code Issues Proposed changes

Allow specifying diskimage metadata/tags

Browse Source 
For drivers that support tagging/metadata (openstack, aws, azure),
Add or enhance support for supplying tags for uploaded diskimages.

This allows users to set metadata on the global diskimage object
which will then be used as default values for metadata on the
provider diskimage values.  The resulting merged dictionary forms
the basis of metadata to be associated with the uploaded image.

The changes needed to reconcile this for the three drivers mentioned
above are:

All: the diskimages[].meta key is added to supply the default values
for provider metadata.

OpenStack: provider diskimage metadata is already supported using
providers[].diskimages[].meta, so no further changes are needed.

AWS, Azure: provider diskimage tags are added using the key
providers[].diskimages[].tags since these providers already use
the "tags" nomenclature for instances.

This results in the somewhat incongruous situation where we have
diskimage "metadata" being combined with provider "tags", but it's
either that or have images with "metadata" while we have instances
with "tags", both of which are "tags" in EC2.  The chosen approach
has consistency within the driver.

Change-Id: I30aadadf022af3aa97772011cda8dbae0113a3d8
This commit is contained in:
James E. Blair 2022-08-08 10:59:10 -07:00
parent 632034700b
commit 916d62a374
16 changed files with 86 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
doc/source/aws.rst
View File

@ -396,6 +396,15 @@ Selecting the ``aws`` driver adds the following options to the
omitted, the volume size reported for the imported snapshot
will be used.
.. attr:: tags
:type: dict
:default: None
A dictionary of tags to add to uploaded images. This will be
merged with any existing metadata from the global `diskimage`
configuration for this image. Avoid the use of `nodepool_`
as a key prefix since Nodepool uses this for internal values.
.. attr:: pools
:type: list

9
doc/source/azure.rst
View File

@ -475,6 +475,15 @@ section of the configuration.
interpreter on Ansible >=2.8, and default to
``/usr/bin/python2`` for earlier versions.
.. attr:: tags
:type: dict
:default: None
A dictionary of tags to add to uploaded images. This will be
merged with any existing metadata from the global `diskimage`
configuration for this image. Avoid the use of `nodepool_`
as a key prefix since Nodepool uses this for internal values.
.. attr:: pools
:type: list

9
doc/source/configuration.rst
View File

@ -441,6 +441,15 @@ Options
``disk-image-create`` are not considered an API and
may change.
.. attr:: metadata
:type: dict
This provides default values for the provider-specific
`metadata` or `tags` values which can be set for diskimage
uploads to specific providers. This is a dictionary of
arbitrary key/value pairs. Avoid the use of `nodepool_` as a
key prefix since Nodepool uses this for internal values.
.. attr:: providers
:type: list

3
nodepool/builder.py
View File

@ -1118,7 +1118,8 @@ class UploadWorker(BaseWorker):
"Could not find matching provider image for %s" % image_name
)
meta = provider_image.meta.copy()
meta = self._config.diskimages[image_name].meta.copy()
meta.update(provider_image.meta)
meta['nodepool_build_id'] = build_id
meta['nodepool_upload_id'] = upload_id

1
nodepool/cmd/config_validator.py
View File

@ -49,6 +49,7 @@ class ConfigValidator:
'username': str,
'python-path': str,
'build-timeout': int,
'metadata': dict,
}
webapp = {

5
nodepool/config.py
View File

@ -302,6 +302,7 @@ class DiskImage(ConfigValue):
self.rebuild_age = self.REBUILD_AGE
self.release = ''
self.username = 'zuul'
self.meta = {}
def setFromConfig(self, config):
'''Merge values from configuration file
@ -350,6 +351,7 @@ class DiskImage(ConfigValue):
username = config.get('username', None)
if username:
self.username = username
self.meta = config.get('metadata', {})
def __eq__(self, other):
if isinstance(other, DiskImage):
@ -364,7 +366,8 @@ class DiskImage(ConfigValue):
other.shell_type == self.shell_type and
other.rebuild_age == self.rebuild_age and
other.release == self.release and
other.username == self.username)
other.username == self.username and
other.meta == self.meta)
return False
def __repr__(self):

3
nodepool/driver/aws/config.py
View File

@ -99,7 +99,7 @@ class AwsProviderDiskImage(ConfigValue):
self.connection_port = image.get(
'connection-port',
default_port_mapping.get(self.connection_type, 22))
self.meta = {}
self.meta = image.get('tags', {})
self.architecture = image.get('architecture', 'x86_64')
self.volume_size = image.get('volume-size', None)
self.volume_type = image.get('volume-type', 'gp2')
@ -121,6 +121,7 @@ class AwsProviderDiskImage(ConfigValue):
'shell-type': str,
'volume-size': int,
'volume-type': str,
'tags': dict,
}

3
nodepool/driver/azure/config.py
View File

@ -110,7 +110,7 @@ class AzureProviderDiskImage(ConfigValue):
self.connection_port = image.get(
'connection-port',
default_port_mapping.get(self.connection_type, 22))
self.meta = {}
self.meta = image.get('tags', {})
@property
def external_name(self):
@ -128,6 +128,7 @@ class AzureProviderDiskImage(ConfigValue):
'connection-port': int,
'python-path': str,
'shell-type': str,
'tags': dict,
}

3
nodepool/driver/fake/provider.py
View File

@ -196,7 +196,8 @@ class FakeOpenStackCloud(object):
key_name=kw.get('key_name', None),
should_fail=should_fail,
over_quota=over_quota,
event=threading.Event())
event=threading.Event(),
_kw=kw)
instance_list.append(s)
t = threading.Thread(target=self._finish,
name='FakeProvider create',

4
nodepool/tests/fixtures/aws/diskimage.yaml vendored
View File

@ -29,6 +29,8 @@ providers:
bucket-name: nodepool
diskimages:
- name: fake-image
tags:
provider_metadata: provider
pools:
- name: main
max-servers: 1
@ -55,3 +57,5 @@ diskimages:
DIB_IMAGE_CACHE: /opt/dib_cache
DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2
metadata:
diskimage_metadata: diskimage

4
nodepool/tests/fixtures/azure-diskimage.yaml vendored
View File

@ -32,6 +32,8 @@ providers:
subnet-id: /subscriptions/c35cf7df-ed75-4c85-be00-535409a85120/resourceGroups/nodepool/providers/Microsoft.Network/virtualNetworks/NodePool/subnets/default
diskimages:
- name: fake-image
tags:
provider_metadata: provider
pools:
- name: main
max-servers: 10
@ -60,3 +62,5 @@ diskimages:
DIB_IMAGE_CACHE: /opt/dib_cache
DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2
metadata:
diskimage_metadata: diskimage

3
nodepool/tests/fixtures/node.yaml vendored
View File

@ -28,6 +28,7 @@ providers:
diskimages:
- name: fake-image
meta:
provider_metadata: provider
key: value
key2: value
pools:
@ -58,3 +59,5 @@ diskimages:
DIB_IMAGE_CACHE: /opt/dib_cache
DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/
BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2
metadata:
diskimage_metadata: diskimage

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

@ -536,6 +536,13 @@ class TestDriverAws(tests.DBTestCase):
image = self.waitForImage('ec2-us-west-2', 'fake-image')
self.assertEqual(image.username, 'zuul')
ec2_image = self.ec2.Image(image.external_id)
self.assertEqual(ec2_image.state, 'available')
self.assertTrue({'Key': 'diskimage_metadata', 'Value': 'diskimage'}
in ec2_image.tags)
self.assertTrue({'Key': 'provider_metadata', 'Value': 'provider'}
in ec2_image.tags)
pool = self.useNodepool(configfile, watermark_sleep=1)
pool.start()
self.patchProvider(pool)

6
nodepool/tests/unit/test_driver_azure.py
View File

@ -105,6 +105,12 @@ class TestDriverAzure(tests.DBTestCase):
self.useBuilder(configfile)
image = self.waitForImage('azure', 'fake-image')
self.assertEqual(image.username, 'zuul')
self.assertEqual(
self.fake_azure.crud['Microsoft.Compute/images'].
items[0]['tags']['provider_metadata'], 'provider')
self.assertEqual(
self.fake_azure.crud['Microsoft.Compute/images'].
items[0]['tags']['diskimage_metadata'], 'diskimage')
configfile = self.setup_config(
'azure-diskimage.yaml',

12
nodepool/tests/unit/test_launcher.py
View File

@ -591,12 +591,20 @@ class TestLauncher(tests.DBTestCase):
"""Test that an image and node are created"""
configfile = self.setup_config('node.yaml')
pool = self.useNodepool(configfile, watermark_sleep=1)
self.useBuilder(configfile)
builder = self.useBuilder(configfile)
pool.start()
image = self.waitForImage('fake-provider', 'fake-image')
self.assertEqual(image.username, 'zuul')
nodes = self.waitForNodes('fake-label')
provider = (builder._upload_workers[0]._config.
provider_managers['fake-provider'])
cloud_image = provider.getImage(image.external_id)
self.assertEqual(
cloud_image._kw.get('diskimage_metadata'), 'diskimage')
self.assertEqual(
cloud_image._kw.get('provider_metadata'), 'provider')
nodes = self.waitForNodes('fake-label')
self.assertEqual(len(nodes), 1)
self.assertEqual(nodes[0].provider, 'fake-provider')
self.assertEqual(nodes[0].type, ['fake-label'])

12
releasenotes/notes/image-tags-ab4f97efdf46e196.yaml Normal file
View File

@ -0,0 +1,12 @@
---
features:
- |
The AWS and Azure drivers now support adding tags to images via
the :attr:`providers.[aws].diskimages.tags`, and
:attr:`providers.[azure].diskimages.tags` attributes. The
OpenStack driver already supported similar behavior with its
`meta` attribute.
The :attr:`diskimages.metadata` attribute has been added and will
supply default values to the provider image tag attributes
mentioned above.