Conversation
There was a problem hiding this comment.
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.yamlsorediscomes beforeredis-exporter. - Updated
deploy/parts/deploy.shto 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.
| 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" |
There was a problem hiding this comment.
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.
No description provided.