Merge pull request #709 from kate-goldenring/fix-broker-envs-suffix

fix: only add device id to broker env vars suffix
project-akri · Oct 25, 2024 · 49eb225 · 49eb225
2 parents 66ce8ac + a85c02e
commit 49eb225
Show file tree

Hide file tree

Showing 8 changed files with 224 additions and 200 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -23,7 +23,7 @@ members = [
 resolver = "2"
 
 [workspace.package]
-version = "0.13.4"
+version = "0.13.5"
 edition = "2021"
 license = "Apache-2.0"
 homepage = "https://docs.akri.sh/"

diff --git a/agent/src/plugin_manager/device_plugin_instance_controller.rs b/agent/src/plugin_manager/device_plugin_instance_controller.rs
@@ -405,29 +405,31 @@ impl InternalDevicePlugin for InstanceDevicePlugin {
                         tonic::Status::unknown("Unable to claim slot")
                     })?;
             }
-            container_responses.push(cdi_device_to_car(&self.instance_name, &self.device));
+            container_responses.push(cdi_device_to_car(&self.device));
         }
         Ok(tonic::Response::new(AllocateResponse {
             container_responses,
         }))
     }
 }
 
-fn cdi_device_to_car(instance_name: &str, device: &cdi::Device) -> ContainerAllocateResponse {
+fn cdi_device_to_car(device: &cdi::Device) -> ContainerAllocateResponse {
+    // Device name is in the format akri.sh/c<config_name>-<instance-hash>
+    // Append deterministic instance hash to broker envs to avoid conflicts
+    let instance_hash = device
+        .name
+        .split('-')
+        .last()
+        .unwrap_or_default()
+        .to_uppercase();
     ContainerAllocateResponse {
         envs: device
             .container_edits
             .env
             .iter()
             .map(|e| match e.split_once('=') {
-                Some((k, v)) => (
-                    format!("{}_{}", k, instance_name.to_uppercase()),
-                    v.to_string(),
-                ),
-                None => (
-                    format!("{}_{}", e, instance_name.to_uppercase()),
-                    "".to_string(),
-                ),
+                Some((k, v)) => (format!("{}_{}", k, instance_hash), v.to_string()),
+                None => (format!("{}_{}", e, instance_hash), "".to_string()),
             })
             .collect(),
         mounts: device
@@ -672,7 +674,7 @@ impl InternalDevicePlugin for ConfigurationDevicePlugin {
                         .get(&dev)
                         .ok_or(tonic::Status::unknown("Invalid slot"))?
                         .clone();
-                    container_responses.push(cdi_device_to_car(&dp.instance_name, &dp.device));
+                    container_responses.push(cdi_device_to_car(&dp.device));
                     dp.claim_slot(
                         None,
                         DeviceUsage::Configuration {

diff --git a/deployment/helm/Chart.yaml b/deployment/helm/Chart.yaml
@@ -16,9 +16,9 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.13.4
+version: 0.13.5
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 0.13.4
+appVersion: 0.13.5
diff --git a/test/e2e/README.md b/test/e2e/README.md
@@ -0,0 +1,16 @@
+# Running E2E Tests Locally
+
+Reference the [Akri developer guide](https://docs.akri.sh/development/test-cases-workflow#run-the-tests-locally) for details on running locally.
+
+## Displaying Output
+By default, pytest captures output. If you always want to disable output capturing when running pytest through poetry, you can set the `PYTEST_ADDOPTS` environment variable:
+
+```sh
+PYTEST_ADDOPTS="-s" poetry run pytest -v | tee output.log
+```
+
+## Running a Specific Test
+A specific test can be executed by specifying its file and name like so:
+```
+poetry run pytest  test_core.py::test_device_offline -vvv
+```
diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py
@@ -71,7 +71,7 @@ def install_akri(request, pytestconfig, akri_version):
             helm_install_command.extend(
                 [
                     "--set",
-                    "agent.allowDebugEcho=true,debugEcho.configuration.shared=false",
+                    "agent.allowDebugEcho=true",
                 ]
             )
         if pytestconfig.getoption("--use-local"):

diff --git a/test/e2e/test_core.py b/test/e2e/test_core.py
@@ -52,7 +52,6 @@ def test_all_scheduled(akri_version, basic_config):
     assert_svc_present(basic_config, True, 2)
     assert_svc_present(basic_config, False, 1)
 
-
 def test_device_offline(akri_version, basic_config):
     # Check we are in sane setup
     assert_akri_instances_present(akri_version, basic_config, 2)
@@ -61,15 +60,35 @@ def test_device_offline(akri_version, basic_config):
     assert_svc_present(basic_config, False, 1)
 
     v1_core = kubernetes.client.CoreV1Api()
-    pods = v1_core.list_namespaced_pod(
+    broker_pods = v1_core.list_namespaced_pod(
         "default",
-        label_selector=f"app.kubernetes.io/name=akri-debug-echo-discovery",
+        label_selector="akri.sh/configuration=akri-debug-echo-foo",
         field_selector="status.phase=Running",
     ).items
     base_command = ["/bin/sh", "-c"]
+    for pod in broker_pods:
+        envs = kubernetes.stream.stream(
+            v1_core.connect_get_namespaced_pod_exec,
+            pod.metadata.name,
+            "default",
+            command="printenv",
+            stdout=True,
+            stdin=False,
+            stderr=False,
+            tty=False,
+        )
+        # TODO: Check env var suffix is as expected
+        if "DEBUG_ECHO_DESCRIPTION" not in envs:
+            raise AssertionError(f"DEBUG_ECHO_DESCRIPTION env var not found in broker")
+
+    dh_pods = v1_core.list_namespaced_pod(
+        "default",
+        label_selector=f"app.kubernetes.io/name=akri-debug-echo-discovery",
+        field_selector="status.phase=Running",
+    ).items
     command = "echo {} > /tmp/debug-echo-availability.txt"
     # Unplug the devices
-    for pod in pods:
+    for pod in dh_pods:
         kubernetes.stream.stream(
             v1_core.connect_get_namespaced_pod_exec,
             pod.metadata.name,
@@ -85,7 +104,7 @@ def test_device_offline(akri_version, basic_config):
     assert_svc_present(basic_config, True, 0)
     assert_svc_present(basic_config, False, 0)
     # Plug them back
-    for pod in pods:
+    for pod in dh_pods:
         kubernetes.stream.stream(
             v1_core.connect_get_namespaced_pod_exec,
             pod.metadata.name,
@@ -101,7 +120,6 @@ def test_device_offline(akri_version, basic_config):
     assert_svc_present(basic_config, True, 2)
     assert_svc_present(basic_config, False, 1)
 
-
 def test_cleanup(akri_version, faker):
     with open(Path(__file__).parent / "yaml/debugEchoConfiguration.yaml") as f:
         body = yaml.safe_load(f)

diff --git a/version.txt b/version.txt
@@ -1 +1 @@
-0.13.4
+0.13.5