Skip to content

changed container order for Redis#74

Open
henzigo wants to merge 1 commit intojg/warmupfrom
jg/redis-order
Open

changed container order for Redis#74
henzigo wants to merge 1 commit intojg/warmupfrom
jg/redis-order

Conversation

@henzigo
Copy link
Copy Markdown
Member

@henzigo henzigo commented Apr 15, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Redis Deployment container ordering (placing the Redis container before the redis-exporter sidecar) and adjusts deployment scripts and test fixtures to match, ensuring resource-patching logic still targets the intended container.

Changes:

  • Reordered containers in kubernetes/deployments/redis.yaml so redis comes before redis-exporter.
  • Updated deploy/parts/deploy.sh to patch the correct Redis container index after the reorder.
  • Updated multiple scenario “expected” Kubernetes YAML fixtures to reflect the new container order and documented the change in UPGRADE.md.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/scenarios/production-with-cloudflare/expected/migrate-first-deploy.yaml Updates expected Redis pod container order (exporter moved after Redis).
tests/scenarios/production-with-cloudflare/expected/migrate-first-deploy-with-demo-data.yaml Same expected ordering update for demo-data variant.
tests/scenarios/production-with-cloudflare/expected/migrate-continuous-deploy.yaml Same expected ordering update for continuous deploy.
tests/scenarios/escaping-env/expected/migrate-first-deploy.yaml Updates expected Redis pod container order.
tests/scenarios/escaping-env/expected/migrate-first-deploy-with-demo-data.yaml Same expected ordering update for demo-data variant.
tests/scenarios/escaping-env/expected/migrate-continuous-deploy.yaml Same expected ordering update for continuous deploy.
tests/scenarios/development-single-domain/expected/migrate-first-deploy.yaml Updates expected Redis pod container order.
tests/scenarios/development-single-domain/expected/migrate-first-deploy-with-demo-data.yaml Same expected ordering update for demo-data variant.
tests/scenarios/development-single-domain/expected/migrate-continuous-deploy.yaml Same expected ordering update for continuous deploy.
tests/scenarios/basic-production/expected/migrate-first-deploy.yaml Updates expected Redis pod container order.
tests/scenarios/basic-production/expected/migrate-first-deploy-with-demo-data.yaml Same expected ordering update for demo-data variant.
tests/scenarios/basic-production/expected/migrate-continuous-deploy.yaml Same expected ordering update for continuous deploy.
kubernetes/deployments/redis.yaml Reorders redis-exporter to come after redis.
deploy/parts/deploy.sh Updates yq paths to patch Redis resources using the new container index.
UPGRADE.md Notes the Redis container order change for v5.1.0 upgrades.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread deploy/parts/deploy.sh
Comment on lines +57 to +61
yq e -i '.spec.template.spec.containers[0].resources.requests.cpu = "0.01"' "${CONFIGURATION_TARGET_PATH}/deployments/redis.yaml"
yq e -i '.spec.template.spec.containers[0].resources.requests.cpu = "0.01"' "${CONFIGURATION_TARGET_PATH}/deployments/rabbitmq.yaml"

yq e -i '.spec.template.spec.containers[0].resources.requests.memory = "100Mi"' "${CONFIGURATION_TARGET_PATH}/deployments/webserver-php-fpm.yaml"
yq e -i '.spec.template.spec.containers[1].resources.requests.memory = "100Mi"' "${CONFIGURATION_TARGET_PATH}/deployments/redis.yaml"
yq e -i '.spec.template.spec.containers[0].resources.requests.memory = "100Mi"' "${CONFIGURATION_TARGET_PATH}/deployments/redis.yaml"
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

These yq updates target Redis by array index (containers[0]), which is brittle (container order changes require updating this script). Consider selecting the container by name instead so future sidecars/reordering won’t accidentally patch the wrong container.

Copilot uses AI. Check for mistakes.
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.

2 participants