From fe9a48e7962a5a423dc39e785305931821547836 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 27 May 2024 09:41:50 +0200 Subject: [PATCH] metrics: Prefer valkey over redis This fork is Fedora's prefered redis API now [1], since redis' license change. Our Fedora bots images have `valkey` since [2]. However, also still test with the original `redis` package as long as that exists in Fedora, in the new `testPmProxySettingsRedis()`. Declare the prefered package in a per-OS "config" manifest field, as detecting it at runtime is too expensive and brittle. [1] https://fedoraproject.org/wiki/Changes/Replace_Redis_With_Valkey [2] https://github.com/cockpit-project/bots/pull/6432 --- doc/guide/feature-pcp.xml | 2 +- pkg/metrics/manifest.json | 6 ++++++ pkg/metrics/metrics.jsx | 36 ++++++++++++++++++++++++++---------- test/verify/check-metrics | 24 +++++++++++++++++++----- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/doc/guide/feature-pcp.xml b/doc/guide/feature-pcp.xml index 559709d27bf0..ee36f3ccb6a7 100644 --- a/doc/guide/feature-pcp.xml +++ b/doc/guide/feature-pcp.xml @@ -32,7 +32,7 @@ $ pmstat These metrics can also be exposed to other machines on a TCP port with pmproxy - and Redis: + and Redis or Valkey: systemctl enable --now redis pmproxy diff --git a/pkg/metrics/manifest.json b/pkg/metrics/manifest.json index 6b3d3ec3b632..5af17f0fcb67 100644 --- a/pkg/metrics/manifest.json +++ b/pkg/metrics/manifest.json @@ -7,5 +7,11 @@ "url": "https://pcp.readthedocs.io/en/latest/" } ] + }, + + "config": { + "redis_package": { + "fedora": "valkey" + } } } diff --git a/pkg/metrics/metrics.jsx b/pkg/metrics/metrics.jsx index a935c2395d15..d5e71c4446d7 100644 --- a/pkg/metrics/metrics.jsx +++ b/pkg/metrics/metrics.jsx @@ -50,6 +50,8 @@ import * as service from "service"; import * as timeformat from "timeformat"; import { superuser } from "superuser"; import { journal } from "journal"; +import { read_os_release } from "os-release"; +import { get_manifest_config_matchlist } from "utils"; import { useObject, useEvent, useInit } from "hooks.js"; import { WithDialogs, useDialogs } from "dialogs.jsx"; @@ -1363,10 +1365,11 @@ const wait_cond = (cond, objects) => { const PCPConfigDialog = ({ firewalldRequest, needsLogout, setNeedsLogout, - s_pmlogger, s_pmproxy, s_redis, s_redis_server + s_pmlogger, s_pmproxy, s_redis, s_redis_server, s_valkey, }) => { const Dialogs = useDialogs(); - const dialogInitialProxyValue = runningService(s_pmproxy) && (runningService(s_redis) || runningService(s_redis_server)); + const dialogInitialProxyValue = runningService(s_pmproxy) && ( + runningService(s_redis) || runningService(s_redis_server) || runningService(s_valkey)); const [dialogError, setDialogError] = useState(null); const [dialogLoggerValue, setDialogLoggerValue] = useState(runningService(s_pmlogger)); const [dialogProxyValue, setDialogProxyValue] = useState(dialogInitialProxyValue); @@ -1380,9 +1383,13 @@ const PCPConfigDialog = ({ const missing = []; if (dialogLoggerValue && !s_pmlogger.exists) missing.push("cockpit-pcp"); - const redisExists = () => s_redis.exists || s_redis_server.exists; - if (dialogProxyValue && !redisExists()) - missing.push("redis"); + const redisExists = () => s_redis.exists || s_redis_server.exists || s_valkey.exists; + if (dialogProxyValue && !redisExists()) { + const os_release = await read_os_release(); + missing.push(get_manifest_config_matchlist("metrics", "redis_package", "redis", + [os_release.PLATFORM_ID, os_release.ID])); + } + if (missing.length > 0) { debug("PCPConfig: missing packages", JSON.stringify(missing), ", offering install"); Dialogs.close(); @@ -1392,7 +1399,7 @@ const PCPConfigDialog = ({ setNeedsLogout(true); await wait_cond(() => (s_pmlogger.exists && (!dialogProxyValue || (s_pmproxy.exists && redisExists()))), - [s_pmlogger, s_pmproxy, s_redis, s_redis_server]); + [s_pmlogger, s_pmproxy, s_redis, s_redis_server, s_valkey]); } }; @@ -1405,7 +1412,10 @@ const PCPConfigDialog = ({ let real_redis; let redis_name; - if (s_redis_server.exists) { + if (s_valkey.exists && s_valkey.unit?.UnitFileState !== 'masked') { + real_redis = s_valkey; + redis_name = "valkey.service"; + } else if (s_redis_server.exists && s_redis_server.unit?.UnitFileState !== 'masked') { real_redis = s_redis_server; redis_name = "redis-server.service"; } else { @@ -1527,15 +1537,20 @@ const PCPConfig = ({ buttonVariant, firewalldRequest, needsLogout, setNeedsLogou // redis.service on Fedora/RHEL, redis-server.service on Debian/Ubuntu with an Alias=redis const s_redis = useObject(() => service.proxy("redis.service"), null, []); const s_redis_server = useObject(() => service.proxy("redis-server.service"), null, []); + const s_valkey = useObject(() => service.proxy("valkey.service"), null, []); useEvent(superuser, "changed"); useEvent(s_pmlogger, "changed"); useEvent(s_pmproxy, "changed"); useEvent(s_redis, "changed"); useEvent(s_redis_server, "changed"); + useEvent(s_valkey, "changed"); debug("PCPConfig s_pmlogger.state", s_pmlogger.state, "needs logout", needsLogout); - debug("PCPConfig s_pmproxy state", s_pmproxy.state, "redis exists", s_redis.exists, "state", s_redis.state, "redis-server exists", s_redis_server.exists, "state", s_redis_server.state); + debug("PCPConfig s_pmproxy state", s_pmproxy.state, + "redis exists", s_redis.exists, "state", s_redis.state, + "redis-server exists", s_redis_server.exists, "state", s_redis_server.state, + "valkey exists", s_valkey.exists, "state", s_valkey.state); if (!superuser.allowed) return null; @@ -1545,12 +1560,13 @@ const PCPConfig = ({ buttonVariant, firewalldRequest, needsLogout, setNeedsLogou needsLogout={needsLogout} setNeedsLogout={setNeedsLogout} s_pmlogger={s_pmlogger} s_pmproxy={s_pmproxy} - s_redis={s_redis} s_redis_server={s_redis_server} />); + s_redis={s_redis} s_redis_server={s_redis_server} s_valkey={s_valkey} />); } return ( ); diff --git a/test/verify/check-metrics b/test/verify/check-metrics index 3e105bbe1491..06c3ba0917b5 100755 --- a/test/verify/check-metrics +++ b/test/verify/check-metrics @@ -68,6 +68,8 @@ def prepareArchive(machine, name, time, hostname="localhost.localdomain"): def redisService(image): if image.startswith(("debian", "ubuntu")): return "redis-server" + if image.startswith("fedora"): + return "valkey" return "redis" @@ -546,7 +548,7 @@ class TestHistoryMetrics(testlib.MachineCase): @testlib.nondestructive @testlib.skipOstree("no PCP support") - def testPmProxySettings(self): + def testPmProxySettings(self, customRedisService=None): b = self.browser m = self.machine @@ -557,10 +559,10 @@ class TestHistoryMetrics(testlib.MachineCase): m.execute("firewall-cmd --zone=public --change-interface eth0 --permanent") m.execute("firewall-cmd --reload") - redis = redisService(m.image) + redis = customRedisService or redisService(m.image) hostname = m.execute("hostname").strip() - self.addCleanup(m.execute, f"systemctl stop {redis}") + self.addCleanup(m.execute, f"systemctl disable --now {redis}") def checkEnable(firewalld_alert): b.click("#metrics-header-section button.pf-m-secondary") @@ -715,6 +717,14 @@ class TestHistoryMetrics(testlib.MachineCase): m.execute("systemctl start pmproxy") checkEnabled(expected=True) + @testlib.nondestructive + @testlib.skipOstree("no PCP support") + @testlib.onlyImage("valkey is the default only in Fedora", "fedora*") + def testPmProxySettingsRedis(self): + self.machine.execute("systemctl mask valkey") + self.addCleanup(self.machine.execute, "systemctl unmask valkey") + self.testPmProxySettings(customRedisService="redis") + @testlib.nondestructive class TestCurrentMetrics(testlib.MachineCase): @@ -1255,6 +1265,7 @@ class TestMetricsPackages(packagelib.PackageCase): m = self.machine redis_service = redisService(m.image) + redis_package = "valkey" if redis_service == "valkey" else "redis" if m.ostree_image: self.login_and_go("/metrics") @@ -1274,7 +1285,10 @@ class TestMetricsPackages(packagelib.PackageCase): # HACK: pcp does not clean up correctly on Debian https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=986074 m.execute("rm -f /etc/systemd/system/pmlogger.service.requires/pmlogger_farm.service") else: - m.execute(f"rpm --erase --verbose cockpit-pcp pcp {redis_service}") + m.execute(f"rpm --erase --verbose cockpit-pcp pcp {redis_package}") + if m.image.startswith("fedora"): + # we removed valkey above, but also remove redis to trigger install dialog + m.execute("rpm --erase --verbose redis") dummy_service = "[Service]\nExecStart=/bin/sleep infinity\n[Install]\nWantedBy=multi-user.target\n" @@ -1293,7 +1307,7 @@ class TestMetricsPackages(packagelib.PackageCase): self.createPackage("cockpit-pcp", "999", "1", content=cpcp_content, depends="pcp", postinst="chmod +x /usr/libexec/cockpit-pcp") self.createPackage("pcp", "999", "1", content=pcp_content, postinst="systemctl daemon-reload") - self.createPackage("redis", "999", "1", content=redis_content, postinst="systemctl daemon-reload") + self.createPackage(redis_package, "999", "1", content=redis_content, postinst="systemctl daemon-reload") self.enableRepo() m.execute("pkcon refresh")