From 8d14fcfd90355db2eb99191c238a378643e59da9 Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:43:16 -0500 Subject: [PATCH] fix: repair one-click deploy paths and production manifests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/deployment/one-click-deploy.sh` could not complete. It aborted at its first gate because `REQUIRED_FILES` named paths that had moved under `infrastructure/` plus two modules that never existed (`mcp_server.py`, `learning_app_processor.py`). Past that gate it would have failed again: every `docker build -f` and `kubectl apply -f` target was also stale. Fixing only the paths would have made a second, worse failure newly reachable. `kubectl apply -f k8s/production/` also creates an `mcp-server` Deployment that mounts ConfigMap `mcp-server-code` to run `/app/mcp_server.py`. Neither exists anywhere in the repository: $ grep -rn "mcp-server-code" . infrastructure/k8s/production/deployment.yaml:151 # the reference itself That pod can never become ready, so `kubectl rollout status` could only time out. It is removed rather than left to CrashLoop. The `enhanced-framework` container was also described as a Node service on port 3000 with readiness `/ready`, but `enhanced-framework:latest` is built from `infrastructure/docker/Dockerfile.production` — a Python/uvicorn image that `EXPOSE`s 8000 and serves `/health` and `/readyz`. There is no `/ready` route, so readiness could never pass. The manifest was the stale side. Changes: - Anchor the script to the repository root so paths no longer depend on the caller's working directory. - Point `REQUIRED_FILES`, `docker build` and all `kubectl apply` invocations at the real `infrastructure/` locations. - Run integration tests via pytest against `tests/integration`, skipping explicitly when pytest is unavailable instead of silently passing. - Retarget the Deployment, both Services and the Prometheus scrape config to port 8000 / `/readyz`, and drop the phantom `mcp-server` resources. - Add `tests/unit/test_deployment_manifests.py`, which derives its expectations from the repository: every script path must resolve, every mounted ConfigMap must be defined, every Service `targetPort` must be a declared `containerPort`, the container port must match the image's `EXPOSE`, and every probe path must be a route in `main.py`. The guard was mutation-tested against all five original defects and catches each one. Fixes #1127 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- infrastructure/k8s/monitoring/monitoring.yaml | 6 - infrastructure/k8s/production/deployment.yaml | 97 +------ infrastructure/k8s/production/service.yaml | 24 +- scripts/deployment/one-click-deploy.sh | 46 ++-- tests/unit/test_deployment_manifests.py | 249 ++++++++++++++++++ 5 files changed, 287 insertions(+), 135 deletions(-) create mode 100644 tests/unit/test_deployment_manifests.py diff --git a/infrastructure/k8s/monitoring/monitoring.yaml b/infrastructure/k8s/monitoring/monitoring.yaml index 30eb3eb24..e97665093 100644 --- a/infrastructure/k8s/monitoring/monitoring.yaml +++ b/infrastructure/k8s/monitoring/monitoring.yaml @@ -16,12 +16,6 @@ data: metrics_path: /metrics scrape_interval: 10s - - job_name: 'mcp-server' - static_configs: - - targets: ['mcp-server:8000'] - metrics_path: /metrics - scrape_interval: 10s - - job_name: 'kubernetes-pods' kubernetes_sd_configs: - role: pod diff --git a/infrastructure/k8s/production/deployment.yaml b/infrastructure/k8s/production/deployment.yaml index 517fd81e7..c09d569b8 100644 --- a/infrastructure/k8s/production/deployment.yaml +++ b/infrastructure/k8s/production/deployment.yaml @@ -29,15 +29,13 @@ spec: image: enhanced-framework:latest imagePullPolicy: Always ports: - - containerPort: 3000 + - containerPort: 8000 name: http env: - - name: NODE_ENV - value: "production" - name: PORT - value: "3000" - - name: MCP_SERVER_URL - value: "http://mcp-server:8000" + value: "8000" + - name: PYTHONUNBUFFERED + value: "1" resources: requests: memory: "256Mi" @@ -45,94 +43,17 @@ spec: limits: memory: "512Mi" cpu: "500m" - livenessProbe: - httpGet: - path: /health - port: 3000 - initialDelaySeconds: 30 - periodSeconds: 10 - timeoutSeconds: 5 - failureThreshold: 3 - readinessProbe: - httpGet: - path: /ready - port: 3000 - initialDelaySeconds: 5 - periodSeconds: 5 - timeoutSeconds: 3 - failureThreshold: 3 - securityContext: - allowPrivilegeEscalation: false - runAsNonRoot: true - runAsUser: 1001 - capabilities: - drop: - - ALL - securityContext: - fsGroup: 1001 - restartPolicy: Always ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - name: mcp-server - namespace: production - labels: - app: mcp-server - version: v1.0.0 - component: mcp-protocol -spec: - replicas: 2 - strategy: - type: RollingUpdate - rollingUpdate: - maxSurge: 1 - maxUnavailable: 0 - selector: - matchLabels: - app: mcp-server - template: - metadata: - labels: - app: mcp-server - version: v1.0.0 - component: mcp-protocol - spec: - containers: - - name: mcp-server - image: python:3.11-slim - command: ["python3", "/app/mcp_server.py"] - ports: - - containerPort: 8000 - name: mcp-http - env: - - name: MCP_HOST - value: "0.0.0.0" - - name: MCP_PORT - value: "8000" - - name: MCP_DEBUG - value: "false" - resources: - requests: - memory: "128Mi" - cpu: "100m" - limits: - memory: "256Mi" - cpu: "200m" - volumeMounts: - - name: app-code - mountPath: /app livenessProbe: httpGet: path: /health port: 8000 - initialDelaySeconds: 15 + initialDelaySeconds: 30 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3 readinessProbe: httpGet: - path: /ready + path: /readyz port: 8000 initialDelaySeconds: 5 periodSeconds: 5 @@ -145,10 +66,6 @@ spec: capabilities: drop: - ALL - volumes: - - name: app-code - configMap: - name: mcp-server-code securityContext: fsGroup: 1001 - restartPolicy: Always \ No newline at end of file + restartPolicy: Always diff --git a/infrastructure/k8s/production/service.yaml b/infrastructure/k8s/production/service.yaml index 9f3666748..fd4ab938a 100644 --- a/infrastructure/k8s/production/service.yaml +++ b/infrastructure/k8s/production/service.yaml @@ -10,7 +10,7 @@ spec: type: ClusterIP ports: - port: 80 - targetPort: 3000 + targetPort: 8000 protocol: TCP name: http selector: @@ -18,24 +18,6 @@ spec: --- apiVersion: v1 kind: Service -metadata: - name: mcp-server - namespace: production - labels: - app: mcp-server - component: mcp-protocol -spec: - type: ClusterIP - ports: - - port: 8000 - targetPort: 8000 - protocol: TCP - name: mcp-http - selector: - app: mcp-server ---- -apiVersion: v1 -kind: Service metadata: name: enhanced-framework-lb namespace: production @@ -46,11 +28,11 @@ spec: type: LoadBalancer ports: - port: 80 - targetPort: 3000 + targetPort: 8000 protocol: TCP name: http - port: 443 - targetPort: 3000 + targetPort: 8000 protocol: TCP name: https selector: diff --git a/scripts/deployment/one-click-deploy.sh b/scripts/deployment/one-click-deploy.sh index c8b7c77e1..974bcb78f 100755 --- a/scripts/deployment/one-click-deploy.sh +++ b/scripts/deployment/one-click-deploy.sh @@ -17,6 +17,14 @@ NAMESPACE="production" DEPLOYMENT_NAME="enhanced-framework" TIMEOUT=300 +# Every path below is relative to the repository root, so anchor there rather +# than depending on the caller's working directory. +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +DOCKERFILE="infrastructure/docker/Dockerfile.production" +PRODUCTION_MANIFESTS="infrastructure/k8s/production" +MONITORING_MANIFESTS="infrastructure/k8s/monitoring" +cd "$REPO_ROOT" + # Logging function log() { echo -e "${BLUE}[$(date +'%Y-%m-%d %H:%M:%S')]${NC} $1" @@ -65,13 +73,11 @@ success "Kubernetes cluster is accessible" # Check if required files exist REQUIRED_FILES=( - "Dockerfile.production" + "$DOCKERFILE" "package.json" - "k8s/production/deployment.yaml" - "k8s/production/service.yaml" - "k8s/monitoring/monitoring.yaml" - "mcp_server.py" - "learning_app_processor.py" + "$PRODUCTION_MANIFESTS/deployment.yaml" + "$PRODUCTION_MANIFESTS/service.yaml" + "$MONITORING_MANIFESTS/monitoring.yaml" ) for file in "${REQUIRED_FILES[@]}"; do @@ -85,14 +91,18 @@ success "All required files are present" log "Step 2: Running Integration Tests" if [[ -d "venv" ]]; then + # shellcheck disable=SC1091 source venv/bin/activate - if python3 tests/integration/test_runner.py; then - success "Integration tests passed" - else - error "Integration tests failed. Please fix issues before deployment." - fi +fi + +if ! command -v python3 &> /dev/null; then + warning "python3 is not available. Skipping integration tests." +elif ! python3 -c "import pytest" &> /dev/null; then + warning "pytest is not installed. Skipping integration tests." +elif PYTHONPATH="src" python3 -m pytest tests/integration -q; then + success "Integration tests passed" else - warning "Virtual environment not found. Skipping integration tests." + error "Integration tests failed. Please fix issues before deployment." fi # Step 3: Build Docker Image @@ -101,7 +111,7 @@ log "Step 3: Building Docker Image" DOCKER_TAG="enhanced-framework:$(date +%Y%m%d-%H%M%S)" LATEST_TAG="enhanced-framework:latest" -if docker build -f Dockerfile.production -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then +if docker build -f "$DOCKERFILE" -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then success "Docker image built successfully: $DOCKER_TAG" else error "Docker image build failed" @@ -111,21 +121,21 @@ fi log "Step 4: Validating Kubernetes Manifests" # Validate deployment manifest -if kubectl apply --dry-run=client -f k8s/production/deployment.yaml; then +if kubectl apply --dry-run=client -f "$PRODUCTION_MANIFESTS/deployment.yaml"; then success "Deployment manifest is valid" else error "Deployment manifest validation failed" fi # Validate service manifest -if kubectl apply --dry-run=client -f k8s/production/service.yaml; then +if kubectl apply --dry-run=client -f "$PRODUCTION_MANIFESTS/service.yaml"; then success "Service manifest is valid" else error "Service manifest validation failed" fi # Validate monitoring manifest -if kubectl apply --dry-run=client -f k8s/monitoring/monitoring.yaml; then +if kubectl apply --dry-run=client -f "$MONITORING_MANIFESTS/monitoring.yaml"; then success "Monitoring manifest is valid" else error "Monitoring manifest validation failed" @@ -144,14 +154,14 @@ fi log "Step 6: Deploying to Kubernetes" # Deploy application -if kubectl apply -f k8s/production/ -n "$NAMESPACE"; then +if kubectl apply -f "$PRODUCTION_MANIFESTS/" -n "$NAMESPACE"; then success "Application deployed successfully" else error "Application deployment failed" fi # Deploy monitoring -if kubectl apply -f k8s/monitoring/ -n "$NAMESPACE"; then +if kubectl apply -f "$MONITORING_MANIFESTS/" -n "$NAMESPACE"; then success "Monitoring stack deployed successfully" else error "Monitoring deployment failed" diff --git a/tests/unit/test_deployment_manifests.py b/tests/unit/test_deployment_manifests.py new file mode 100644 index 000000000..2d58806a0 --- /dev/null +++ b/tests/unit/test_deployment_manifests.py @@ -0,0 +1,249 @@ +"""Structural guards for the one-click deployment path. + +``scripts/deployment/one-click-deploy.sh`` aborted at its first gate because +``REQUIRED_FILES`` named paths that had moved under ``infrastructure/`` and two +modules that never existed. Once past that gate it would have rolled out a +manifest describing the Python image as a Node service on port 3000, and a +sibling Deployment mounting a ConfigMap that is defined nowhere in the +repository. + +Every assertion here is derived from the repository rather than restated by +hand, so the checks keep tracking the deployment as it moves. +""" + +import re +import unittest +from pathlib import Path + +import yaml + + +def _repo_root(): + for candidate in Path(__file__).resolve().parents: + if (candidate / "scripts" / "deployment" / "one-click-deploy.sh").exists(): + return candidate + raise AssertionError("repository root not found") + + +REPO_ROOT = _repo_root() +DEPLOY_SCRIPT = REPO_ROOT / "scripts" / "deployment" / "one-click-deploy.sh" +PRODUCTION_DIR = REPO_ROOT / "infrastructure" / "k8s" / "production" +MONITORING_DIR = REPO_ROOT / "infrastructure" / "k8s" / "monitoring" + + +def _script(): + return DEPLOY_SCRIPT.read_text(encoding="utf-8") + + +def _script_variables(script): + """Collect the literal top-level assignments the script's paths are built from.""" + + variables = {} + for name, value in re.findall( + r'^([A-Z_]+)="([^"$]*)"$', script, flags=re.MULTILINE + ): + variables[name] = value + return variables + + +def _expand(value, variables): + def replace(match): + name = match.group(1) or match.group(2) + if name not in variables: + raise AssertionError(f"unresolved shell variable in path: ${name}") + return variables[name] + + return re.sub(r'\$\{([A-Z_]+)\}|\$([A-Z_]+)', replace, value) + + +def _manifests(directory): + for path in sorted(directory.glob("*.yaml")): + for document in yaml.safe_load_all(path.read_text(encoding="utf-8")): + if document: + yield path, document + + +class DeployScriptPathTests(unittest.TestCase): + def test_required_files_all_exist(self): + script = _script() + variables = _script_variables(script) + block = re.search( + r"REQUIRED_FILES=\(\s*(.*?)\s*\)", script, flags=re.DOTALL + ) + self.assertIsNotNone(block, "REQUIRED_FILES array not found") + + entries = re.findall(r'"([^"]+)"', block.group(1)) + self.assertGreater(len(entries), 0) + + for entry in entries: + with self.subTest(entry=entry): + resolved = REPO_ROOT / _expand(entry, variables) + self.assertTrue( + resolved.is_file(), + f"REQUIRED_FILES names {entry}, which does not exist", + ) + + def test_every_referenced_path_exists(self): + """`docker build -f` and `kubectl apply -f` targets must be real.""" + + script = _script() + variables = _script_variables(script) + referenced = re.findall( + r'(?:docker build|kubectl apply)[^\n]*?-f "([^"]+)"', script + ) + self.assertGreater(len(referenced), 0) + + for entry in referenced: + if entry == "-": # `kubectl apply -f -` reads stdin. + continue + with self.subTest(entry=entry): + resolved = REPO_ROOT / _expand(entry, variables).rstrip("/") + self.assertTrue( + resolved.exists(), + f"script applies {entry}, which does not exist", + ) + + def test_script_runs_from_the_repository_root(self): + """Relative paths are only meaningful once the script anchors itself.""" + + script = _script() + self.assertIn('REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")', script) + self.assertIn('cd "$REPO_ROOT"', script) + self.assertLess( + script.index('cd "$REPO_ROOT"'), + script.index("REQUIRED_FILES=("), + ) + + +class ProductionManifestTests(unittest.TestCase): + def _documents(self): + return [document for _, document in _manifests(PRODUCTION_DIR)] + + def _deployments(self): + return [d for d in self._documents() if d.get("kind") == "Deployment"] + + def _services(self): + return [d for d in self._documents() if d.get("kind") == "Service"] + + def test_mounted_configmaps_are_defined_in_the_repository(self): + """A pod mounting an undefined ConfigMap can never start. + + The removed `mcp-server` Deployment mounted `mcp-server-code`, which no + manifest creates, so `kubectl rollout status` could only ever time out. + """ + + defined = set() + for directory in (PRODUCTION_DIR, MONITORING_DIR): + for _, document in _manifests(directory): + if document.get("kind") == "ConfigMap": + defined.add(document["metadata"]["name"]) + + for path, document in _manifests(PRODUCTION_DIR): + if document.get("kind") != "Deployment": + continue + volumes = ( + document["spec"]["template"]["spec"].get("volumes") or [] + ) + for volume in volumes: + config_map = volume.get("configMap") + if not config_map: + continue + with self.subTest(path=path.name, name=config_map["name"]): + self.assertIn( + config_map["name"], + defined, + f"{path.name} mounts ConfigMap " + f"{config_map['name']}, which is defined nowhere", + ) + + def test_service_target_ports_match_a_container_port(self): + """Traffic sent to a port no container listens on is silently dropped.""" + + container_ports = {} + for deployment in self._deployments(): + app = deployment["spec"]["selector"]["matchLabels"]["app"] + ports = set() + for container in deployment["spec"]["template"]["spec"]["containers"]: + for port in container.get("ports") or []: + ports.add(port["containerPort"]) + container_ports[app] = ports + + for service in self._services(): + app = service["spec"]["selector"]["app"] + with self.subTest(service=service["metadata"]["name"]): + self.assertIn( + app, + container_ports, + f"service selects app={app}, which no Deployment provides", + ) + for port in service["spec"]["ports"]: + self.assertIn( + port["targetPort"], + container_ports[app], + f"targetPort {port['targetPort']} is not exposed by " + f"any {app} container", + ) + + def test_container_port_matches_the_image_it_runs(self): + """The manifest and the Dockerfile must agree on the listening port.""" + + dockerfile = ( + REPO_ROOT / "infrastructure" / "docker" / "Dockerfile.production" + ).read_text(encoding="utf-8") + exposed = { + int(port) + for port in re.findall(r"^EXPOSE\s+(\d+)", dockerfile, re.MULTILINE) + } + self.assertTrue(exposed, "Dockerfile.production declares no EXPOSE") + + for deployment in self._deployments(): + if deployment["metadata"]["name"] != "enhanced-framework": + continue + for container in deployment["spec"]["template"]["spec"]["containers"]: + for port in container.get("ports") or []: + self.assertIn( + port["containerPort"], + exposed, + "enhanced-framework listens on " + f"{port['containerPort']} but the image exposes " + f"{sorted(exposed)}", + ) + + def test_probe_paths_are_served_by_the_application(self): + """A probe on an unrouted path fails forever and blocks the rollout.""" + + main = ( + REPO_ROOT / "src" / "youtube_extension" / "main.py" + ).read_text(encoding="utf-8") + routes = set(re.findall(r'@app\.get\(\s*"([^"]+)"', main)) + self.assertIn("/health", routes, "sanity: /health route not found") + + for deployment in self._deployments(): + if deployment["metadata"]["name"] != "enhanced-framework": + continue + for container in deployment["spec"]["template"]["spec"]["containers"]: + for probe in ("livenessProbe", "readinessProbe"): + spec = container.get(probe) + if not spec or "httpGet" not in spec: + continue + with self.subTest(probe=probe): + self.assertIn( + spec["httpGet"]["path"], + routes, + f"{probe} targets {spec['httpGet']['path']}, " + "which the application does not route", + ) + + def test_no_manifest_references_a_removed_service(self): + """Nothing should point at the phantom MCP server that was removed.""" + + for directory in (PRODUCTION_DIR, MONITORING_DIR): + for path in sorted(directory.glob("*.yaml")): + with self.subTest(path=path.name): + self.assertNotIn( + "mcp-server", path.read_text(encoding="utf-8") + ) + + +if __name__ == "__main__": + unittest.main()