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] cockpit-project/bots#6432

doc/guide/feature-pcp.xml

@@ -32,7 +32,7 @@ $ <command>pmstat</command>
 
   <para>These metrics can also be exposed to other machines on a TCP port with
   <ulink url="https://linux.die.net/man/1/pmproxy">pmproxy</ulink>
-  and <ulink url="https://redis.io/">Redis</ulink>:</para>
+  and <ulink url="https://redis.io/">Redis</ulink> or <ulink url="https://valkey.io/">Valkey</ulink>:</para>
 
 <programlisting>
 systemctl enable --now redis pmproxy

pkg/metrics/manifest.json

@@ -7,5 +7,11 @@
                 "url": "https://pcp.readthedocs.io/en/latest/"
             }
         ]
+    },
+
+    "config": {
+        "redis_package": {
+            "fedora": "valkey"
+        }
     }
 }

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 (
         <Button variant={buttonVariant} icon={<CogIcon />}
-                isDisabled={ invalidService(s_pmlogger) || invalidService(s_pmproxy) || invalidService(s_redis) || invalidService(s_redis_server) }
+                isDisabled={ invalidService(s_pmlogger) || invalidService(s_pmproxy) ||
+                             invalidService(s_redis) || invalidService(s_redis_server) || invalidService(s_valkey) }
                 onClick={show_dialog}>
             { _("Metrics settings") }
         </Button>);

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")