fix: allow multiple apps to write safely to etc/environment (#95)

src/config.py

@@ -21,7 +21,7 @@
     AuthMechanism,
     Scope,
 )
-from utils import safe_write_to_file
+from utils import map_env, safe_write_to_file, update_env
 
 if TYPE_CHECKING:
     from charm import KafkaCharm
@@ -510,10 +510,9 @@ def set_client_properties(self) -> None:
 
     def set_environment(self) -> None:
         """Writes the env-vars needed for passing to charmed-kafka service."""
-        environment = f"{self.kafka_opts}\n" f"{self.jmx_opts}\n" f"{self.log4j_opts}\n"
-
-        safe_write_to_file(
-            content=environment,
-            path="/etc/environment",
-            mode="a",
-        )
+        updated_env_list = [
+            self.kafka_opts,
+            self.jmx_opts,
+            self.log4j_opts,
+        ]
+        update_env(env=map_env(env=updated_env_list))

src/utils.py

@@ -132,3 +132,36 @@ def parse_tls_file(raw_content: str) -> str:
     if re.match(r"(-+(BEGIN|END) [A-Z ]+-+)", raw_content):
         return raw_content
     return base64.b64decode(raw_content).decode("utf-8")
+
+
+def map_env(env: list[str]) -> dict[str, str]:
+    """Builds environment map for arbitrary env-var strings.
+
+    Returns:
+        Dict of env-var and value
+    """
+    map_env = {}
+    for var in env:
+        key = "".join(var.split("=", maxsplit=1)[0])
+        value = "".join(var.split("=", maxsplit=1)[1:])
+
+        map_env[key] = value
+
+    return map_env
+
+
+def get_env() -> dict[str, str]:
+    """Builds map of current basic environment for all processes.
+
+    Returns:
+        Dict of env-var and value
+    """
+    raw_env = safe_get_file("/etc/environment") or []
+    return map_env(env=raw_env)
+
+
+def update_env(env: dict[str, str]) -> None:
+    """Updates /etc/environment file."""
+    updated_env = get_env() | env
+    content = "\n".join([f"{key}={value}" for key, value in updated_env.items()])
+    safe_write_to_file(content=content, path="/etc/environment", mode="w")

tests/integration/test_charm.py

@@ -27,6 +27,49 @@
 logger = logging.getLogger(__name__)
 
 DUMMY_NAME = "app"
+SAME_ZK = f"{ZK_NAME}-same"
+SAME_KAFKA = f"{APP_NAME}-same"
+
+
+@pytest.mark.abort_on_fail
+async def test_build_and_deploy_same_machine(ops_test: OpsTest, kafka_charm):
+    # deploying 1 machine
+    await ops_test.model.add_machine(series="jammy")
+    machine_ids = await ops_test.model.get_machines()
+
+    # deploying both kafka + zk to same machine
+    await asyncio.gather(
+        ops_test.model.deploy(
+            ZK_NAME,
+            channel="edge",
+            application_name=SAME_ZK,
+            num_units=1,
+            series="jammy",
+            to=machine_ids[0],
+        ),
+        ops_test.model.deploy(
+            kafka_charm,
+            application_name=SAME_KAFKA,
+            num_units=1,
+            series="jammy",
+            to=machine_ids[0],
+        ),
+    )
+    await ops_test.model.wait_for_idle(apps=[SAME_ZK, SAME_KAFKA], idle_period=30, timeout=3600)
+    assert ops_test.model.applications[SAME_KAFKA].status == "blocked"
+    assert ops_test.model.applications[SAME_ZK].status == "active"
+
+    await ops_test.model.add_relation(SAME_KAFKA, SAME_ZK)
+    async with ops_test.fast_forward():
+        await ops_test.model.wait_for_idle(apps=[SAME_ZK, SAME_KAFKA], idle_period=30)
+        assert ops_test.model.applications[SAME_ZK].status == "active"
+        assert ops_test.model.applications[SAME_KAFKA].status == "active"
+
+    await asyncio.gather(
+        ops_test.model.applications[SAME_KAFKA].remove(force=True, no_wait=True),
+        ops_test.model.applications[SAME_ZK].remove(force=True, no_wait=True),
+    )
+    await ops_test.model.machines[machine_ids[0]].destroy()
 
 
 @pytest.mark.abort_on_fail

tests/unit/test_utils.py

@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+from unittest.mock import patch
+
+from src.utils import map_env
+
+from utils import get_env, update_env
+
+
+def test_map_env_populated():
+    example_env = [
+        "KAFKA_OPTS=orcs -Djava=wargs -Dkafka=goblins",
+        "SERVER_JVMFLAGS=dwarves -Djava=elves -Dzookeeper=men",
+    ]
+    env = map_env(env=example_env)
+
+    assert len(env) == 2
+    assert sorted(env.keys()) == sorted(["KAFKA_OPTS", "SERVER_JVMFLAGS"])
+
+    for value in env.values():
+        assert type(value) == str
+        # checks handles multiple equals signs in value
+        assert len(value.split()) == 3
+
+
+def test_get_env_empty():
+    with patch("utils.safe_get_file", return_value=[]):
+        assert not get_env()
+        assert get_env() == {}
+
+
+def test_update_env():
+    example_get_env = {
+        "KAFKA_OPTS": "orcs -Djava=wargs -Dkafka=goblins",
+        "SERVER_JVMFLAGS": "dwarves -Djava=elves -Dzookeeper=men",
+    }
+    example_update_env = {
+        "SERVER_JVMFLAGS": "gimli -Djava=legolas -Dzookeeper=aragorn",
+    }
+
+    with (
+        patch("utils.get_env", return_value=example_get_env),
+        patch("utils.safe_write_to_file") as safe_write,
+    ):
+        update_env(env=example_update_env)
+
+        assert all(
+            updated in safe_write.call_args.kwargs["content"]
+            for updated in ["gimli", "legolas", "aragorn"]
+        )
+        assert "KAFKA_OPTS" in safe_write.call_args.kwargs["content"]
+        assert safe_write.call_args.kwargs["path"] == "/etc/environment"
+        assert safe_write.call_args.kwargs["mode"] == "w"