diff --git a/doc/source/aws.rst b/doc/source/aws.rst index 1c11336b4..3d378747a 100644 --- a/doc/source/aws.rst +++ b/doc/source/aws.rst @@ -627,6 +627,46 @@ Selecting the ``aws`` driver adds the following options to the A dictionary of tags to add to the EC2 instances. Values must be supplied as strings. + .. attr:: dynamic-tags + :type: dict + :default: None + + Similar to + :attr:`providers.[aws].pools.labels.tags`, + but is interpreted as a format string with the following + values available: + + * request: Information about the request which prompted the + creation of this node (note that the node may ultimately + be used for a different request and in that case this + information will not be updated). + + * id: The request ID. + + * labels: The list of labels in the request. + + * requestor: The name of the requestor. + + * requestor_data: Key/value information from the requestor. + + * relative_priority: The relative priority of the request. + + * event_id: The external event ID of the request. + + * created_time: The creation time of the request. + + * tenant_name: The name of the tenant associated with the + request. + + For example: + + .. code-block:: yaml + + labels: + - name: precise + dynamic-tags: + request_info: "Created for request {request.id}" + .. _`EBS volume type`: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html .. _`AWS region`: https://docs.aws.amazon.com/general/latest/gr/rande.html .. _`Boto configuration`: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html diff --git a/doc/source/azure.rst b/doc/source/azure.rst index b331ce0c0..34e12ecfc 100644 --- a/doc/source/azure.rst +++ b/doc/source/azure.rst @@ -623,6 +623,46 @@ section of the configuration. A dictionary of tags to add to newly created VMs. + .. attr:: dynamic-tags + :type: dict + :default: None + + Similar to + :attr:`providers.[azure].pools.labels.tags`, + but is interpreted as a format string with the following + values available: + + * request: Information about the request which prompted the + creation of this node (note that the node may ultimately + be used for a different request and in that case this + information will not be updated). + + * id: The request ID. + + * labels: The list of labels in the request. + + * requestor: The name of the requestor. + + * requestor_data: Key/value information from the requestor. + + * relative_priority: The relative priority of the request. + + * event_id: The external event ID of the request. + + * created_time: The creation time of the request. + + * tenant_name: The name of the tenant associated with the + request. + + For example: + + .. code-block:: yaml + + labels: + - name: precise + dynamic-tags: + request_info: "Created for request {request.id}" + .. attr:: user-data :type: str :default: None diff --git a/doc/source/openstack.rst b/doc/source/openstack.rst index 1f3e48e54..8524aa503 100644 --- a/doc/source/openstack.rst +++ b/doc/source/openstack.rst @@ -657,6 +657,46 @@ Selecting the OpenStack driver adds the following options to the ``meta-data`` on the active server (e.g. within ``config-drive:openstack/latest/meta_data.json``) + .. attr:: dynamic-instance-properties + :type: dict + :default: None + + Similar to + :attr:`providers.[openstack].pools.labels.instance-properties`, + but is interpreted as a format string with the following + values available: + + * request: Information about the request which prompted the + creation of this node (note that the node may ultimately + be used for a different request and in that case this + information will not be updated). + + * id: The request ID. + + * labels: The list of labels in the request. + + * requestor: The name of the requestor. + + * requestor_data: Key/value information from the requestor. + + * relative_priority: The relative priority of the request. + + * event_id: The external event ID of the request. + + * created_time: The creation time of the request. + + * tenant_name: The name of the tenant associated with the + request. + + For example: + + .. code-block:: yaml + + labels: + - name: precise + dynamic-instance-properties: + request_info: "Created for request {request.id}" + .. attr:: userdata :type: str :default: None diff --git a/nodepool/driver/aws/__init__.py b/nodepool/driver/aws/__init__.py index 2be0e4a99..bf5822840 100644 --- a/nodepool/driver/aws/__init__.py +++ b/nodepool/driver/aws/__init__.py @@ -16,13 +16,15 @@ # limitations under the License. from nodepool.driver.statemachine import StateMachineDriver -from nodepool.driver.aws.config import AwsProviderConfig -from nodepool.driver.aws.adapter import AwsAdapter +# Import the modules rather than the class so that the unit tests can +# override the classes to add some test-specific methods/data. +import nodepool.driver.aws.config as driver_config +import nodepool.driver.aws.adapter as driver_adapter class AwsDriver(StateMachineDriver): def getProviderConfig(self, provider): - return AwsProviderConfig(self, provider) + return driver_config.AwsProviderConfig(self, provider) def getAdapter(self, provider_config): - return AwsAdapter(provider_config) + return driver_adapter.AwsAdapter(provider_config) diff --git a/nodepool/driver/aws/adapter.py b/nodepool/driver/aws/adapter.py index a4ce36e83..4216a917b 100644 --- a/nodepool/driver/aws/adapter.py +++ b/nodepool/driver/aws/adapter.py @@ -77,6 +77,8 @@ QUOTA_CODES = { 'x': 'L-7295265B', } +CACHE_TTL = 10 + class AwsInstance(statemachine.Instance): def __init__(self, instance, quota): @@ -140,7 +142,7 @@ class AwsCreateStateMachine(statemachine.StateMachine): COMPLETE = 'complete' def __init__(self, adapter, hostname, label, image_external_id, - metadata, retries, log): + metadata, retries, request, log): self.log = log super().__init__() self.adapter = adapter @@ -149,6 +151,11 @@ class AwsCreateStateMachine(statemachine.StateMachine): self.image_external_id = image_external_id self.metadata = metadata self.tags = label.tags.copy() or {} + for k, v in label.dynamic_tags.items(): + try: + self.tags[k] = v.format(request=request.getSafeAttributes()) + except Exception: + self.log.exception("Error formatting tag %s", k) self.tags.update(metadata) self.tags['Name'] = hostname self.hostname = hostname @@ -269,10 +276,10 @@ class AwsAdapter(statemachine.Adapter): self.create_executor.shutdown() self._running = False - def getCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries, log): - return AwsCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries, log) + def getCreateStateMachine(self, hostname, label, image_external_id, + metadata, retries, request, log): + return AwsCreateStateMachine(self, hostname, label, image_external_id, + metadata, retries, request, log) def getDeleteStateMachine(self, external_id, log): return AwsDeleteStateMachine(self, external_id, log) @@ -387,7 +394,7 @@ class AwsAdapter(statemachine.Adapter): # Import snapshot self.log.debug(f"Importing {image_name}") with self.rate_limiter: - import_snapshot_task = self._import_snapshot( + import_snapshot_task = self.ec2_client.import_snapshot( DiskContainer={ 'Format': image_format, 'UserBucket': { @@ -404,7 +411,8 @@ class AwsAdapter(statemachine.Adapter): ) task_id = import_snapshot_task['ImportTaskId'] - paginator = self._get_paginator('describe_import_snapshot_tasks') + paginator = self.ec2_client.get_paginator( + 'describe_import_snapshot_tasks') done = False while not done: time.sleep(self.IMAGE_UPLOAD_SLEEP) @@ -586,7 +594,8 @@ class AwsAdapter(statemachine.Adapter): self.not_our_snapshots.add(snap.id) def _listImportSnapshotTasks(self): - paginator = self._get_paginator('describe_import_snapshot_tasks') + paginator = self.ec2_client.get_paginator( + 'describe_import_snapshot_tasks') with self.non_mutating_rate_limiter: for page in paginator.paginate(): for task in page['ImportSnapshotTasks']: @@ -639,30 +648,30 @@ class AwsAdapter(statemachine.Adapter): return instance return None - @cachetools.func.ttl_cache(maxsize=1, ttl=10) + @cachetools.func.ttl_cache(maxsize=1, ttl=CACHE_TTL) def _listInstances(self): with self.non_mutating_rate_limiter( self.log.debug, "Listed instances"): return list(self.ec2.instances.all()) - @cachetools.func.ttl_cache(maxsize=1, ttl=10) + @cachetools.func.ttl_cache(maxsize=1, ttl=CACHE_TTL) def _listVolumes(self): with self.non_mutating_rate_limiter: return list(self.ec2.volumes.all()) - @cachetools.func.ttl_cache(maxsize=1, ttl=10) + @cachetools.func.ttl_cache(maxsize=1, ttl=CACHE_TTL) def _listAmis(self): # Note: this is overridden in tests due to the filter with self.non_mutating_rate_limiter: return list(self.ec2.images.filter(Owners=['self'])) - @cachetools.func.ttl_cache(maxsize=1, ttl=10) + @cachetools.func.ttl_cache(maxsize=1, ttl=CACHE_TTL) def _listSnapshots(self): # Note: this is overridden in tests due to the filter with self.non_mutating_rate_limiter: return list(self.ec2.snapshots.filter(OwnerIds=['self'])) - @cachetools.func.ttl_cache(maxsize=1, ttl=10) + @cachetools.func.ttl_cache(maxsize=1, ttl=CACHE_TTL) def _listObjects(self): bucket_name = self.provider.object_storage.get('bucket-name') if not bucket_name: @@ -896,12 +905,3 @@ class AwsAdapter(statemachine.Adapter): with self.rate_limiter: self.log.debug(f"Deleting object {external_id}") self.s3.Object(bucket_name, external_id).delete() - - # These methods allow the tests to patch our use of boto to - # compensate for missing methods in the boto mocks. - def _import_snapshot(self, *args, **kw): - return self.ec2_client.import_snapshot(*args, **kw) - - def _get_paginator(self, *args, **kw): - return self.ec2_client.get_paginator(*args, **kw) - # End test methods diff --git a/nodepool/driver/aws/config.py b/nodepool/driver/aws/config.py index a39581e25..73745fa0e 100644 --- a/nodepool/driver/aws/config.py +++ b/nodepool/driver/aws/config.py @@ -166,6 +166,7 @@ class AwsLabel(ConfigValue): self.userdata = label.get('userdata', None) self.iam_instance_profile = label.get('iam-instance-profile', None) self.tags = label.get('tags', {}) + self.dynamic_tags = label.get('dynamic-tags', {}) @staticmethod def getSchema(): @@ -184,6 +185,7 @@ class AwsLabel(ConfigValue): v.Exclusive('arn', 'iam_instance_profile_id'): str }, 'tags': dict, + 'dynamic-tags': dict, } diff --git a/nodepool/driver/azure/adapter.py b/nodepool/driver/azure/adapter.py index e50171ebb..ba5c6b5bf 100644 --- a/nodepool/driver/azure/adapter.py +++ b/nodepool/driver/azure/adapter.py @@ -169,8 +169,9 @@ class AzureCreateStateMachine(statemachine.StateMachine): COMPLETE = 'complete' def __init__(self, adapter, hostname, label, image_external_id, - metadata, retries): + metadata, retries, request, log): super().__init__() + self.log = log self.adapter = adapter self.retries = retries self.attempts = 0 @@ -178,6 +179,11 @@ class AzureCreateStateMachine(statemachine.StateMachine): self.image_reference = None self.metadata = metadata self.tags = label.tags.copy() or {} + for k, v in label.dynamic_tags.items(): + try: + self.tags[k] = v.format(request=request.getSafeAttributes()) + except Exception: + self.log.exception("Error formatting tag %s", k) self.tags.update(metadata) self.hostname = hostname self.label = label @@ -329,9 +335,11 @@ class AzureAdapter(statemachine.Adapter): self._getSKUs() def getCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries, log): + image_external_id, metadata, retries, + request, log): return AzureCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries) + image_external_id, metadata, + retries, request, log) def getDeleteStateMachine(self, external_id, log): return AzureDeleteStateMachine(self, external_id) diff --git a/nodepool/driver/azure/config.py b/nodepool/driver/azure/config.py index ae65b2c59..bd439443f 100644 --- a/nodepool/driver/azure/config.py +++ b/nodepool/driver/azure/config.py @@ -169,6 +169,7 @@ class AzureLabel(ConfigValue): self.hardware_profile = label['hardware-profile'] self.tags = label.get('tags', {}) + self.dynamic_tags = label.get('dynamic-tags', {}) self.user_data = self._encodeData(label.get('user-data', None)) self.custom_data = self._encodeData(label.get('custom-data', None)) @@ -189,6 +190,7 @@ class AzureLabel(ConfigValue): 'diskimage': str, v.Required('hardware-profile'): azure_hardware_profile, 'tags': dict, + 'dynamic-tags': dict, 'user-data': str, 'custom-data': str, } diff --git a/nodepool/driver/ibmvpc/adapter.py b/nodepool/driver/ibmvpc/adapter.py index 5e60e4ea8..9bc9078fa 100644 --- a/nodepool/driver/ibmvpc/adapter.py +++ b/nodepool/driver/ibmvpc/adapter.py @@ -379,9 +379,11 @@ class IBMVPCAdapter(statemachine.Adapter): return authenticator def getCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries, log): + image_external_id, metadata, retries, + request, log): return IBMVPCCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries) + image_external_id, metadata, + retries) def getDeleteStateMachine(self, external_id, log): return IBMVPCDeleteStateMachine(self, external_id) diff --git a/nodepool/driver/metastatic/adapter.py b/nodepool/driver/metastatic/adapter.py index dd12bea05..28e08a599 100644 --- a/nodepool/driver/metastatic/adapter.py +++ b/nodepool/driver/metastatic/adapter.py @@ -271,7 +271,8 @@ class MetastaticAdapter(statemachine.Adapter): return self._provider._zk def getCreateStateMachine(self, hostname, label, - image_external_id, metadata, retries, log): + image_external_id, metadata, retries, + request, log): return MetastaticCreateStateMachine(self, hostname, label, image_external_id, metadata, retries) diff --git a/nodepool/driver/openstack/config.py b/nodepool/driver/openstack/config.py index 8b87ddc67..6c39a6489 100644 --- a/nodepool/driver/openstack/config.py +++ b/nodepool/driver/openstack/config.py @@ -175,7 +175,9 @@ class ProviderPool(ConfigPool): False)) pl.volume_size = label.get('volume-size', 50) pl.instance_properties = label.get('instance-properties', - None) + {}) + pl.dynamic_instance_properties = label.get( + 'dynamic-instance-properties', {}) pl.userdata = label.get('userdata', None) pl.networks = label.get('networks', self.networks) pl.host_key_checking = label.get( @@ -319,6 +321,7 @@ class OpenStackProviderConfig(ProviderConfig): 'boot-from-volume': bool, 'volume-size': int, 'instance-properties': dict, + 'dynamic-instance-properties': dict, 'userdata': str, 'networks': [str], 'host-key-checking': bool, diff --git a/nodepool/driver/openstack/handler.py b/nodepool/driver/openstack/handler.py index 9ac8cf072..fccb0cd01 100644 --- a/nodepool/driver/openstack/handler.py +++ b/nodepool/driver/openstack/handler.py @@ -28,7 +28,8 @@ from nodepool.driver import NodeRequestHandler class OpenStackNodeLauncher(NodeLauncher): - def __init__(self, handler, node, provider_config, provider_label): + def __init__(self, handler, node, provider_config, provider_label, + request): ''' Initialize the launcher. @@ -38,6 +39,8 @@ class OpenStackNodeLauncher(NodeLauncher): describing the provider launching this node. :param ProviderLabel provider_label: A ProviderLabel object describing the label to use for the node. + :param NodeRequest request: The NodeRequest that prompted the + launch. ''' super().__init__(handler, node, provider_config) @@ -46,6 +49,7 @@ class OpenStackNodeLauncher(NodeLauncher): self.label = provider_label self.pool = provider_label.pool + self.request = request def _logConsole(self, server_id, hostname): if not self.label.console_log: @@ -123,6 +127,16 @@ class OpenStackNodeLauncher(NodeLauncher): # because that isn't available in ZooKeeper until after the server is # active, which could cause a race in leak detection. + props = self.label.instance_properties.copy() + for k, v in self.label.dynamic_instance_properties.items(): + try: + props[k] = v.format(request=self.request.getSafeAttributes()) + except Exception: + self.log.exception( + "Error formatting dynamic instance property %s", k) + if not props: + props = None + try: server = self.handler.manager.createServer( hostname, @@ -140,7 +154,7 @@ class OpenStackNodeLauncher(NodeLauncher): security_groups=self.pool.security_groups, boot_from_volume=self.label.boot_from_volume, volume_size=self.label.volume_size, - instance_properties=self.label.instance_properties, + instance_properties=props, userdata=self.label.userdata) except openstack.cloud.exc.OpenStackCloudCreateException as e: if e.resource_id: @@ -457,6 +471,7 @@ class OpenStackNodeRequestHandler(NodeRequestHandler): def launch(self, node): label = self.pool.labels[node.type[0]] - thd = OpenStackNodeLauncher(self, node, self.provider, label) + thd = OpenStackNodeLauncher(self, node, self.provider, label, + self.request) thd.start() self._threads.append(thd) diff --git a/nodepool/driver/statemachine.py b/nodepool/driver/statemachine.py index 482380681..f74a45d6e 100644 --- a/nodepool/driver/statemachine.py +++ b/nodepool/driver/statemachine.py @@ -136,7 +136,8 @@ class StateMachineNodeLauncher(stats.StatsReporter): 'nodepool_pool_name': self.handler.pool.name, 'nodepool_provider_name': self.manager.provider.name} self.state_machine = self.manager.adapter.getCreateStateMachine( - hostname, label, image_external_id, metadata, retries, self.log) + hostname, label, image_external_id, metadata, retries, + self.handler.request, self.log) def updateNodeFromInstance(self, instance): if instance is None: diff --git a/nodepool/nodeutils.py b/nodepool/nodeutils.py index 4e121b89b..fcbe9df5d 100644 --- a/nodepool/nodeutils.py +++ b/nodepool/nodeutils.py @@ -161,3 +161,10 @@ def nodescan(ip, port=22, timeout=60, gather_hostkeys=True): sock = None return keys + + +class Attributes(object): + """A class to hold attributes for string formatting.""" + + def __init__(self, **kw): + setattr(self, '__dict__', kw) diff --git a/nodepool/tests/__init__.py b/nodepool/tests/__init__.py index 925cb8b20..602c98243 100644 --- a/nodepool/tests/__init__.py +++ b/nodepool/tests/__init__.py @@ -285,6 +285,8 @@ class BaseTestCase(testtools.TestCase): continue if t.name.startswith("PoolWorker"): continue + if t.name.startswith("ThreadPoolExecutor"): + continue if t.name not in whitelist: done = False if done: diff --git a/nodepool/tests/fixtures/aws/aws-min-ready.yaml b/nodepool/tests/fixtures/aws/aws-min-ready.yaml new file mode 100644 index 000000000..dd2aa05db --- /dev/null +++ b/nodepool/tests/fixtures/aws/aws-min-ready.yaml @@ -0,0 +1,47 @@ +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} + +zookeeper-tls: + ca: {zookeeper_ca} + cert: {zookeeper_cert} + key: {zookeeper_key} + +tenant-resource-limits: + - tenant-name: tenant-1 + max-cores: 1024 + +labels: + - name: ubuntu1404-with-tags + min-ready: 1 + +providers: + - name: ec2-us-west-2 + driver: aws + region-name: us-west-2 + cloud-images: + - name: ubuntu1404 + image-id: ami-1e749f67 + username: ubuntu + pools: + - name: main + max-servers: 1 + subnet-id: {subnet_id} + security-group-id: {security_group_id} + node-attributes: + key1: value1 + key2: value2 + labels: + - name: ubuntu1404-with-tags + cloud-image: ubuntu1404 + instance-type: t3.medium + key-name: zuul + tags: + has-tags: true + Name: ignored-name + dynamic-tags: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" diff --git a/nodepool/tests/fixtures/aws/aws.yaml b/nodepool/tests/fixtures/aws/aws.yaml index eb875581a..f745bf572 100644 --- a/nodepool/tests/fixtures/aws/aws.yaml +++ b/nodepool/tests/fixtures/aws/aws.yaml @@ -101,6 +101,11 @@ providers: tags: has-tags: true Name: ignored-name + dynamic-tags: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" - name: ubuntu1404-with-shell-type cloud-image: ubuntu1404-with-shell-type instance-type: t3.medium diff --git a/nodepool/tests/fixtures/azure-min-ready.yaml b/nodepool/tests/fixtures/azure-min-ready.yaml new file mode 100644 index 000000000..eaeda8869 --- /dev/null +++ b/nodepool/tests/fixtures/azure-min-ready.yaml @@ -0,0 +1,62 @@ +webapp: + port: 8005 + listen_address: '0.0.0.0' + +zookeeper-servers: + - host: {zookeeper_host} + port: {zookeeper_port} + chroot: {zookeeper_chroot} + +zookeeper-tls: + ca: {zookeeper_ca} + cert: {zookeeper_cert} + key: {zookeeper_key} + +tenant-resource-limits: + - tenant-name: tenant-1 + max-cores: 1024 + +labels: + - name: bionic + min-ready: 1 + +providers: + - name: azure + driver: azure + zuul-public-key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC+mplenM+m6pNY9Un3fpO9eqf808Jrfb3d1gXg7BZVawCvtEZ/cDYvLQ3OF1AeL2kcIC0UAIglM5JXae7yO5CJbJRdkbXvv0u1LvpLxYSPM4ATR0r4IseC5YVxkfJQNi4ixSwTqD4ScEkuCXcSqSU9M+hB+KlnwXoR4IcYHf7vD2Z0Mdwm2ikk3SeERmspmMxx/uz0SPn58QxONuoTlNWQKqDWsV6bRyoPa6HWccMrIH1/e7E69Nw/30oioOQpKBgaDCauh+QkDtSkjRpRMOV47ZFh16Q9DqMgLx+FD8z6++9rsHlB65Zas1xyQsiRCFG09s00b7OR7Xz9ukQ5+vXV + resource-group-location: centralus + location: centralus + resource-group: nodepool + auth-path: {auth_path} + subnet-id: /subscriptions/c35cf7df-ed75-4c85-be00-535409a85120/resourceGroups/nodepool/providers/Microsoft.Network/virtualNetworks/NodePool/subnets/default + cloud-images: + - name: bionic + username: zuul + shell-type: sh + image-reference: + sku: 18.04-LTS + publisher: Canonical + version: latest + offer: UbuntuServer + pools: + - name: main + max-servers: 10 + node-attributes: + key1: value1 + key2: value2 + labels: + - name: bionic + cloud-image: bionic + hardware-profile: + vm-size: Standard_B1ls + tags: + department: R&D + team: DevOps + systemPurpose: CI + dynamic-tags: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" + user-data: "This is the user data" + custom-data: "This is the custom data" diff --git a/nodepool/tests/fixtures/azure.yaml b/nodepool/tests/fixtures/azure.yaml index 143f94c9c..7aa6516cb 100644 --- a/nodepool/tests/fixtures/azure.yaml +++ b/nodepool/tests/fixtures/azure.yaml @@ -86,6 +86,11 @@ providers: department: R&D team: DevOps systemPurpose: CI + dynamic-tags: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" user-data: "This is the user data" custom-data: "This is the custom data" - name: image-by-name diff --git a/nodepool/tests/fixtures/node.yaml b/nodepool/tests/fixtures/node.yaml index c794f53c9..b4d93887d 100644 --- a/nodepool/tests/fixtures/node.yaml +++ b/nodepool/tests/fixtures/node.yaml @@ -46,6 +46,13 @@ providers: diskimage: fake-image min-ram: 8192 flavor-name: 'Fake' + instance-properties: + prop1: foo + dynamic-instance-properties: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" diskimages: - name: fake-image diff --git a/nodepool/tests/fixtures/node_no_min_ready.yaml b/nodepool/tests/fixtures/node_no_min_ready.yaml index d9e7e37de..1f0b424c5 100644 --- a/nodepool/tests/fixtures/node_no_min_ready.yaml +++ b/nodepool/tests/fixtures/node_no_min_ready.yaml @@ -41,6 +41,13 @@ providers: diskimage: fake-image min-ram: 8192 flavor-name: 'Fake' + instance-properties: + prop1: foo + dynamic-instance-properties: + # Note: we double the braces to deal with unit-test + # pre-processing of this file. The output and actual + # file syntax is single braces. + dynamic-tenant: "Tenant is {{request.tenant_name}}" - name: fake-label2 diskimage: fake-image min-ram: 8192 diff --git a/nodepool/tests/unit/test_driver_aws.py b/nodepool/tests/unit/test_driver_aws.py index a996e6607..6bf965822 100644 --- a/nodepool/tests/unit/test_driver_aws.py +++ b/nodepool/tests/unit/test_driver_aws.py @@ -30,6 +30,7 @@ from nodepool.zk import zookeeper as zk from nodepool.nodeutils import iterate_timeout import nodepool.driver.statemachine from nodepool.driver.statemachine import StateMachineProvider +import nodepool.driver.aws.adapter from nodepool.driver.aws.adapter import AwsInstance, AwsAdapter from nodepool.tests.unit.fake_aws import FakeAws @@ -43,6 +44,48 @@ class Dummy: pass +class FakeAwsAdapter(AwsAdapter): + # Patch/override adapter methods to aid unit tests + def __init__(self, *args, **kw): + super().__init__(*args, **kw) + + # Note: boto3 doesn't handle ipv6 addresses correctly + # when in fake mode so we need to intercept the + # create_instances call and validate the args we supply. + def _fake_create_instances(*args, **kwargs): + self.__testcase.create_instance_calls.append(kwargs) + return self.ec2.create_instances_orig(*args, **kwargs) + + self.ec2.create_instances_orig = self.ec2.create_instances + self.ec2.create_instances = _fake_create_instances + self.ec2_client.import_snapshot = \ + self.__testcase.fake_aws.import_snapshot + self.ec2_client.get_paginator = \ + self.__testcase.fake_aws.get_paginator + + # moto does not mock service-quotas, so we do it ourselves: + def _fake_get_service_quota(ServiceCode, QuotaCode, *args, **kwargs): + # This is a simple fake that only returns the number + # of cores. + if self.__quotas is None: + return {'Quota': {'Value': 100}} + else: + return {'Quota': {'Value': self.__quotas.get(QuotaCode)}} + self.aws_quotas.get_service_quota = _fake_get_service_quota + + +def aws_quotas(quotas): + """Specify a set of AWS quota values for use by a test method. + + :arg dict quotas: The quota dictionary. + """ + + def decorator(test): + test.__aws_quotas__ = quotas + return test + return decorator + + class TestDriverAws(tests.DBTestCase): log = logging.getLogger("nodepool.TestDriverAws") mock_ec2 = mock_ec2() @@ -54,6 +97,7 @@ class TestDriverAws(tests.DBTestCase): StateMachineProvider.MINIMUM_SLEEP = 0.1 StateMachineProvider.MAXIMUM_SLEEP = 1 AwsAdapter.IMAGE_UPLOAD_SLEEP = 1 + aws_id = 'AK000000000000000000' aws_key = '0123456789abcdef0123456789abcdef0123456789abcdef' self.useFixture( @@ -103,9 +147,13 @@ class TestDriverAws(tests.DBTestCase): Description='Zuul Nodes') self.security_group_id = self.security_group['GroupId'] self.patch(nodepool.driver.statemachine, 'nodescan', fake_nodescan) - self.patch(AwsAdapter, '_import_snapshot', - self.fake_aws.import_snapshot) - self.patch(AwsAdapter, '_get_paginator', self.fake_aws.get_paginator) + test_name = self.id().split('.')[-1] + test = getattr(self, test_name) + if hasattr(test, '__aws_quotas__'): + quotas = getattr(test, '__aws_quotas__') + else: + quotas = None + self.patchAdapter(quotas=quotas) def tearDown(self): self.mock_ec2.stop() @@ -117,47 +165,18 @@ class TestDriverAws(tests.DBTestCase): kw['security_group_id'] = self.security_group_id return super().setup_config(*args, **kw) - def patchProvider(self, nodepool, provider_name='ec2-us-west-2', - quotas=None): - for _ in iterate_timeout( - 30, Exception, 'wait for provider'): - try: - provider_manager = nodepool.getProviderManager(provider_name) - if provider_manager.adapter.ec2 is not None: - break - except Exception: - pass - - # Note: boto3 doesn't handle ipv6 addresses correctly - # when in fake mode so we need to intercept the - # create_instances call and validate the args we supply. - def _fake_create_instances(*args, **kwargs): - self.create_instance_calls.append(kwargs) - return provider_manager.adapter.ec2.create_instances_orig( - *args, **kwargs) - - provider_manager.adapter.ec2.create_instances_orig =\ - provider_manager.adapter.ec2.create_instances - provider_manager.adapter.ec2.create_instances =\ - _fake_create_instances - - # moto does not mock service-quotas, so we do it ourselves: - def _fake_get_service_quota(ServiceCode, QuotaCode, *args, **kwargs): - # This is a simple fake that only returns the number - # of cores. - if quotas is None: - return {'Quota': {'Value': 100}} - else: - return {'Quota': {'Value': quotas.get(QuotaCode)}} - provider_manager.adapter.aws_quotas.get_service_quota =\ - _fake_get_service_quota + def patchAdapter(self, quotas=None): + self.patch(nodepool.driver.aws.adapter, 'AwsAdapter', FakeAwsAdapter) + self.patch(nodepool.driver.aws.adapter.AwsAdapter, + '_FakeAwsAdapter__testcase', self) + self.patch(nodepool.driver.aws.adapter.AwsAdapter, + '_FakeAwsAdapter__quotas', quotas) def requestNode(self, config_path, label): # A helper method to perform a single node request configfile = self.setup_config(config_path) pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool) req = zk.NodeRequest() req.state = zk.REQUESTED @@ -190,7 +209,6 @@ class TestDriverAws(tests.DBTestCase): configfile = self.setup_config('aws/aws-multiple.yaml') pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool) reqs = [] for x in range(4): @@ -211,15 +229,15 @@ class TestDriverAws(tests.DBTestCase): for node in nodes: self.waitForNodeDeletion(node) + @aws_quotas({ + 'L-1216C47A': 1, + 'L-43DA4232': 224, + }) def test_aws_multi_quota(self): # Test multiple instance type quotas (standard and high-mem) configfile = self.setup_config('aws/aws-quota.yaml') pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool, quotas={ - 'L-1216C47A': 1, - 'L-43DA4232': 224, - }) # Create a high-memory node request. req1 = zk.NodeRequest() @@ -265,16 +283,16 @@ class TestDriverAws(tests.DBTestCase): req3 = self.waitForNodeRequest(req3) self.assertSuccess(req3) + @aws_quotas({ + 'L-1216C47A': 1000, + 'L-43DA4232': 1000, + }) def test_aws_multi_pool_limits(self): # Test multiple instance type quotas (standard and high-mem) # with pool resource limits configfile = self.setup_config('aws/aws-limits.yaml') pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool, quotas={ - 'L-1216C47A': 1000, - 'L-43DA4232': 1000, - }) # Create a standard node request. req1 = zk.NodeRequest() @@ -310,16 +328,16 @@ class TestDriverAws(tests.DBTestCase): req2 = self.waitForNodeRequest(req2) self.assertSuccess(req2) + @aws_quotas({ + 'L-1216C47A': 1000, + 'L-43DA4232': 1000, + }) def test_aws_multi_tenant_limits(self): # Test multiple instance type quotas (standard and high-mem) # with tenant resource limits configfile = self.setup_config('aws/aws-limits.yaml') pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool, quotas={ - 'L-1216C47A': 1000, - 'L-43DA4232': 1000, - }) # Create a high node request. req1 = zk.NodeRequest() @@ -501,6 +519,26 @@ class TestDriverAws(tests.DBTestCase): self.assertIn({"Key": "has-tags", "Value": "true"}, tag_list) self.assertIn({"Key": "Name", "Value": "np0000000000"}, tag_list) self.assertNotIn({"Key": "Name", "Value": "ignored-name"}, tag_list) + self.assertIn( + {"Key": "dynamic-tenant", "Value": "Tenant is tenant-1"}, tag_list) + + def test_aws_min_ready(self): + # Test dynamic tag formatting without a real node request + configfile = self.setup_config('aws/aws-min-ready.yaml') + pool = self.useNodepool(configfile, watermark_sleep=1) + pool.start() + node = self.waitForNodes('ubuntu1404-with-tags')[0] + + self.assertEqual(node.host_keys, ['ssh-rsa FAKEKEY']) + self.assertEqual(node.image_id, 'ubuntu1404') + + instance = self.ec2.Instance(node.external_id) + tag_list = instance.tags + self.assertIn({"Key": "has-tags", "Value": "true"}, tag_list) + self.assertIn({"Key": "Name", "Value": "np0000000000"}, tag_list) + self.assertNotIn({"Key": "Name", "Value": "ignored-name"}, tag_list) + self.assertIn( + {"Key": "dynamic-tenant", "Value": "Tenant is None"}, tag_list) def test_aws_shell_type(self): req = self.requestNode('aws/shell-type.yaml', @@ -545,7 +583,6 @@ class TestDriverAws(tests.DBTestCase): pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool) req = zk.NodeRequest() req.state = zk.REQUESTED @@ -574,8 +611,6 @@ class TestDriverAws(tests.DBTestCase): self.waitForBuildDeletion('fake-image', '0000000001') def test_aws_resource_cleanup(self): - self.patch(AwsAdapter, '_get_paginator', self.fake_aws.get_paginator) - # Start by setting up leaked resources instance_tags = [ {'Key': 'nodepool_node_id', 'Value': '0000000042'}, @@ -682,7 +717,6 @@ class TestDriverAws(tests.DBTestCase): configfile = self.setup_config('aws/diskimage.yaml') pool = self.useNodepool(configfile, watermark_sleep=1) pool.start() - self.patchProvider(pool) for _ in iterate_timeout(30, Exception, 'instance deletion'): instance = self.ec2.Instance(instance_id) diff --git a/nodepool/tests/unit/test_driver_azure.py b/nodepool/tests/unit/test_driver_azure.py index 56ebd3a0b..35d7fe996 100644 --- a/nodepool/tests/unit/test_driver_azure.py +++ b/nodepool/tests/unit/test_driver_azure.py @@ -96,6 +96,39 @@ class TestDriverAzure(tests.DBTestCase): self.fake_azure.crud['Microsoft.Compute/virtualMachines']. requests[0]['properties']['userData'], 'VGhpcyBpcyB0aGUgdXNlciBkYXRh') # This is the user data + tags = (self.fake_azure.crud['Microsoft.Compute/virtualMachines']. + requests[0]['tags']) + self.assertEqual(tags.get('team'), 'DevOps') + self.assertEqual(tags.get('dynamic-tenant'), 'Tenant is tenant-1') + + def test_azure_min_ready(self): + configfile = self.setup_config( + 'azure-min-ready.yaml', + auth_path=self.fake_azure.auth_file.name) + pool = self.useNodepool(configfile, watermark_sleep=1) + pool.start() + node = self.waitForNodes('bionic')[0] + + self.assertEqual(node.state, zk.READY) + self.assertIsNotNone(node.launcher) + self.assertEqual(node.connection_type, 'ssh') + self.assertEqual(node.shell_type, 'sh') + self.assertEqual(node.attributes, + {'key1': 'value1', 'key2': 'value2'}) + self.assertEqual(node.host_keys, ['ssh-rsa FAKEKEY']) + self.assertEqual(node.python_path, 'auto') + self.assertEqual( + self.fake_azure.crud['Microsoft.Compute/virtualMachines']. + items[0]['properties']['osProfile']['customData'], + 'VGhpcyBpcyB0aGUgY3VzdG9tIGRhdGE=') # This is the custom data + self.assertEqual( + self.fake_azure.crud['Microsoft.Compute/virtualMachines']. + requests[0]['properties']['userData'], + 'VGhpcyBpcyB0aGUgdXNlciBkYXRh') # This is the user data + tags = (self.fake_azure.crud['Microsoft.Compute/virtualMachines']. + requests[0]['tags']) + self.assertEqual(tags.get('team'), 'DevOps') + self.assertEqual(tags.get('dynamic-tenant'), 'Tenant is None') def test_azure_diskimage(self): configfile = self.setup_config( diff --git a/nodepool/tests/unit/test_launcher.py b/nodepool/tests/unit/test_launcher.py index 2346d706f..71680cc47 100644 --- a/nodepool/tests/unit/test_launcher.py +++ b/nodepool/tests/unit/test_launcher.py @@ -86,6 +86,18 @@ class TestLauncher(tests.DBTestCase): 'ram': 8192, } self.assertEqual(node.resources, resources) + + # We check the "cloud" side attributes are set from nodepool side + provider = pool.getProviderManager('fake-provider') + cloud_node = provider.getServer(node.hostname) + self.assertEqual( + cloud_node.metadata['nodepool_provider_name'], + 'fake-provider') + self.assertEqual(cloud_node.metadata['nodepool_pool_name'], 'main') + self.assertEqual(cloud_node.metadata['prop1'], 'foo') + self.assertEqual(cloud_node.metadata['dynamic-tenant'], + 'Tenant is tenant-1') + self.zk.lockNode(node, blocking=False) self.zk.unlockNode(node) @@ -632,6 +644,9 @@ class TestLauncher(tests.DBTestCase): cloud_node.metadata['nodepool_provider_name'], 'fake-provider') self.assertEqual(cloud_node.metadata['nodepool_pool_name'], 'main') + self.assertEqual(cloud_node.metadata['prop1'], 'foo') + self.assertEqual(cloud_node.metadata['dynamic-tenant'], + 'Tenant is None') def test_node_network_cli(self): """Same as test_node but using connection-type network_cli""" diff --git a/nodepool/zk/zookeeper.py b/nodepool/zk/zookeeper.py index e8c297d0f..b693d358a 100644 --- a/nodepool/zk/zookeeper.py +++ b/nodepool/zk/zookeeper.py @@ -27,6 +27,7 @@ from nodepool import exceptions as npe from nodepool.logconfig import get_annotated_logger from nodepool.zk.components import COMPONENT_REGISTRY from nodepool.zk import ZooKeeperBase +from nodepool.nodeutils import Attributes # States: # We are building this image (or node) but it is not ready for use. @@ -463,6 +464,19 @@ class NodeRequest(BaseModel): self.created_time = d.get('created_time') self.tenant_name = d.get('tenant_name') + def getSafeAttributes(self): + '''Return a dict of attributes safe for user-visible templating''' + return Attributes( + id=self.id, + labels=self.node_types, + requestor=self.requestor, + requestor_data=self.requestor_data, + relative_priority=self.relative_priority, + event_id=self.event_id, + created_time=self.created_time, + tenant_name=self.tenant_name, + ) + class Node(BaseModel): ''' diff --git a/releasenotes/notes/dynamic-tags-693c8f61f74600c1.yaml b/releasenotes/notes/dynamic-tags-693c8f61f74600c1.yaml new file mode 100644 index 000000000..7364725dd --- /dev/null +++ b/releasenotes/notes/dynamic-tags-693c8f61f74600c1.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + The OpenStack, AWS, and Azure drivers now support adding tags (AKA + metadata, AKA properties) to instances with dynamic data about the + corresponding node request using string formatting. + + See :attr:`providers.[openstack].pools.labels.instance-properties`, + :attr:`providers.[aws].pools.labels.dynamic-tags`, and + :attr:`providers.[azure].pools.labels.dynamic-tags` for details.