From 707a9260573c58b908f789c5e92301f9e5ff77ab Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Wed, 29 Jan 2025 13:22:01 -0600 Subject: [PATCH] Remove unused compute_ctl HTTP routes (#10544) These are not used anywhere within the platform, so let's remove dead code. Signed-off-by: Tristan Partin --- compute_tools/src/http/openapi_spec.yaml | 29 ---- compute_tools/src/http/routes/info.rs | 11 -- .../src/http/routes/installed_extensions.rs | 33 ---- compute_tools/src/http/routes/mod.rs | 2 - compute_tools/src/http/server.rs | 8 +- libs/compute_api/src/responses.rs | 5 - test_runner/fixtures/endpoint/http.py | 5 - test_runner/regress/test_compute_metrics.py | 91 +++++++++++ .../regress/test_installed_extensions.py | 154 ------------------ 9 files changed, 92 insertions(+), 246 deletions(-) delete mode 100644 compute_tools/src/http/routes/info.rs delete mode 100644 compute_tools/src/http/routes/installed_extensions.rs delete mode 100644 test_runner/regress/test_installed_extensions.py diff --git a/compute_tools/src/http/openapi_spec.yaml b/compute_tools/src/http/openapi_spec.yaml index 50319cdd85..bbdb7d0917 100644 --- a/compute_tools/src/http/openapi_spec.yaml +++ b/compute_tools/src/http/openapi_spec.yaml @@ -68,35 +68,6 @@ paths: schema: $ref: "#/components/schemas/ComputeInsights" - /installed_extensions: - get: - tags: - - Info - summary: Get installed extensions. - description: "" - operationId: getInstalledExtensions - responses: - 200: - description: List of installed extensions - content: - application/json: - schema: - $ref: "#/components/schemas/InstalledExtensions" - /info: - get: - tags: - - Info - summary: Get info about the compute pod / VM. - description: "" - operationId: getInfo - responses: - 200: - description: Info - content: - application/json: - schema: - $ref: "#/components/schemas/Info" - /dbs_and_roles: get: tags: diff --git a/compute_tools/src/http/routes/info.rs b/compute_tools/src/http/routes/info.rs deleted file mode 100644 index 32d6fea74c..0000000000 --- a/compute_tools/src/http/routes/info.rs +++ /dev/null @@ -1,11 +0,0 @@ -use axum::response::Response; -use compute_api::responses::InfoResponse; -use http::StatusCode; - -use crate::http::JsonResponse; - -/// Get information about the physical characteristics about the compute. -pub(in crate::http) async fn get_info() -> Response { - let num_cpus = num_cpus::get_physical(); - JsonResponse::success(StatusCode::OK, &InfoResponse { num_cpus }) -} diff --git a/compute_tools/src/http/routes/installed_extensions.rs b/compute_tools/src/http/routes/installed_extensions.rs deleted file mode 100644 index db74a6b195..0000000000 --- a/compute_tools/src/http/routes/installed_extensions.rs +++ /dev/null @@ -1,33 +0,0 @@ -use std::sync::Arc; - -use axum::{extract::State, response::Response}; -use compute_api::responses::ComputeStatus; -use http::StatusCode; -use tokio::task; - -use crate::{compute::ComputeNode, http::JsonResponse, installed_extensions}; - -/// Get a list of installed extensions. -pub(in crate::http) async fn get_installed_extensions( - State(compute): State>, -) -> Response { - let status = compute.get_status(); - if status != ComputeStatus::Running { - return JsonResponse::invalid_status(status); - } - - let conf = compute.get_conn_conf(None); - let res = task::spawn_blocking(move || installed_extensions::get_installed_extensions(conf)) - .await - .unwrap(); - - match res { - Ok(installed_extensions) => { - JsonResponse::success(StatusCode::OK, Some(installed_extensions)) - } - Err(e) => JsonResponse::error( - StatusCode::INTERNAL_SERVER_ERROR, - format!("failed to get list of installed extensions: {e}"), - ), - } -} diff --git a/compute_tools/src/http/routes/mod.rs b/compute_tools/src/http/routes/mod.rs index 3efa1153ad..a67be7fd5a 100644 --- a/compute_tools/src/http/routes/mod.rs +++ b/compute_tools/src/http/routes/mod.rs @@ -10,9 +10,7 @@ pub(in crate::http) mod extension_server; pub(in crate::http) mod extensions; pub(in crate::http) mod failpoints; pub(in crate::http) mod grants; -pub(in crate::http) mod info; pub(in crate::http) mod insights; -pub(in crate::http) mod installed_extensions; pub(in crate::http) mod metrics; pub(in crate::http) mod metrics_json; pub(in crate::http) mod status; diff --git a/compute_tools/src/http/server.rs b/compute_tools/src/http/server.rs index da650585fc..e41ed9df2d 100644 --- a/compute_tools/src/http/server.rs +++ b/compute_tools/src/http/server.rs @@ -22,8 +22,7 @@ use uuid::Uuid; use super::routes::{ check_writability, configure, database_schema, dbs_and_roles, extension_server, extensions, - grants, info as info_route, insights, installed_extensions, metrics, metrics_json, status, - terminate, + grants, insights, metrics, metrics_json, status, terminate, }; use crate::compute::ComputeNode; @@ -60,12 +59,7 @@ async fn serve(port: u16, compute: Arc) { ) .route("/extensions", post(extensions::install_extension)) .route("/grants", post(grants::add_grant)) - .route("/info", get(info_route::get_info)) .route("/insights", get(insights::get_insights)) - .route( - "/installed_extensions", - get(installed_extensions::get_installed_extensions), - ) .route("/metrics", get(metrics::get_metrics)) .route("/metrics.json", get(metrics_json::get_metrics)) .route("/status", get(status::get_status)) diff --git a/libs/compute_api/src/responses.rs b/libs/compute_api/src/responses.rs index 9ce605089b..5286e0e61d 100644 --- a/libs/compute_api/src/responses.rs +++ b/libs/compute_api/src/responses.rs @@ -15,11 +15,6 @@ pub struct GenericAPIError { pub error: String, } -#[derive(Debug, Clone, Serialize)] -pub struct InfoResponse { - pub num_cpus: usize, -} - #[derive(Debug, Clone, Serialize)] pub struct ExtensionInstallResponse { pub extension: PgIdent, diff --git a/test_runner/fixtures/endpoint/http.py b/test_runner/fixtures/endpoint/http.py index aa0d95fe80..6e8210e978 100644 --- a/test_runner/fixtures/endpoint/http.py +++ b/test_runner/fixtures/endpoint/http.py @@ -28,11 +28,6 @@ class EndpointHttpClient(requests.Session): res.raise_for_status() return res.text - def installed_extensions(self): - res = self.get(f"http://localhost:{self.port}/installed_extensions") - res.raise_for_status() - return res.json() - def extensions(self, extension: str, version: str, database: str): body = { "extension": extension, diff --git a/test_runner/regress/test_compute_metrics.py b/test_runner/regress/test_compute_metrics.py index 5dcc93acff..99d41e410a 100644 --- a/test_runner/regress/test_compute_metrics.py +++ b/test_runner/regress/test_compute_metrics.py @@ -5,16 +5,22 @@ import os import shutil import sys from enum import StrEnum +from logging import debug from pathlib import Path from typing import TYPE_CHECKING, cast import pytest import requests import yaml +from fixtures.endpoint.http import EndpointHttpClient from fixtures.log_helper import log +from fixtures.metrics import parse_metrics from fixtures.paths import BASE_DIR, COMPUTE_CONFIG_DIR +from fixtures.utils import wait_until +from prometheus_client.samples import Sample if TYPE_CHECKING: + from collections.abc import Callable from types import TracebackType from typing import Self, TypedDict @@ -467,3 +473,88 @@ def test_perf_counters(neon_simple_env: NeonEnv): cur.execute("CREATE EXTENSION neon VERSION '1.5'") cur.execute("SELECT * FROM neon_perf_counters") cur.execute("SELECT * FROM neon_backend_perf_counters") + + +def collect_metric( + client: EndpointHttpClient, + name: str, + filter: dict[str, str], + predicate: Callable[[list[Sample]], bool], +) -> Callable[[], list[Sample]]: + """ + Call this function as the first argument to wait_until(). + """ + + def __collect_metric() -> list[Sample]: + resp = client.metrics() + debug("Metrics: %s", resp) + m = parse_metrics(resp) + samples = m.query_all(name, filter) + debug("Samples: %s", samples) + assert predicate(samples), "predicate failed" + return samples + + return __collect_metric + + +def test_compute_installed_extensions_metric(neon_simple_env: NeonEnv): + """ + Test that the compute_installed_extensions properly reports accurate + results. Important to note that currently this metric is only gathered on + compute start. + """ + env = neon_simple_env + + endpoint = env.endpoints.create_start("main") + + client = endpoint.http_client() + + def __has_plpgsql(samples: list[Sample]) -> bool: + """ + Check that plpgsql is installed in the template1 and postgres databases + """ + return len(samples) == 1 and samples[0].value == 2 + + wait_until( + collect_metric( + client, + "compute_installed_extensions", + {"extension_name": "plpgsql", "version": "1.0", "owned_by_superuser": "1"}, + __has_plpgsql, + ), + name="compute_installed_extensions", + ) + + # Install the neon extension, so we can check for it on the restart + endpoint.safe_psql("CREATE EXTENSION neon VERSION '1.0'") + + # The metric is only gathered on compute start, so restart to check if the + # neon extension will now be there. + endpoint.stop() + endpoint.start() + + client = endpoint.http_client() + + def __has_neon(samples: list[Sample]) -> bool: + return len(samples) == 1 and samples[0].value == 1 + + wait_until( + collect_metric( + client, + "compute_installed_extensions", + {"extension_name": "neon", "version": "1.0", "owned_by_superuser": "1"}, + __has_neon, + ), + name="compute_installed_extensions", + ) + + # Double check that we also still have plpgsql + wait_until( + collect_metric( + client, + "compute_installed_extensions", + {"extension_name": "plpgsql", "version": "1.0", "owned_by_superuser": "1"}, + __has_plpgsql, + ), + name="compute_installed_extensions", + ) diff --git a/test_runner/regress/test_installed_extensions.py b/test_runner/regress/test_installed_extensions.py deleted file mode 100644 index 4e51e7e10c..0000000000 --- a/test_runner/regress/test_installed_extensions.py +++ /dev/null @@ -1,154 +0,0 @@ -from __future__ import annotations - -import time -from logging import info -from typing import TYPE_CHECKING - -from fixtures.log_helper import log -from fixtures.metrics import parse_metrics - -if TYPE_CHECKING: - from fixtures.neon_fixtures import NeonEnv - - -def test_installed_extensions(neon_simple_env: NeonEnv): - """basic test for the endpoint that returns the list of installed extensions""" - - env = neon_simple_env - - env.create_branch("test_installed_extensions") - - endpoint = env.endpoints.create_start("test_installed_extensions") - - endpoint.safe_psql("CREATE DATABASE test_installed_extensions") - endpoint.safe_psql("CREATE DATABASE test_installed_extensions_2") - - client = endpoint.http_client() - res = client.installed_extensions() - - info("Extensions list: %s", res) - info("Extensions: %s", res["extensions"]) - # 'plpgsql' is a default extension that is always installed. - assert any( - ext["extname"] == "plpgsql" and ext["version"] == "1.0" for ext in res["extensions"] - ), "The 'plpgsql' extension is missing" - - # check that the neon_test_utils extension is not installed - assert not any( - ext["extname"] == "neon_test_utils" for ext in res["extensions"] - ), "The 'neon_test_utils' extension is installed" - - pg_conn = endpoint.connect(dbname="test_installed_extensions") - with pg_conn.cursor() as cur: - cur.execute("CREATE EXTENSION neon_test_utils") - cur.execute( - "SELECT default_version FROM pg_available_extensions WHERE name = 'neon_test_utils'" - ) - res = cur.fetchone() - neon_test_utils_version = res[0] - - with pg_conn.cursor() as cur: - cur.execute("CREATE EXTENSION neon version '1.1'") - - pg_conn_2 = endpoint.connect(dbname="test_installed_extensions_2") - with pg_conn_2.cursor() as cur: - cur.execute("CREATE EXTENSION neon version '1.2'") - - res = client.installed_extensions() - - info("Extensions list: %s", res) - info("Extensions: %s", res["extensions"]) - - # check that the neon_test_utils extension is installed only in 1 database - # and has the expected version - assert any( - ext["extname"] == "neon_test_utils" - and ext["version"] == neon_test_utils_version - and ext["n_databases"] == 1 - for ext in res["extensions"] - ) - - # check that the plpgsql extension is installed in all databases - # this is a default extension that is always installed - assert any(ext["extname"] == "plpgsql" and ext["n_databases"] == 4 for ext in res["extensions"]) - - # check that the neon extension is installed and has expected versions - for ext in res["extensions"]: - if ext["extname"] == "neon": - assert ext["version"] in ["1.1", "1.2"] - assert ext["n_databases"] == 1 - - with pg_conn.cursor() as cur: - cur.execute("ALTER EXTENSION neon UPDATE TO '1.3'") - - res = client.installed_extensions() - - info("Extensions list: %s", res) - info("Extensions: %s", res["extensions"]) - - # check that the neon_test_utils extension is updated - for ext in res["extensions"]: - if ext["extname"] == "neon": - assert ext["version"] in ["1.2", "1.3"] - assert ext["n_databases"] == 1 - - # check that /metrics endpoint is available - # ensure that we see the metric before and after restart - res = client.metrics() - info("Metrics: %s", res) - m = parse_metrics(res) - neon_m = m.query_all( - "compute_installed_extensions", - {"extension_name": "neon", "version": "1.2", "owned_by_superuser": "1"}, - ) - assert len(neon_m) == 1 - for sample in neon_m: - assert sample.value == 1 - neon_m = m.query_all( - "compute_installed_extensions", - {"extension_name": "neon", "version": "1.3", "owned_by_superuser": "1"}, - ) - assert len(neon_m) == 1 - for sample in neon_m: - assert sample.value == 1 - - endpoint.stop() - endpoint.start() - - timeout = 10 - while timeout > 0: - try: - res = client.metrics() - timeout = -1 - if len(parse_metrics(res).query_all("compute_installed_extensions")) < 4: - # Assume that not all metrics that are collected yet - time.sleep(1) - timeout -= 1 - continue - except Exception: - log.exception("failed to get metrics, assume they are not collected yet") - time.sleep(1) - timeout -= 1 - continue - - assert ( - len(parse_metrics(res).query_all("compute_installed_extensions")) >= 4 - ), "Not all metrics are collected" - - info("After restart metrics: %s", res) - m = parse_metrics(res) - neon_m = m.query_all( - "compute_installed_extensions", - {"extension_name": "neon", "version": "1.2", "owned_by_superuser": "1"}, - ) - assert len(neon_m) == 1 - for sample in neon_m: - assert sample.value == 1 - - neon_m = m.query_all( - "compute_installed_extensions", - {"extension_name": "neon", "version": "1.3", "owned_by_superuser": "1"}, - ) - assert len(neon_m) == 1 - for sample in neon_m: - assert sample.value == 1