fix(argocd): stop the redis restart from making a split worse

Two failure paths the first cut got wrong:

- If argocd-redis itself fails to restart, restarting its clients hands
  them the new password while redis still requires the old one — exactly
  the WRONGPASS split this is meant to prevent. Redis now runs first and
  is the only target allowed to abort the sequence; a 404 means there is
  nothing to keep in step, anything else is raised.
- `.catch(() => undefined)` on the read swallowed RBAC and transient
  errors, so a real rotation could silently skip the restart. getK8sSecret
  already maps 404 to undefined, so the catch only ever hid real failures.
  A failed read now assumes rotation — a redundant restart is cheap, a
  missed one is an outage.

Author: aweingarten

src/common/k8s.test.ts

@@ -1081,6 +1081,19 @@ describe('createArgoCdRedisSecret', () => {
     expect(deps.restartArgoCdRedisConsumers).not.toHaveBeenCalled()
   })
 
+  it('should restart when the existing secret cannot be read at all', async () => {
+    const deps = makeDeps({
+      getK8sSecret: jest.fn(async () => {
+        throw new Error('forbidden')
+      }),
+    })
+
+    await k8s.createArgoCdRedisSecret({ apps: { argocd: { redisPassword: password } } }, deps as any)
+
+    expect(objectApi.patch).toHaveBeenCalled()
+    expect(deps.restartArgoCdRedisConsumers).toHaveBeenCalledWith('argocd')
+  })
+
   it('should skip reconciliation entirely when no password is supplied', async () => {
     const deps = makeDeps()
 
@@ -1121,4 +1134,32 @@ describe('restartArgoCdRedisConsumers', () => {
 
     expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-repo-server', 'argocd')
   })
+
+  it('should not restart the clients when redis itself fails to restart', async () => {
+    const deps = makeDeps({
+      restartDeployment: jest.fn(async (name: string) => {
+        if (name === 'argocd-redis') throw new Error('boom')
+      }),
+    })
+
+    await expect(k8s.restartArgoCdRedisConsumers('argocd', deps as any)).rejects.toThrow('boom')
+
+    expect(deps.restartDeployment).toHaveBeenCalledTimes(1)
+    expect(deps.restartDeployment).toHaveBeenCalledWith('argocd-redis', 'argocd')
+    expect(deps.restartStatefulSet).not.toHaveBeenCalled()
+  })
+
+  it('should leave the clients alone when redis is not present at all', async () => {
+    const notFound = new MockApiException(404, 'not found', {}, {})
+    const deps = makeDeps({
+      restartDeployment: jest.fn(async (name: string) => {
+        if (name === 'argocd-redis') throw notFound
+      }),
+    })
+
+    await expect(k8s.restartArgoCdRedisConsumers('argocd', deps as any)).resolves.toBeUndefined()
+
+    expect(deps.restartDeployment).toHaveBeenCalledTimes(1)
+    expect(deps.restartStatefulSet).not.toHaveBeenCalled()
+  })
 })

src/common/k8s.ts

@@ -838,7 +838,25 @@ export const restartArgoCdRedisConsumers = async (
   deps = { restartDeployment, restartStatefulSet },
 ): Promise<void> => {
   const d = terminal('common:k8s:restartArgoCdRedisConsumers')
-  for (const target of ARGOCD_REDIS_RESTART_TARGETS) {
+  const [redis, ...consumers] = ARGOCD_REDIS_RESTART_TARGETS
+
+  // Redis has to come back on the new password before anything is pointed at it. If it does not
+  // restart, restarting the clients is worse than doing nothing: they would pick up the new
+  // password while redis still requires the old one, which is the split this function exists to
+  // prevent. So redis, and only redis, is allowed to stop the sequence.
+  try {
+    await deps.restartDeployment(redis.name, namespace)
+    d.info(`Restarted ${redis.kind}/${redis.name} after redis password change`)
+  } catch (error) {
+    if (error instanceof ApiException && error.code === 404) {
+      d.debug(`Could not restart ${redis.kind}/${redis.name}: not found — leaving its clients alone`)
+      return
+    }
+    d.error(`Could not restart ${redis.kind}/${redis.name}, not restarting its clients:`, error)
+    throw error
+  }
+
+  for (const target of consumers) {
     try {
       if (target.kind === 'deployment') await deps.restartDeployment(target.name, namespace)
       else await deps.restartStatefulSet(target.name, namespace)
@@ -871,8 +889,17 @@ export const createArgoCdRedisSecret = async (
 
   // Read before writing so the restart below is limited to an actual rotation. This runs on every
   // install, and restarting the whole Argo stack on an unchanged password would be its own outage.
-  const existingSecret = await deps.getK8sSecret(secretName, argocdNamespace).catch(() => undefined)
-  const passwordChanged = existingSecret !== undefined && existingSecret.auth !== redisPassword
+  // getK8sSecret already maps 404 to undefined (first install — nothing to restart), so anything
+  // that throws here is a real read failure and leaves us unable to tell. Assume it rotated: a
+  // redundant restart costs seconds, a missed one leaves redis and its clients split on WRONGPASS.
+  let passwordChanged: boolean
+  try {
+    const existingSecret = await deps.getK8sSecret(secretName, argocdNamespace)
+    passwordChanged = existingSecret !== undefined && existingSecret.auth !== redisPassword
+  } catch (error) {
+    d.warn(`Could not read Secret ${secretName} to detect a password change, assuming it rotated:`, error)
+    passwordChanged = true
+  }
 
   try {
     await k8s.object().patch(