Merge pull request #22 from wolsen/cleanup

Add unit tests and integration tests

.github/workflows/ci.yaml

@@ -63,3 +63,26 @@ jobs:
       - name: Run tests
         run: tox -e unit
 
+  integration-test:
+    strategy:
+      fail-fast: true
+      matrix:
+        bases:
+          - ubuntu@20.04
+          - ubuntu@22.04
+      name: Integration tests (LXD) | ${{ matrix.bases }}
+      runs-on: ubuntu-latest
+      needs:
+        - inclusive-naming-check
+        - lint
+        - unit-test
+      steps:
+        - name: Checkout
+          uses: actions/checkout@v3
+        - name: Setup operator environment
+          uses: charmed-kubernetes/actions-operator@main
+          with:
+            provider: lxd
+            juju-channel: 3/stable
+        - name: Run tests
+          run: tox run -e integration -- --charm-base=${{ matrix.bases }}

charmcraft.yaml

@@ -1,34 +1,60 @@
-# Learn more about charmcraft.yaml configuration at:
-# https://juju.is/docs/sdk/charmcraft-config
 type: "charm"
+name: lldpd
+title: LLDPd Operator
+summary: An operator that provides lldpd.
+description: |
+  An operator charm that provides the link-layer discover protocol (LLDP) services.
+  This operator installs and manages the lldpd package and services.
+
+  LLDP is a layer 2 neighbor discovery protocol that allows devices to advertise their
+  device information to their neighbors.
+
+  LLDP is generally disabled by default on linux bridges and may not allow for transmission
+  of LLDP packets by default. As such, the use of this LLDP in an environment configured
+  with linux bridging may need additional tweaks at the host level.
+
+requires:
+  juju-info:
+    interface: juju-info
+    scope: container
+  master:
+    interface: lldp
+provides:
+  nrpe-external-master:
+    interface: nrpe-external-master
+    scope: container
+subordinate: true
+
 bases:
   - build-on:
-    - name: "ubuntu"
-      channel: "22.04"
+      - name: "ubuntu"
+        channel: "22.04"
+        architectures: [amd64]
     run-on:
-    - name: "ubuntu"
-      channel: "22.04"
+      - name: "ubuntu"
+        channel: "22.04"
+        architectures: [amd64]
   - build-on:
-    - name: "ubuntu"
-      channel: "22.04"
+      - name: "ubuntu"
+        channel: "22.04"
+        architectures: [arm64]
     run-on:
-    - name: "ubuntu"
-      channel: "22.10"
+      - name: "ubuntu"
+        channel: "22.04"
+        architectures: [arm64]
   - build-on:
-    - name: "ubuntu"
-      channel: "22.04"
+      - name: "ubuntu"
+        channel: "20.04"
+        architectures: [amd64]
     run-on:
-    - name: "ubuntu"
-      channel: "23.04"
+      - name: "ubuntu"
+        channel: "20.04"
+        architectures: [amd64]
   - build-on:
-    - name: "ubuntu"
-      channel: "20.04"
+      - name: "ubuntu"
+        channel: "20.04"
+        architectures: [arm64]
     run-on:
-    - name: "ubuntu"
-      channel: "20.04"
-  - build-on:
-    - name: "ubuntu"
-      channel: "20.04"
-    run-on:
-    - name: "ubuntu"
-      channel: "18.04"
+      - name: "ubuntu"
+        channel: "20.04"
+        architectures: [arm64]

src/charm.py

@@ -80,27 +80,32 @@ def install(self):
         apt.update()
         apt.add_package(PACKAGES)
 
+    @property
+    def machine_id(self):
+        return os.environ.get("JUJU_MACHINE_ID", None)
+
     def configure(self):
         """Base config-changed hook."""
-        configs = self.model.config
-        if "i40e-lldp-stop" in configs:
+        config = self.model.config
+
+        # handle the side effects first
+        if config["i40e-lldp-stop"]:
             self.disable_i40e_lldp()
-        conf = open(PATHS["lldpddef"], "w")
-        args = ['DAEMON_ARGS="']
-        if configs.get("systemid-from-interface"):
-            args.append("-C {} ".format(str(configs["systemid-from-interface"])))
-        if configs.get("interfaces-regex"):
-            args.append("-I {} ".format(str(configs["interfaces-regex"])))
-        if configs.get("enable-snmp"):
-            args.append("-x ")
-        if configs.get("short-name"):
-            self.short_name()
-        machine_id = os.environ["JUJU_MACHINE_ID"]
-        if machine_id:
-            args.append("-S juju_machine_id={}".format(str(machine_id)))
-        args.append('"\n')
-        conf.write("".join(args))
-        conf.close()
+        if config["short-name"]:
+            self.update_short_name()
+
+        args = []
+        if config["systemid-from-interface"]:
+            args.append("-C {}".format(config["systemid-from-interface"]))
+        if config["interfaces-regex"]:
+            args.append("-I {}".format(config["interfaces-regex"]))
+        if config["enable-snmp"]:
+            args.append("-x")
+        if self.machine_id:
+            args.append("-S juju_machine_id={}".format(self.machine_id))
+
+        with open(PATHS["lldpddef"], "w") as conf:
+            conf.write('DAEMON_ARGS="{}"\n'.format(" ".join(args)))
         service_reload("lldpd", restart_on_failure=True)
         self.framework.model.unit.status = ActiveStatus("ready")
 
@@ -142,12 +147,11 @@ def i40e_filter(path: Path) -> bool:
                 check=True,
             )
 
-    def short_name(self):
+    def update_short_name(self):
         """Add system shortname to lldpd."""
         shortname = os.uname()[1]
-        cmd = open(PATHS["lldpdconf"], "w")
-        cmd.write("configure system hostname {}\n".format(str(shortname)))
-        cmd.close()
+        with open(PATHS["lldpdconf"], "w") as f:
+            f.write("configure system hostname {}\n".format(str(shortname)))
 
     def setup_nrpe(self):
         ## FIXME use ops-lib-nrpe

tests/integration/conftest.py

@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+# Copyright 2024 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Configure lldpd operator integration tests."""
+
+import logging
+import platform
+from pathlib import Path
+
+import pytest
+from pytest_operator.plugin import OpsTest
+
+
+logger = logging.getLogger(__name__)
+
+
+def pytest_addoption(parser) -> None:
+    parser.addoption(
+        "--charm-base",
+        action="store",
+        default="ubuntu@22.04",
+        help="Charm base version to use for integration tests",
+    )
+    parser.addoption(
+        "--enable-discovery",
+        action="store_true",
+        default=False,
+        help="Enables lldpd discovery tests. "
+        "May not succeed if using VMs or containers",
+    )
+
+
+@pytest.fixture(scope="module")
+def charm_base(request) -> str:
+    """Get the lldp charm base to use."""
+    return request.config.option.charm_base
+
+
+@pytest.fixture(scope="module")
+async def lldpd_charm(ops_test: OpsTest, charm_base: str) -> Path:
+    # Multiple charms will be built, but the build_charm function only returns
+    # the path to one of the charms. Find the charm that matches the charm_base
+    # in order to test the right one.
+    await ops_test.build_charm(".")
+
+    base = charm_base.replace("@", "-")
+    arch = platform.machine()
+    # convert the x86_64 arch into the amd64 arch used by charmcraft.
+    if arch == "x86_64":
+        arch = "amd64"
+
+    build_dir = (ops_test.tmp_path / "charms").absolute()
+    charm_file = build_dir / f"lldpd_{base}-{arch}.charm"
+    if not charm_file.exists():
+        raise ValueError(f"Unable to find charm file {charm_file}")
+
+    return charm_file

tests/integration/test_charm.py

@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+# Copyright 2024 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Test lldpd charm deployment."""
+
+import asyncio
+import logging
+import pytest
+
+from pytest_operator.plugin import OpsTest
+
+
+logger = logging.getLogger(__name__)
+
+NUM_UNITS = 2
+
+
+@pytest.mark.abort_on_fail
+@pytest.mark.skip_if_deployed
+@pytest.mark.order(1)
+async def test_build_and_deploy(
+    ops_test: OpsTest, charm_base: str, lldpd_charm
+) -> None:
+    """Test the lldpd charm builds and deploys."""
+    logger.info(f"Building and deploying lldp charms for base: {charm_base}")
+    lldpd = await lldpd_charm
+
+    logger.info(f"lldpd charm is located at: {lldpd}")
+
+    # Deploy ubuntu and lldpd charms.
+    await asyncio.gather(
+        ops_test.model.deploy(
+            "ubuntu",
+            application_name="ubuntu",
+            num_units=NUM_UNITS,
+            base=charm_base,
+        ),
+        ops_test.model.deploy(
+            str(lldpd),
+            application_name="lldpd",
+            num_units=0,
+            base=charm_base,
+        ),
+    )
+
+    # Integrate lldpd with ubuntu
+    await ops_test.model.integrate("ubuntu:juju-info", "lldpd:juju-info")
+    async with ops_test.fast_forward():
+        await ops_test.model.wait_for_idle(
+            apps=["lldpd", "ubuntu"], status="active", timeout=1800
+        )
+        for unit in range(NUM_UNITS):
+            uname = f"lldpd/{unit}"
+            assert ops_test.model.units.get(uname).workload_status == "active"
+
+
+@pytest.mark.order(2)
+async def test_lldpd_is_active(ops_test: OpsTest) -> None:
+    """Test that the lldpd services are active in each juju unit."""
+    logger.info("Validating that lldpd is active inside each juju unit.")
+    for unit in ops_test.model.applications["lldpd"].units:
+        status = (await unit.ssh("systemctl is-active lldpd")).strip()
+        assert status == "active", f"{unit.name} lldpd is not active"