Skip to content

fix(redis): dynamic reconfigure fails for hyphenated parameters#2741

Open
weicao wants to merge 2 commits into
mainfrom
redis-reconfigure-hyphen-env-fix
Open

fix(redis): dynamic reconfigure fails for hyphenated parameters#2741
weicao wants to merge 2 commits into
mainfrom
redis-reconfigure-hyphen-env-fix

Conversation

@weicao
Copy link
Copy Markdown
Contributor

@weicao weicao commented Jun 2, 2026

Summary

  • Fix dynamic reconfigure (CONFIG SET) failing for Redis parameters with hyphens in their names (e.g. maxmemory-policy, repl-backlog-size, slowlog-max-len)
  • Root cause was a three-layer bug stack:
    1. /bin/sh strips env vars with invalid POSIX names (hyphens) from its environment, making env/printenv unable to see them
    2. set -eu + pipe while loop (tr | while) runs in subshell where set -e silently exits on first redis-cli stderr warning
    3. Redis Cluster scripts ConfigMap was missing reload-parameter.sh, breaking reconfigure on cluster topology
  • Fix reads from /proc/self/environ (preserves original execve environment), uses file-redirect loop to avoid subshell, and includes reload-parameter.sh in redis-cluster scripts

Changes

  • _helpers.tpl redis.config.reconfigureAction: Replace env | cut + pipe while with /proc/self/environ + file-redirect while
  • _helpers.tpl redis-cluster.extend.scripts: Add reload-parameter.sh to redis-cluster scripts ConfigMap

Test plan

  • Standalone: 4-param reconfigure (maxmemory-policy, hz, slowlog-max-len, repl-backlog-size) — CONFIG GET all updated, no pod restart
  • Replication: Same 4 params — both data pods updated
  • Twemproxy: Same 4 params — both backing Redis data pods updated
  • Redis Cluster: Same 4 params — all 6 pods (3 shards x 2 replicas) updated
  • Full parameters test suite regression

🤖 Generated with Claude Code

The reconfigure exec command running in the kbagent container failed to
apply CONFIG SET for parameters with hyphens (e.g. maxmemory-policy,
repl-backlog-size, slowlog-max-len). Three stacked issues:

1. /bin/sh strips env vars with invalid POSIX names (containing hyphens)
   from its environment, making them invisible to `env` and `printenv`.

2. The original `set -eu` + pipe-based `while` loop (`tr | while`)
   created a subshell where `set -e` caused silent exit on the first
   redis-cli stderr warning.

3. The redis-cluster scripts ConfigMap was missing reload-parameter.sh,
   causing the reconfigure script to fail with "not found" on Redis
   Cluster topology.

Fix: read parameters from /proc/self/environ (which preserves the
original execve environment including hyphenated names), use file-
redirect loop instead of pipe to avoid subshell, and include
reload-parameter.sh in redis-cluster scripts ConfigMap.

Validated on all 4 topologies (standalone, replication, twemproxy,
Redis Cluster) with 4 parameters (3 hyphenated + 1 non-hyphenated).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@weicao weicao requested review from a team, leon-ape and wangyelei as code owners June 2, 2026 12:38
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (bead379) to head (418b13a).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #2741   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         73      73           
  Lines       9236    9236           
=====================================
  Misses      9236    9236           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Accumulate reload-parameter.sh exit code so a failed CONFIG SET
surfaces as a non-zero exit instead of being masked by the final
rm -f (which always returns 0).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wangyelei
Copy link
Copy Markdown
Contributor

wangyelei commented Jun 4, 2026

https://github.com/apecloud/kubeblocks/pull/10182/changes look at this pr. only can reconfigure parameters by reconfigure operation.

@wangyelei wangyelei closed this Jun 4, 2026
@wangyelei wangyelei reopened this Jun 4, 2026

env | cut -d= -f1 | grep -E '^[a-z0-9_.-][a-z0-9_.-]*$' | sort -u | while IFS= read -r param; do
[ -n "${param}" ] || continue
/scripts/reload-parameter.sh "${param}" "$(printenv "${param}")"
Copy link
Copy Markdown
Contributor

@wangyelei wangyelei Jun 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only replace param _ to -

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace ^[a-z0-9_.-][a-z0-9_.-]*$ to ^[a-zA-Z0-9_.-]+$

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants