From cb8366d70c69194b56c6fe0510bc93474256067a Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jim@acmegating.com>
Date: Mon, 23 Oct 2023 10:18:32 -0700
Subject: [PATCH] Use backing node attributes as metastatic default

To support the use case where one has multiple pools providing
metastatic backing nodes, and those pools are in different regions,
and a user wishes to use Zuul executor zones to communicate with
whatever metastatic nodes eventually produced from those regions,
this change updates the launcher and metastatic driver to use
the node attributes (where zuul executor region names are specified)
as default values for metastatic node attributes.  This lets users
configure nodepool with zuul executor zones only on the backing pools.

Change-Id: Ie6bdad190f8f0d61dab0fec37642d7a078ab52b3
Co-Authored-By: Benedikt Loeffler <benedikt.loeffler@bmw.de>
---
 doc/source/metastatic.rst                        | 10 ++++++++--
 nodepool/driver/metastatic/adapter.py            |  1 +
 nodepool/driver/statemachine.py                  | 16 ++++++++++++++++
 nodepool/tests/unit/test_driver_metastatic.py    |  1 +
 ...astatic-node-attributes-900d688f5ec9c269.yaml |  8 ++++++++
 5 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/metastatic-node-attributes-900d688f5ec9c269.yaml

diff --git a/doc/source/metastatic.rst b/doc/source/metastatic.rst
index a01a8986d..a518aa22c 100644
--- a/doc/source/metastatic.rst
+++ b/doc/source/metastatic.rst
@@ -88,8 +88,14 @@ itself, which is "meta".
        .. attr:: node-attributes
           :type: dict
 
-          A dictionary of key-value pairs that will be stored with the node data
-          in ZooKeeper. The keys and values can be any arbitrary string.
+          A dictionary of key-value pairs that will be stored with the
+          node data in ZooKeeper. The keys and values can be any
+          arbitrary string.
+
+          The metastatic driver will automatically use the values
+          supplied by the backing node as default values.  Any values
+          specified here for top-level dictionary keys will override
+          those supplied by the backing node.
 
        .. attr:: max-servers
           :type: int
diff --git a/nodepool/driver/metastatic/adapter.py b/nodepool/driver/metastatic/adapter.py
index 3569b4bff..e39f9a872 100644
--- a/nodepool/driver/metastatic/adapter.py
+++ b/nodepool/driver/metastatic/adapter.py
@@ -124,6 +124,7 @@ class MetastaticInstance(statemachine.Instance):
             self.connection_port = backing_node.connection_port
             self.connection_type = backing_node.connection_type
             self.host_keys = backing_node.host_keys
+            self.node_attributes = backing_node.attributes
             backing_node_id = backing_node.id
         else:
             backing_node_id = None
diff --git a/nodepool/driver/statemachine.py b/nodepool/driver/statemachine.py
index b3204eac6..f8fbabccc 100644
--- a/nodepool/driver/statemachine.py
+++ b/nodepool/driver/statemachine.py
@@ -173,6 +173,16 @@ class StateMachineNodeLauncher(stats.StatsReporter):
             if hasattr(instance, attr):
                 setattr(node, attr, getattr(instance, attr))
 
+        # As a special case for metastatic, if we got node_attributes
+        # from the backing driver, use them as default values and let
+        # the values from the pool override.
+        instance_node_attrs = getattr(instance, 'node_attributes', None)
+        if instance_node_attrs is not None:
+            attrs = instance_node_attrs.copy()
+            if node.attributes:
+                attrs.update(node.attributes)
+            node.attributes = attrs
+
         self.zk.storeNode(node)
 
     def runDeleteStateMachine(self):
@@ -991,6 +1001,12 @@ class Instance:
     * connection_port: str
     * connection_type: str
     * host_keys: [str]
+
+    This is extremely optional, in fact, it's difficult to imagine
+    that it's useful for anything other than the metastatic driver:
+
+    * node_attributes: dict
+
     """
     def __init__(self):
         self.ready = False
diff --git a/nodepool/tests/unit/test_driver_metastatic.py b/nodepool/tests/unit/test_driver_metastatic.py
index 5b7098aae..7215c5ea0 100644
--- a/nodepool/tests/unit/test_driver_metastatic.py
+++ b/nodepool/tests/unit/test_driver_metastatic.py
@@ -115,6 +115,7 @@ class TestDriverMetastatic(tests.DBTestCase):
             'testattr': 'backing',
         })
         self.assertEqual(node1.attributes, {
+            'backattr': 'back',
             'metaattr': 'meta',
             'testattr': 'metastatic',
         })
diff --git a/releasenotes/notes/metastatic-node-attributes-900d688f5ec9c269.yaml b/releasenotes/notes/metastatic-node-attributes-900d688f5ec9c269.yaml
new file mode 100644
index 000000000..298f8dd8f
--- /dev/null
+++ b/releasenotes/notes/metastatic-node-attributes-900d688f5ec9c269.yaml
@@ -0,0 +1,8 @@
+---
+features:
+  - |
+    The metastatic driver will now automatically use the
+    `node-attributes` from backing nodes as default values for
+    `node-attributes` of metastatic nodes.  Any `node-attribute`
+    values specified in the metastatic pool config will override those
+    from the backing node.