launchpad-reviewers team mailing list archive

[Quentin Debhi <mp+488593@xxxxxxxxxxxxxxxxxx>]

Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:native-pub-ensure-deps into launchpad:master.

Commit message:
Ensure dependencies at configuration time

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/488593
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:native-pub-ensure-deps into launchpad:master.

diff --git a/charm/launchpad-native-publisher/config.yaml b/charm/launchpad-native-publisher/config.yaml
index 6af814d..f691ce5 100644
--- a/charm/launchpad-native-publisher/config.yaml
+++ b/charm/launchpad-native-publisher/config.yaml
@@ -3,12 +3,12 @@ options:
     type: boolean
     description: Should the cron jobs and the celery services be active?
     default: true
-  rust-version:
+  rust_version:
     type: string
     default: "1.80"
     description: >
       Rust version to install, cargo will be installed with the same version.
-  java-version:
+  java_version:
     type: string
     default: "21"
     description: >
diff --git a/charm/launchpad-native-publisher/reactive/launchpad-native-publisher.py b/charm/launchpad-native-publisher/reactive/launchpad-native-publisher.py
index 63f7999..43171f4 100644
--- a/charm/launchpad-native-publisher/reactive/launchpad-native-publisher.py
+++ b/charm/launchpad-native-publisher/reactive/launchpad-native-publisher.py
@@ -15,7 +15,6 @@ from charms.launchpad.payload import (
 )
 from charms.reactive import (
     helpers,
-    hook,
     remove_state,
     set_state,
     when,
@@ -90,12 +89,147 @@ def perform_action_on_services(services, action):
         action(service)
 
 
+def check_binaries_exist():
+    """Check if required binaries exist and are executable."""
+    required_binaries = ["java", "mvn", "rustc", "cargo"]
+    missing = []
+
+    for binary in required_binaries:
+        try:
+            subprocess.check_call(
+                ["which", binary],
+                stderr=subprocess.DEVNULL,
+                stdout=subprocess.DEVNULL,
+            )
+            hookenv.log(f"{binary} is available")
+        except subprocess.CalledProcessError:
+            missing.append(binary)
+            hookenv.log(f"{binary} is missing")
+
+    return missing
+
+
+def install_dependencies():
+    """Install Java and Rust dependencies."""
+    config = hookenv.config()
+    java_version = config.get("java_version")
+    rust_version = config.get("rust_version")
+
+    # Check what's currently missing to give better logging
+    missing = check_binaries_exist()
+    missing_java = any(binary in missing for binary in ["java", "mvn"])
+    missing_rust = any(binary in missing for binary in ["rustc", "cargo"])
+
+    if missing_java and missing_rust:
+        hookenv.log(
+            f"Installing Java {java_version}/mvn &"
+            f"Rust {rust_version}/cargo..."
+        )
+    elif missing_java:
+        hookenv.log(f"Installing Java {java_version}/mvn...")
+    elif missing_rust:
+        hookenv.log(f"Installing Rust {rust_version}/cargo...")
+    else:
+        hookenv.log("Installing dependencies (some binaries missing)...")
+
+    try:
+        subprocess.check_call(["apt-get", "update"])
+
+        # Install Java and Maven
+        subprocess.check_call(
+            [
+                "apt-get",
+                "install",
+                "-y",
+                f"openjdk-{java_version}-jdk",
+                "maven",
+                "libmaven-deploy-plugin-java",
+            ]
+        )
+
+        # Install Rust and Cargo
+        subprocess.check_call(
+            [
+                "apt-get",
+                "install",
+                "-y",
+                f"rustc-{rust_version}",
+                f"cargo-{rust_version}",
+            ]
+        )
+
+        # Use update-alternatives to create generic command names
+        hookenv.log("Setting up alternatives for Rust and Cargo...")
+        subprocess.check_call(
+            [
+                "update-alternatives",
+                "--install",
+                "/usr/bin/rustc",
+                "rustc",
+                f"/usr/bin/rustc-{rust_version}",
+                "100",
+            ]
+        )
+        subprocess.check_call(
+            [
+                "update-alternatives",
+                "--install",
+                "/usr/bin/cargo",
+                "cargo",
+                f"/usr/bin/cargo-{rust_version}",
+                "100",
+            ]
+        )
+
+        # Verify installations
+        hookenv.log("Verifying installations...")
+        missing = check_binaries_exist()
+        if missing:
+            raise subprocess.CalledProcessError(
+                1, f"Missing binaries: {missing}"
+            )
+
+        hookenv.log("All dependencies successfully installed and verified")
+        return True
+
+    except subprocess.CalledProcessError as e:
+        hookenv.log(f"Failed to install dependencies: {e}", level="ERROR")
+        return False
+
+
+def ensure_dependencies():
+    """Ensure required dependencies are installed."""
+    missing = check_binaries_exist()
+
+    if missing:
+        hookenv.log(f"Missing binaries: {missing}")
+        if install_dependencies():
+            set_state("packages.installed")
+            remove_state("packages.failed")
+        else:
+            set_state("packages.failed")
+            remove_state("packages.installed")
+            return False
+    else:
+        hookenv.log("All required binaries are available")
+        set_state("packages.installed")
+        remove_state("packages.failed")
+
+    return True
+
+
 @when(
     "launchpad.db.configured",
 )
 @when_not("service.configured")
 def configure():
     """Configure the native publisher service."""
+
+    # Ensure dependencies are installed first
+    if not ensure_dependencies():
+        hookenv.status_set("blocked", "Failed to install required packages")
+        return
+
     config = get_service_config()
     try:
         if config.get("craftbuild_config"):
@@ -159,81 +293,3 @@ def check_is_running():
 )
 def deconfigure():
     remove_state("service.configured")
-
-
-@hook("install")
-def install_packages():
-    """Install Rust, Cargo, Java and Maven dependencies."""
-    hookenv.status_set("maintenance", "Installing packages")
-
-    # Get configuration values
-    config = hookenv.config()
-    rust_version = config.get("rust-version")
-    java_version = config.get("java-version")
-
-    subprocess.check_call(["apt-get", "update"])
-
-    hookenv.log(f"Installing Java {java_version} and Maven...")
-    subprocess.check_call(
-        [
-            "apt-get",
-            "install",
-            "-y",
-            f"openjdk-{java_version}-jdk",
-            "maven",
-            "libmaven-deploy-plugin-java",
-        ]
-    )
-
-    hookenv.log(f"Installing Rust {rust_version} and Cargo...")
-    subprocess.check_call(
-        [
-            "apt-get",
-            "install",
-            "-y",
-            f"rustc-{rust_version}",
-            f"cargo-{rust_version}",
-        ]
-    )
-
-    # Use update-alternatives to create generic command names
-    hookenv.log("Setting up alternatives for Rust and Cargo...")
-    subprocess.check_call(
-        [
-            "update-alternatives",
-            "--install",
-            "/usr/bin/rustc",
-            "rustc",
-            f"/usr/bin/rustc-{rust_version}",
-            "100",
-        ]
-    )
-    subprocess.check_call(
-        [
-            "update-alternatives",
-            "--install",
-            "/usr/bin/cargo",
-            "cargo",
-            f"/usr/bin/cargo-{rust_version}",
-            "100",
-        ]
-    )
-
-    # Verify the installations
-    hookenv.log("Verifying installations...")
-    try:
-        subprocess.check_call(["which", "java"])
-        subprocess.check_output(["java", "-version"], stderr=subprocess.STDOUT)
-        subprocess.check_call(["which", "mvn"])
-        subprocess.check_output(["mvn", "--version"])
-        subprocess.check_call(["which", "rustc"])
-        subprocess.check_output(["rustc", "--version"])
-        subprocess.check_call(["which", "cargo"])
-        subprocess.check_output(["cargo", "--version"])
-
-        hookenv.log("All packages successfully installed")
-        set_state("packages.installed")
-    except subprocess.CalledProcessError as e:
-        hookenv.log(f"Failed to verify installations: {str(e)}", level="ERROR")
-        set_state("packages.failed")
-        hookenv.status_set("blocked", "Failed to install packages")