Merge pull request #251 from stevekeay/PUC-448

chore: add testing and linting to neutron driver plugin

.github/workflows/code-test.yaml

@@ -6,11 +6,13 @@ on:
       - main
     paths:
       - "python/ironic-understack/**"
+      - "python/neutron-understack/**"
       - "python/understack-workflows/**"
       - ".github/workflows/code-test.yaml"
   pull_request_target:
     paths:
       - "python/ironic-understack/**"
+      - "python/neutron-understack/**"
       - "python/understack-workflows/**"
       - ".github/workflows/code-test.yaml"
   workflow_dispatch:
@@ -51,3 +53,24 @@ jobs:
         with:
           coverageFile: python/${{ matrix.project }}/coverage.xml
           token: ${{ secrets.GITHUB_TOKEN }}
+
+  neutron-understack:
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: ./python/neutron-understack
+
+    steps:
+      - uses: actions/checkout@v4
+      - run: pipx install poetry==1.7.1 && poetry self add 'poetry-dynamic-versioning[plugin]'
+      - uses: actions/setup-python@v5
+        with:
+          python-version-file: python/neutron-understack/pyproject.toml
+          cache: "poetry"
+      - run: poetry install --sync --with test
+      - run: poetry build
+      - run: "poetry run pytest --cov-report xml:coverage.xml"
+      - uses: orgoro/[email protected]
+        with:
+          coverageFile: python/ironic-understack/coverage.xml
+          token: ${{ secrets.GITHUB_TOKEN }}

python/neutron-understack/neutron_understack/argo/workflows.py

@@ -1,24 +1,31 @@
-import requests
 import time
+
+import requests
 import urllib3
 
 urllib3.disable_warnings()
 
 
 class ArgoClient:
+    DEFAULT_TOKEN_FILENAME = "/run/secrets/kubernetes.io/serviceaccount/token"
+
     def __init__(
         self,
-        token: str,
+        token: str = None,
         namespace="default",
         api_url="https://argo-server.argo.svc.cluster.local:2746",
-        logger=None
+        logger=None,
     ):
         self.token = token
         self.namespace = namespace
         self.api_url = api_url
         self.headers = {"Authorization": f"Bearer {self.token}"}
         self.logger = logger
 
+        if token is None:
+            with open(DEFAULT_TOKEN_FILENAME) as token_file:
+                token = token_file.read()
+
     def submit(
         self,
         template_name: str,

python/neutron-understack/neutron_understack/neutron_understack_mech.py

@@ -1,15 +1,17 @@
 import json
 import logging
-import uuid
 from pprint import pprint
+from uuid import UUID
 
 import neutron_lib.api.definitions.portbindings as portbindings
 from neutron_lib import constants as p_const
 from neutron_lib.plugins.ml2 import api
-from neutron_lib.plugins.ml2.api import MechanismDriver
-from neutron_lib.plugins.ml2.api import NetworkContext
-from neutron_lib.plugins.ml2.api import PortContext
-from neutron_lib.plugins.ml2.api import SubnetContext
+from neutron_lib.plugins.ml2.api import (
+    MechanismDriver,
+    NetworkContext,
+    PortContext,
+    SubnetContext,
+)
 from oslo_config import cfg
 
 from neutron_understack.argo.workflows import ArgoClient
@@ -56,17 +58,6 @@ def setup_conf():
 
 setup_conf()
 
-argo_token = None
-with open("/run/secrets/kubernetes.io/serviceaccount/token") as token_file:
-    argo_token = token_file.read()
-
-argo_client = ArgoClient(
-    argo_token,
-    logger=LOG,
-    api_url=cfg.CONF.ml2_type_understack.argo_api_url,
-    namespace=cfg.CONF.ml2_type_understack.argo_namespace,
-)
-
 
 def dump_context(
     context: NetworkContext | SubnetContext | PortContext,
@@ -149,9 +140,7 @@ def log_call(
 
 class UnderstackDriver(MechanismDriver):
     # See MechanismDriver docs for resource_provider_uuid5_namespace
-    resource_provider_uuid5_namespace = uuid.UUID(
-        "6eae3046-4072-11ef-9bcf-d6be6370a162"
-    )
+    resource_provider_uuid5_namespace = UUID("6eae3046-4072-11ef-9bcf-d6be6370a162")
 
     def initialize(self):
         pass
@@ -204,8 +193,19 @@ def update_port_precommit(self, context):
     def update_port_postcommit(self, context):
         log_call("update_port_postcommit", context)
 
-        if context.current["binding:vif_type"] == portbindings.VIF_TYPE_OTHER:
-            self._move_to_network(context)
+        argo_client = ArgoClient(
+            logger=LOG,
+            api_url=cfg.CONF.ml2_type_understack.argo_api_url,
+            namespace=cfg.CONF.ml2_type_understack.argo_namespace,
+        )
+
+        self._move_to_network(
+            vif_type=context.current["binding:vif_type"],
+            mac_address=context.current["mac_address"],
+            device_uuid=context.current["binding:host_id"],
+            network_id=context.current["network_id"],
+            argo_client=argo_client,
+        )
 
     def delete_port_precommit(self, context):
         log_call("delete_port_precommit", context)
@@ -256,23 +256,39 @@ def check_segment(self, segment):
     def check_vlan_transparency(self, context):
         log_call("check_vlan_transparency", context)
 
-    def __network_name(self, network_id: str):
+    def _move_to_network(
+        self,
+        vif_type: str,
+        mac_address: str,
+        device_uuid: UUID,
+        network_id: str,
+        argo_client: ArgoClient,
+    ):
+        """Triggers Argo "trigger-undersync" workflow.
+
+        This has the effect of connecting our server to the given networks: either
+        "provisioning" (for PXE booting) or "tenant" (normal access to customer's
+        networks).  The choice of network is based on the network ID.
+
+        This only happens when vif_type is VIF_TYPE_OTHER.
+
+        argo_client is injected by the caller to make testing easier
+        """
+        if vif_type != portbindings.VIF_TYPE_OTHER:
+            return
+
         if network_id == cfg.CONF.ml2_type_understack.provisioning_network:
-            return "provisioning"
+            network_name = "provisioning"
         else:
-            return "tenant"
+            network_name = "tenant"
 
-    def _move_to_network(self, context):
-        interface_mac = context.current["mac_address"]
-        device_uuid = context.current["binding:host_id"]
-        network_name = self.__network_name(context.current["network_id"])
-        LOG.debug(f"Selected {network_name=} for {device_uuid=} {interface_mac=}")
+        LOG.debug(f"Selected {network_name=} for {device_uuid=} {mac_address=}")
 
         result = argo_client.submit(
             template_name="undersync-device",
             entrypoint="trigger-undersync",
             parameters={
-                "interface_mac": interface_mac,
+                "interface_mac": mac_address,
                 "device_uuid": device_uuid,
                 "network_name": network_name,
                 "dry_run": cfg.CONF.ml2_type_understack.argo_dry_run,
@@ -281,4 +297,3 @@ def _move_to_network(self, context):
             service_account=cfg.CONF.ml2_type_understack.argo_workflow_sa,
         )
         LOG.info(f"Binding workflow submitted: {result}")
-        return context

python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py

@@ -1,37 +1,35 @@
-from dataclasses import dataclass
-from unittest.mock import patch, MagicMock
+from unittest.mock import MagicMock
+
 import pytest
-from neutron_understack.argo.workflows import ArgoClient
 
 from neutron_understack.neutron_understack_mech import UnderstackDriver
+from neutron_understack.argo.workflows import ArgoClient
 
-# TODO: I am not sure how we run tests in this project.
 
-@dataclass
-class ContextDouble:
-  current: dict
+@pytest.fixture
+def argo_client() -> ArgoClient:
+    return MagicMock(spec_set=ArgoClient)
 
-def mock_context_data(file):
-    ref = pathlib.Path(__file__).joinpath(filename)
-    with ref.open("r") as f:
-        return ContextDouble(json.load(f))
 
-@patch('neutron_understack.argo.workflows.ArgoClient')
-def test_update_port_postcommit(mock_argo_client):
-    context_data = mock_context_data(
-        "fixtures/neutron_update_port_postcommit.json"
+def test_move_to_network__provisioning(argo_client):
+    driver = UnderstackDriver()
+    driver._move_to_network(
+        vif_type="other",
+        mac_address="fa:16:3e:35:1c:3d",
+        device_uuid="41d18c6a-5548-4ee9-926f-4e3ebf43153f",
+        network_id="c2702769-5592-4555-8ae6-e670db82c31e",
+        argo_client=argo_client,
     )
 
-    UnderstackDriver.update_port_postcommit(context_data)
-
-    mock_argo_client.assert_called_once_with(
+    argo_client.submit.assert_called_once_with(
         template_name="undersync-device",
         entrypoint="trigger-undersync",
         parameters={
-            "interface_uuid": "e5d5cd73-ca9a-4b74-9d52-43188d0cdcaa",
+            "interface_mac": "fa:16:3e:35:1c:3d",
             "device_uuid": "41d18c6a-5548-4ee9-926f-4e3ebf43153f",
-            "network_name": "provisioning",
-            "dry_run": false,
-            "force": false,
+            "network_name": "tenant",
+            "dry_run": True,
+            "force": False,
         },
+        service_account="workflow",
     )