diff --git a/backend/openapi.json b/backend/openapi.json index 8ebd672..232b2dc 100644 --- a/backend/openapi.json +++ b/backend/openapi.json @@ -1299,6 +1299,49 @@ } } }, + "/api/k8s/clusters/{cluster_id}/resync": { + "post": { + "tags": [ + "k8s-clusters" + ], + "summary": "Resync Cluster", + "description": "Force a fresh inventory sync for a cluster (owner or admin only).\n\nIssue #194: an explicit, documented rescan trigger. Operators previously\nrelied on a no-op ``PUT`` to force a refresh; this endpoint makes that\nintent first-class and reliable. It enqueues a background scan (the same\ntask registration/PUT use) and returns immediately \u2014 the scan stamps\n``last_synced_at`` on completion. An unknown cluster 404s via the\n``require_cluster_owner`` dependency before this body runs, so there is no\nsilently-swallowed background no-op.", + "operationId": "resync_cluster_api_k8s_clusters__cluster_id__resync_post", + "parameters": [ + { + "name": "cluster_id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "title": "Cluster Id" + } + } + ], + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ClusterOperationResponse" + } + } + } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } + } + } + } + }, "/api/k8s/clusters/{cluster_id}/refresh-kubeconfig": { "post": { "tags": [ diff --git a/backend/routes/k8s/clusters.py b/backend/routes/k8s/clusters.py index 3211514..8527159 100644 --- a/backend/routes/k8s/clusters.py +++ b/backend/routes/k8s/clusters.py @@ -113,6 +113,27 @@ def update_cluster(cluster_id: int, cluster_data: ClusterUpdateRequest, user: Us return result +@router.post("/k8s/clusters/{cluster_id}/resync", response_model=ClusterOperationResponse) +@handle_route_errors("resync cluster") +def resync_cluster(cluster_id: int, user: User = Depends(require_cluster_owner), db: Session = Depends(get_db)): + """Force a fresh inventory sync for a cluster (owner or admin only). + + Issue #194: an explicit, documented rescan trigger. Operators previously + relied on a no-op ``PUT`` to force a refresh; this endpoint makes that + intent first-class and reliable. It enqueues a background scan (the same + task registration/PUT use) and returns immediately — the scan stamps + ``last_synced_at`` on completion. An unknown cluster 404s via the + ``require_cluster_owner`` dependency before this body runs, so there is no + silently-swallowed background no-op. + """ + enqueue_cluster_scan(cluster_id) + return { + "success": True, + "message": "Inventory sync enqueued", + "cluster_id": cluster_id, + } + + @router.delete("/k8s/clusters/{cluster_id}", response_model=ClusterOperationResponse) @handle_route_errors("delete cluster") def delete_cluster(cluster_id: int, user: User = Depends(require_cluster_owner), db: Session = Depends(get_db)): diff --git a/backend/services/scanner/__init__.py b/backend/services/scanner/__init__.py index e65082d..04ccbe5 100644 --- a/backend/services/scanner/__init__.py +++ b/backend/services/scanner/__init__.py @@ -227,6 +227,16 @@ def scan(self, cluster_id: int) -> dict[str, Any]: end_time = datetime.now(UTC) duration_ms = int((end_time - start_time).total_seconds() * 1000) + # Issue #194: record when this cluster was last successfully scanned so + # "never scanned" (last_synced_at IS NULL) is distinguishable from + # "scanned and genuinely empty". Set only after all analysis has + # completed — a scan that raises earlier must NOT stamp a sync time. + # Every scan path (registration/PUT async task, the /scan endpoint, + # upgrade pre-checks) flows through here, so this is the single place + # that keeps last_synced_at honest. Flushed here; the caller commits. + cluster.last_synced_at = end_time + self.db.flush() + return { "cluster_id": cluster_id, "cluster_name": cluster.name, diff --git a/backend/tests/component/test_cluster_inventory_sync.py b/backend/tests/component/test_cluster_inventory_sync.py new file mode 100644 index 0000000..db2fe41 --- /dev/null +++ b/backend/tests/component/test_cluster_inventory_sync.py @@ -0,0 +1,166 @@ +""" +Issue #194: Cluster inventory sync — last_synced_at + pod inventory. + +Two defects are locked here: + +1. ClusterScanner.scan() never wrote ``last_synced_at``, so a registered + cluster stayed "never synced" forever (NULL) even after the scan ran and + after repeated no-op PUTs. These tests prove the scan now stamps + ``last_synced_at`` on completion, that the stamp is only written on success + (a scan that raises early must NOT stamp), and that it persists across a + commit (the async registration/PUT task path). + +2. Over a fetch that surfaces Multus pods in a namespace the scan actually reads + (kube-system), real analyze_multus counts them (not 0) and the scan records + ``last_synced_at`` — so "genuinely empty" is decidable from "never scanned". + NOTE: the reporter's own "0 Multus pods on OpenShift" was a SEPARATE, + pre-existing namespace-scoping gap — Multus runs in ``openshift-multus``, + which the pod fetch never queries (tracked in #202) — retracted by the + reporter; this PR does not fix or claim to fix that symptom. +""" + +import contextlib +from datetime import datetime +from unittest.mock import MagicMock, patch + +_EMPTY_FETCH_DATA = { + "version_info": {}, "nodes": [], "namespaces": [], "crds": [], + "crd_names": set(), "crd_groups": set(), "cert_manager_pods": [], + "helm_releases": [], "kube_system_pods": [], "daemonsets": [], + "storage_classes": [], "gateways": [], "gatewayclasses": [], + "f5_tenant_pods": [], "f5_utils_pods": [], "dpf_operator_configs": [], + "dpudevices": [], "dpusets": [], "dpuclusters": [], "dpuservices": [], + "bfbs": [], "kamaji_pods": [], "kamaji_tcps": [], "cis_controllers": [], + "cis_virtualservers": [], "cis_transportservers": [], "cis_ingresslinks": [], + "cis_as3_configmaps": [], "cis_f5_ingresses": [], "openshift_routes": [], + "cneinstances": [], "vlans": [], +} + +# Analysis functions patched to no-ops when a test isolates one code path. +_ANALYZERS = [ + "services.scanner.analyze_cluster_info", + "services.scanner.analyze_cert_manager", + "services.scanner.analyze_multus", + "services.scanner.analyze_sriov", + "services.scanner.analyze_hugepages", + "services.scanner.analyze_storage", + "services.scanner.analyze_gateway_api", + "services.scanner.analyze_dpf", + "services.scanner.analyze_kamaji", + "services.scanner.analyze_cis", + "services.scanner.analyze_bnk_install", +] + + +def _run_scan(db, cluster, *, fetch_data=None, skip_analyzers=(), fetch_side_effect=None): + """Run ClusterScanner.scan() with I/O and (optionally) analyzers mocked. + + ``skip_analyzers`` names analyzers to leave REAL so a test can assert on + their output; the rest are patched to return ``{}``. ``fetch_side_effect`` + (e.g. an exception) simulates a scan that fails before completion. + """ + from services.scanner import ClusterScanner + + scanner = ClusterScanner(db) + platform_ctx = MagicMock() + platform_ctx.to_dict.return_value = {} + platform_ctx.detected_platform_profile = "roks" + + with contextlib.ExitStack() as stack: + stack.enter_context(patch.object(scanner.k8s_service, "get_cluster", return_value=cluster)) + stack.enter_context(patch.object(scanner.k8s_service, "load_kubeconfig", return_value=MagicMock())) + if fetch_side_effect is not None: + stack.enter_context(patch("services.scanner.fetch_scan_data", side_effect=fetch_side_effect)) + else: + stack.enter_context(patch( + "services.scanner.fetch_scan_data", + return_value=fetch_data if fetch_data is not None else dict(_EMPTY_FETCH_DATA), + )) + stack.enter_context(patch( + "services.scanner.PlatformContextService.apply_cluster_context", + return_value=platform_ctx, + )) + for name in _ANALYZERS: + if name in skip_analyzers: + continue + stack.enter_context(patch(name, return_value={})) + stack.enter_context(patch("services.scanner.build_recommendations", return_value=[])) + stack.enter_context(patch("services.scanner.build_proxy_recommendations", return_value=[])) + return scanner.scan(cluster.id) + + +class TestLastSyncedAtStamp: + def test_scan_stamps_last_synced_at(self, db, make_k8s_cluster): + """A completed scan sets last_synced_at (was permanently NULL — #194).""" + cluster = make_k8s_cluster() + assert cluster.last_synced_at is None # never scanned + + result = _run_scan(db, cluster) + + db.refresh(cluster) + assert isinstance(cluster.last_synced_at, datetime) + # Result metadata still reports the scan timing. + assert "scanned_at" in result["scan_metadata"] + + def test_last_synced_at_persists_across_commit(self, db, make_k8s_cluster): + """The stamp survives the commit the async registration/PUT task does.""" + cluster = make_k8s_cluster() + _run_scan(db, cluster) + db.commit() # mirrors scan_cluster_async's own commit + + db.expire_all() + reloaded = db.query(type(cluster)).filter_by(id=cluster.id).one() + assert reloaded.last_synced_at is not None + + def test_failed_scan_does_not_stamp_last_synced_at(self, db, make_k8s_cluster): + """A scan that raises before completion must NOT stamp last_synced_at. + + Mutation guard: moving the stamp above the analysis (or dropping the + 'only on success' property) would let a failed scan look synced. + """ + cluster = make_k8s_cluster() + assert cluster.last_synced_at is None + + import pytest + with pytest.raises(RuntimeError, match="cluster unreachable"): + _run_scan(db, cluster, fetch_side_effect=RuntimeError("cluster unreachable")) + + db.refresh(cluster) + assert cluster.last_synced_at is None # still never-synced + + +class TestPodInventoryPopulated: + def test_multus_pods_are_counted_not_zero(self, db, make_k8s_cluster): + """analyze_multus counts Multus pods the fetch surfaced, and the scan stamps. + + Locks the analysis + ``last_synced_at`` behaviour: over a fetch whose + Multus pods sit in kube-system (the namespace the scan actually reads), + the running count is reported, not 0. This is NOT a proof of the + reporter's OpenShift "0 pods" symptom, which is a separate + namespace-scoping gap (#202: Multus lives in openshift-multus, unfetched). + """ + cluster = make_k8s_cluster() + + fetch = dict(_EMPTY_FETCH_DATA) + fetch["crd_names"] = {"network-attachment-definitions.k8s.cni.cncf.io"} + fetch["daemonsets"] = [ + {"name": "multus", "namespace": "kube-system", "desired": 6, "ready": 6}, + ] + fetch["kube_system_pods"] = [ + {"name": f"multus-{i}", "phase": "Running"} for i in range(6) + ] + + from services.scanner.constants import PrerequisiteStatus + + result = _run_scan( + db, cluster, fetch_data=fetch, + skip_analyzers=("services.scanner.analyze_multus",), + ) + + multus = result["prerequisites"]["multus"] + assert multus["running_pods"] == 6 + assert multus["status"] == PrerequisiteStatus.DETECTED + assert multus["nad_crd_installed"] is True + # And the scan is recorded, so "empty" vs "never scanned" is decidable. + db.refresh(cluster) + assert cluster.last_synced_at is not None diff --git a/backend/tests/integration/test_routes_k8s_clusters.py b/backend/tests/integration/test_routes_k8s_clusters.py index 0623b9f..b019013 100644 --- a/backend/tests/integration/test_routes_k8s_clusters.py +++ b/backend/tests/integration/test_routes_k8s_clusters.py @@ -45,6 +45,27 @@ def test_create_cluster(self, mock_svc_cls, client, admin_headers, sample_user, assert data["cloud_provider"] == "aws" mock_svc.create_cluster.assert_called_once() + @patch("routes.k8s.clusters.enqueue_cluster_scan") + @patch("routes.k8s.clusters.ClusterManagementService") + def test_registration_enqueues_initial_scan(self, mock_svc_cls, mock_enqueue, client, admin_headers, + sample_user, sample_project): + """Issue #194 defect 1: registering a cluster enqueues the first inventory sync. + + This is the path a roks/ibm register hits — the initial sync must be + enqueued so last_synced_at can be stamped when it completes. + """ + mock_svc = MagicMock() + mock_svc.create_cluster.return_value = {"id": 16, "name": "f5e2e1", "cloud_provider": "ibm"} + mock_svc_cls.return_value = mock_svc + + response = client.post( + f"/api/projects/{sample_project.id}/k8s/clusters", + json={"name": "f5e2e1", "kubeconfig": "YXBpVmVyc2lvbjogdjEK", "cloud_provider": "ibm"}, + headers=admin_headers, + ) + assert response.status_code == 200 + mock_enqueue.assert_called_once_with(16) + @patch("routes.k8s.clusters.ClusterManagementService") def test_create_cluster_operator_allowed(self, mock_svc_cls, client, operator_headers, all_test_users, sample_project): """Operator can create clusters.""" @@ -175,6 +196,62 @@ def test_viewer_cannot_update(self, client, viewer_headers, all_test_users, samp ) assert response.status_code == 403 + @patch("routes.k8s.clusters.enqueue_cluster_scan") + @patch("routes.k8s.clusters.ClusterManagementService") + def test_noop_put_enqueues_rescan(self, mock_svc_cls, mock_enqueue, client, admin_headers, + sample_user, sample_project, make_k8s_cluster): + """Issue #194 defect 2: a no-op PUT (empty body) still enqueues a rescan. + + Operators use a no-op PUT to force a refresh; the route must enqueue a + scan regardless of whether any field actually changed. + """ + cluster = make_k8s_cluster(project=sample_project, name="noop-put") + mock_svc = MagicMock() + mock_svc.update_cluster.return_value = {"id": cluster.id, "name": "noop-put"} + mock_svc_cls.return_value = mock_svc + + response = client.put( + f"/api/k8s/clusters/{cluster.id}", json={}, headers=admin_headers + ) + assert response.status_code == 200 + mock_enqueue.assert_called_once_with(cluster.id) + + +class TestClusterResync: + """POST /api/k8s/clusters/{id}/resync — explicit inventory-sync trigger (#194).""" + + @patch("routes.k8s.clusters.enqueue_cluster_scan") + @patch("routes.k8s.clusters.ClusterManagementService") + def test_resync_enqueues_scan(self, mock_svc_cls, mock_enqueue, client, admin_headers, + sample_user, sample_project, make_k8s_cluster): + """Admin can force a resync; the endpoint enqueues a background scan.""" + cluster = make_k8s_cluster(project=sample_project, name="resync-me") + mock_svc = MagicMock() + mock_svc.get_cluster_details.return_value = {"id": cluster.id} + mock_svc_cls.return_value = mock_svc + + response = client.post(f"/api/k8s/clusters/{cluster.id}/resync", headers=admin_headers) + + assert response.status_code == 200 + data = response.json() + assert data["success"] is True + assert data["cluster_id"] == cluster.id + mock_enqueue.assert_called_once_with(cluster.id) + + @patch("routes.k8s.clusters.enqueue_cluster_scan") + def test_resync_unknown_cluster_404(self, mock_enqueue, client, admin_headers, sample_user): + """Resync of an unknown cluster is a clean 404 — no scan enqueued.""" + response = client.post("/api/k8s/clusters/99999/resync", headers=admin_headers) + assert response.status_code == 404 + mock_enqueue.assert_not_called() + + def test_viewer_cannot_resync(self, client, viewer_headers, all_test_users, + sample_project, make_k8s_cluster): + """Viewer cannot trigger a resync — returns 403.""" + cluster = make_k8s_cluster(project=sample_project) + response = client.post(f"/api/k8s/clusters/{cluster.id}/resync", headers=viewer_headers) + assert response.status_code == 403 + class TestClusterDelete: """DELETE /api/k8s/clusters/{id}.""" diff --git a/frontend-v2/src/types/api-generated.ts b/frontend-v2/src/types/api-generated.ts index 44a03e8..0a3b72b 100644 --- a/frontend-v2/src/types/api-generated.ts +++ b/frontend-v2/src/types/api-generated.ts @@ -574,6 +574,34 @@ export interface paths { patch?: never; trace?: never; }; + "/api/k8s/clusters/{cluster_id}/resync": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Resync Cluster + * @description Force a fresh inventory sync for a cluster (owner or admin only). + * + * Issue #194: an explicit, documented rescan trigger. Operators previously + * relied on a no-op ``PUT`` to force a refresh; this endpoint makes that + * intent first-class and reliable. It enqueues a background scan (the same + * task registration/PUT use) and returns immediately — the scan stamps + * ``last_synced_at`` on completion. An unknown cluster 404s via the + * ``require_cluster_owner`` dependency before this body runs, so there is no + * silently-swallowed background no-op. + */ + post: operations["resync_cluster_api_k8s_clusters__cluster_id__resync_post"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/k8s/clusters/{cluster_id}/refresh-kubeconfig": { parameters: { query?: never; @@ -24070,6 +24098,37 @@ export interface operations { }; }; }; + resync_cluster_api_k8s_clusters__cluster_id__resync_post: { + parameters: { + query?: never; + header?: never; + path: { + cluster_id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Successful Response */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ClusterOperationResponse"]; + }; + }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; + }; + }; refresh_cluster_kubeconfig_api_k8s_clusters__cluster_id__refresh_kubeconfig_post: { parameters: { query?: never;