LP#2030108 - Add L2 Advertisements deployed via the charm (#34)

* Add L2 Advertisements deployed via the charm

* remove node12 warnings in gh

.github/workflows/workflow.yaml

@@ -29,7 +29,7 @@ jobs:
         rbac: [ "without RBAC", "with RBAC" ]
     steps:
       - name: Check out code
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
       - name: Setup operator environment
         uses: charmed-kubernetes/actions-operator@main
         with:

src/charm.py

@@ -84,8 +84,10 @@ def __init__(self, *args):
         self.native_manifest = MetallbNativeManifest(self, self.config)
         self.native_collector = Collector(self.native_manifest)
         self.client = Client(namespace=self.model.name, field_manager=self.app.name)
-        self.pool_name = f"{self.model.name}-{self.app.name}"
+        self.l2_adv_name = self.pool_name = f"{self.model.name}-{self.app.name}"
+
         # Create generic lightkube resource class for the MetalLB IPAddressPool
+        # Create generic lightkube resource class for the MetalLB L2Advertisement
         # https://metallb.universe.tf/configuration/
         self.IPAddressPool = create_namespaced_resource(
             group="metallb.io",
@@ -94,12 +96,34 @@ def __init__(self, *args):
             plural="ipaddresspools",
         )
 
+        self.L2Advertisement = create_namespaced_resource(
+            group="metallb.io",
+            version="v1beta1",
+            kind="L2Advertisement",
+            plural="l2advertisements",
+        )
+
         self.framework.observe(self.on.install, self._install_or_upgrade)
         self.framework.observe(self.on.config_changed, self._on_config_changed)
         self.framework.observe(self.on.upgrade_charm, self._install_or_upgrade)
         self.framework.observe(self.on.update_status, self._update_status)
         self.framework.observe(self.on.remove, self._cleanup)
 
+    def _confirm_resource(self, resource, name) -> bool:
+        try:
+            self.client.get(resource, name=name, namespace=self.config["namespace"])
+        except ApiError as e:
+            if "not found" in e.status.message:
+                logger.info(f"{resource.__name__} not found yet")
+                self.unit.status = WaitingStatus(f"Waiting for {resource.__name__} to be created")
+                return False
+            else:
+                # surface any other errors besides not found
+                logger.exception(e)
+                self.unit.status = WaitingStatus("Waiting for Kubernetes API")
+                return False
+        return True
+
     def _update_status(self, _):
         missing = _missing_resources(self.native_manifest)
         if len(missing) != 0:
@@ -115,20 +139,11 @@ def _update_status(self, _):
             self.unit.status = WaitingStatus(", ".join(native_unready))
             return
 
-        try:
-            self.client.get(
-                self.IPAddressPool, name=self.pool_name, namespace=self.config["namespace"]
-            )
-        except ApiError as e:
-            if "not found" in e.status.message:
-                logger.info("IPAddressPool not found yet")
-                self.unit.status = WaitingStatus("Waiting for IPAddressPool to be created")
-                return
-            else:
-                # surface any other errors besides not found
-                logger.exception(e)
-                self.unit.status = WaitingStatus("Waiting for Kubernetes API")
-                return
+        if not self._confirm_resource(self.IPAddressPool, self.pool_name):
+            return
+        if not self._confirm_resource(self.L2Advertisement, self.l2_adv_name):
+            return
+
         self.unit.status = ActiveStatus("Ready")
         self.unit.set_workload_version(self.native_collector.short_version)
         self.app.status = ActiveStatus(self.native_collector.long_version)
@@ -156,13 +171,14 @@ def _on_config_changed(self, event):
 
         addresses = stripped.split(",")
         self._update_ip_pool(addresses)
+        self._update_l2_adv()
         self.unit.status = ActiveStatus()
 
     # retrying is necessary as the ip address pool webhooks take some time to come up
     @retry(
         retry=retry_if_exception_type(ApiError),
         reraise=True,
-        before=before_log(logger, logging.INFO),
+        before=before_log(logger, logging.DEBUG),
         wait=wait_exponential(multiplier=1, min=2, max=60 * 2),
     )
     def _update_ip_pool(self, addresses):
@@ -173,6 +189,14 @@ def _update_ip_pool(self, addresses):
 
         self.client.apply(ip_pool, force=True)
 
+    def _update_l2_adv(self):
+        l2_adv = self.L2Advertisement(
+            metadata={"name": self.l2_adv_name, "namespace": self.config["namespace"]},
+            spec={"ipAddressPools": [self.pool_name]},
+        )
+
+        self.client.apply(l2_adv, force=True)
+
 
 if __name__ == "__main__":  # pragma: nocover
     main(MetallbCharm)

tests/data/microbot.yaml

@@ -24,7 +24,7 @@ spec:
         app: microbot-lb
     spec:
       containers:
-      - image: dontrebootme/microbot:v1
+      - image: rocks.canonical.com:443/cdk/cdkbot/microbot-amd64:latest
         imagePullPolicy: ""
         name: microbot-lb
         ports:

tests/integration/conftest.py

@@ -11,6 +11,20 @@
 logger = logging.getLogger(__name__)
 
 
+def pytest_addoption(parser):
+    parser.addoption(
+        "--iprange",
+        action="store",
+        default="10.1.240.240-10.1.240.241",
+        help="Juju controller to use",
+    )
+
+
+@pytest.fixture
+def iprange(request):
+    return request.config.getoption("--iprange")
+
+
 @pytest.fixture(scope="module")
 def client():
     return Client()

tests/integration/test_charm.py

@@ -37,7 +37,7 @@ async def test_build_and_deploy(ops_test: OpsTest):
     )
 
 
-async def test_iprange_config_option(ops_test: OpsTest, client, ip_address_pool):
+async def test_iprange_config_option(ops_test: OpsTest, client, ip_address_pool, iprange):
     # test that default option is applied correctly before changing
     pool_name = f"{ops_test.model_name}-{APP_NAME}"
     pool = client.get(ip_address_pool, name=pool_name, namespace=NAMESPACE)
@@ -47,12 +47,12 @@ async def test_iprange_config_option(ops_test: OpsTest, client, ip_address_pool)
     logger.info("Updating iprange ...")
     await app.set_config(
         {
-            "iprange": "10.1.240.240-10.1.240.241",
+            "iprange": iprange,
         }
     )
     await ops_test.model.wait_for_idle(status="active", timeout=60 * 10)
     pool = client.get(ip_address_pool, name=pool_name, namespace=NAMESPACE)
-    assert pool.spec["addresses"][0] == "10.1.240.240-10.1.240.241"
+    assert pool.spec["addresses"][0] == iprange
 
 
 async def test_loadbalancer_service(ops_test: OpsTest, client, microbot_service_ip):

tests/unit/test_charm.py

@@ -66,11 +66,17 @@ def test_config_change_updates_ip_pool(harness, lk_charm_client):
 
     # test with single ip range
     harness.update_config({"iprange": "10.1.240.240-10.1.240.241"})
-    harness.charm.on.config_changed.emit()
-    for call in lk_charm_client.apply.call_args_list:
-        call_obj = call.args[0].to_dict()
-        assert len(call_obj["spec"]["addresses"]) == 1
-        assert call_obj["spec"]["addresses"][0] == "10.1.240.240-10.1.240.241"
+    assert len(lk_charm_client.apply.call_args_list) == 2
+    apply_ip_pool_call = lk_charm_client.apply.call_args_list[0]
+    apply_l2_adv_call = lk_charm_client.apply.call_args_list[1]
+
+    call_obj = apply_ip_pool_call.args[0].to_dict()
+    assert len(call_obj["spec"]["addresses"]) == 1
+    assert call_obj["spec"]["addresses"][0] == "10.1.240.240-10.1.240.241"
+
+    call_obj = apply_l2_adv_call.args[0].to_dict()
+    assert len(call_obj["spec"]["ipAddressPools"]) == 1
+    assert call_obj["spec"]["ipAddressPools"][0] == "None-metallb"
 
     # test with multiple ranges
     lk_charm_client.reset_mock()
@@ -79,14 +85,14 @@ def test_config_change_updates_ip_pool(harness, lk_charm_client):
             "iprange": "192.168.1.240-192.168.1.247,10.1.240.240-10.1.240.241,192.168.10.0/24,fc00:f853:0ccd:e799::/124"
         }
     )
-    harness.charm.on.config_changed.emit()
-    for call in lk_charm_client.apply.call_args_list:
-        call_obj = call.args[0].to_dict()
-        assert len(call_obj["spec"]["addresses"]) == 4
-        assert call_obj["spec"]["addresses"][0] == "192.168.1.240-192.168.1.247"
-        assert call_obj["spec"]["addresses"][1] == "10.1.240.240-10.1.240.241"
-        assert call_obj["spec"]["addresses"][2] == "192.168.10.0/24"
-        assert call_obj["spec"]["addresses"][3] == "fc00:f853:0ccd:e799::/124"
+    assert len(lk_charm_client.apply.call_args_list) == 2
+    apply_ip_pool_call = lk_charm_client.apply.call_args_list[0]
+    call_obj = apply_ip_pool_call.args[0].to_dict()
+    assert len(call_obj["spec"]["addresses"]) == 4
+    assert call_obj["spec"]["addresses"][0] == "192.168.1.240-192.168.1.247"
+    assert call_obj["spec"]["addresses"][1] == "10.1.240.240-10.1.240.241"
+    assert call_obj["spec"]["addresses"][2] == "192.168.10.0/24"
+    assert call_obj["spec"]["addresses"][3] == "fc00:f853:0ccd:e799::/124"
 
     # test with multiple ranges with spaces thrown in
     lk_charm_client.reset_mock()
@@ -95,43 +101,39 @@ def test_config_change_updates_ip_pool(harness, lk_charm_client):
             "iprange": "  192. 168.1.240-192. 168.1.247, 10.1.240.240 -10.1.240.241,   192.168.10.0/24,fc00:f853:0ccd:e799::/124   "
         }
     )
-    harness.charm.on.config_changed.emit()
-    for call in lk_charm_client.apply.call_args_list:
-        call_obj = call.args[0].to_dict()
-        assert len(call_obj["spec"]["addresses"]) == 4
-        assert call_obj["spec"]["addresses"][0] == "192.168.1.240-192.168.1.247"
-        assert call_obj["spec"]["addresses"][1] == "10.1.240.240-10.1.240.241"
-        assert call_obj["spec"]["addresses"][2] == "192.168.10.0/24"
-        assert call_obj["spec"]["addresses"][3] == "fc00:f853:0ccd:e799::/124"
+    assert len(lk_charm_client.apply.call_args_list) == 2
+    apply_ip_pool_call = lk_charm_client.apply.call_args_list[0]
+    call_obj = apply_ip_pool_call.args[0].to_dict()
+    assert len(call_obj["spec"]["addresses"]) == 4
+    assert call_obj["spec"]["addresses"][0] == "192.168.1.240-192.168.1.247"
+    assert call_obj["spec"]["addresses"][1] == "10.1.240.240-10.1.240.241"
+    assert call_obj["spec"]["addresses"][2] == "192.168.10.0/24"
+    assert call_obj["spec"]["addresses"][3] == "fc00:f853:0ccd:e799::/124"
 
     # test with an empty range
     lk_charm_client.reset_mock()
     harness.update_config({"iprange": ""})
-    harness.charm.on.config_changed.emit()
     assert harness.charm.model.unit.status == BlockedStatus(
         "Invalid iprange: iprange must not be empty"
     )
 
     # test with an invalid range
     lk_charm_client.reset_mock()
     harness.update_config({"iprange": "256.256.256.256-256.256.256.256,10.1.240.240-10.1.240.241"})
-    harness.charm.on.config_changed.emit()
     assert harness.charm.model.unit.status == BlockedStatus(
         "Invalid iprange: 256.256.256.256-256.256.256.256 is not a valid CIDR or ip range"
     )
 
     # test with an invalid separator in range
     lk_charm_client.reset_mock()
     harness.update_config({"iprange": "10.1.240.240+10.1.240.241"})
-    harness.charm.on.config_changed.emit()
     assert harness.charm.model.unit.status == BlockedStatus(
         "Invalid iprange: 10.1.240.240+10.1.240.241 is not a valid CIDR or ip range"
     )
 
     # test with an invalid CIDR
     lk_charm_client.reset_mock()
     harness.update_config({"iprange": "256.256.256.256/24"})
-    harness.charm.on.config_changed.emit()
     assert harness.charm.model.unit.status == BlockedStatus(
         "Invalid iprange: 256.256.256.256/24 is not a valid CIDR or ip range"
     )