Conversation
Test & Lint Summary
|
Test & Lint Summary (update)
|
noa-lucent
left a comment
There was a problem hiding this comment.
Requested changes after full review.
Blocking items:
- [major] Concurrency group naming does not match the agreed issue specification (
bootstrap-pr-<pr_number>for PRs,bootstrap-mainfor main). Please align this exactly to avoid unintended overlap behavior and keep workflow policy explicit.
Everything else in this PR looks aligned with the requested scope (platform apply, health checks, optional Argo verification, and PR-only reverse destroy).
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete. The previously requested concurrency fix is now implemented correctly with explicit PR/main groups, and the workflow remains aligned with issue #21 scope. Approving.
| TF_VAR_kubeconfig_path: ${{ github.workspace }}/stacks/k8s/.kube/agyn-local-kubeconfig.yaml | ||
| TF_VAR_argocd_server_addr: localhost:8080 | ||
| TF_VAR_argocd_admin_username: admin | ||
| TF_VAR_argocd_admin_password: admin | ||
| TF_VAR_platform_repo_url: https://github.com/agynio/platform.git | ||
| TF_VAR_platform_repo_username: "" | ||
| TF_VAR_platform_repo_password: "" | ||
| TF_VAR_platform_target_revision: v0.15.2 | ||
| TF_VAR_platform_namespace: platform | ||
| TF_VAR_destination_server: https://kubernetes.default.svc | ||
| TF_VAR_platform_server_image_tag: 0.15.2 | ||
| TF_VAR_docker_runner_image_tag: 0.15.2 | ||
| TF_VAR_docker_runner_replica_count: "1" | ||
| TF_VAR_argocd_automated_sync_enabled: "true" | ||
| TF_VAR_argocd_prune_enabled: "true" | ||
| TF_VAR_argocd_self_heal_enabled: "true" | ||
| TF_VAR_platform_db_password: agents | ||
| TF_VAR_platform_db_pvc_size: 5Gi | ||
| TF_VAR_litellm_db_password: change-me | ||
| TF_VAR_litellm_db_pvc_size: 5Gi | ||
| TF_VAR_litellm_master_key: sk-dev-master | ||
| TF_VAR_litellm_salt_key: sk-dev-salt | ||
| TF_VAR_docker_runner_shared_secret: change-me | ||
| TF_VAR_vault_pvc_size: 5Gi | ||
| TF_VAR_registry_mirror_pvc_size: 5Gi |
| runs-on: ubuntu-latest | ||
| timeout-minutes: 45 | ||
| env: | ||
| KUBECONFIG: ${{ github.workspace }}/stacks/k8s/.kube/agyn-local-kubeconfig.yaml |
Local Validation
All commands completed successfully with no failures. |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete. The workflow now uses the strict single health verification script and remains aligned with issue #21 requirements (trigger scope, stack apply order, explicit vars, health gating, PR-only reverse destroy, and concurrency policy). Approving.
| TF_VAR_argocd_server_addr: localhost:8080 | ||
| TF_VAR_argocd_admin_username: admin | ||
| TF_VAR_argocd_admin_password: admin | ||
| TF_VAR_platform_repo_url: https://github.com/agynio/platform.git | ||
| TF_VAR_platform_repo_username: "" | ||
| TF_VAR_platform_repo_password: "" | ||
| TF_VAR_platform_target_revision: v0.15.2 | ||
| TF_VAR_platform_namespace: platform | ||
| TF_VAR_destination_server: https://kubernetes.default.svc | ||
| TF_VAR_platform_server_image_tag: 0.15.2 | ||
| TF_VAR_docker_runner_image_tag: 0.15.2 | ||
| TF_VAR_docker_runner_replica_count: "1" | ||
| TF_VAR_argocd_automated_sync_enabled: "true" | ||
| TF_VAR_argocd_prune_enabled: "true" | ||
| TF_VAR_argocd_self_heal_enabled: "true" | ||
| TF_VAR_platform_db_password: agents | ||
| TF_VAR_platform_db_pvc_size: 5Gi | ||
| TF_VAR_litellm_db_password: change-me | ||
| TF_VAR_litellm_db_pvc_size: 5Gi | ||
| TF_VAR_litellm_master_key: sk-dev-master | ||
| TF_VAR_litellm_salt_key: sk-dev-salt | ||
| TF_VAR_docker_runner_shared_secret: change-me | ||
| TF_VAR_vault_pvc_size: 5Gi |
There was a problem hiding this comment.
delete. apply should work without any additional variables provided
| echo "KUBECONFIG=${{ github.workspace }}/stacks/k8s/.kube/agyn-local-kubeconfig.yaml" >> "$GITHUB_ENV" | ||
| echo "TF_VAR_kubeconfig_path=${{ github.workspace }}/stacks/k8s/.kube/agyn-local-kubeconfig.yaml" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
delete. apply should work without additional variables
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete.\n\nRequesting changes due to one blocking regression:\n- [major] Workflow no longer sets the explicit platform TF_VAR values required by issue #21. The issue/spec explicitly asked to pass these in CI even when defaults exist. Please restore them so behavior remains explicit and requirement-compliant.\n\nEverything else reviewed in this update looks fine.
|
Maintainer request incorporated: workflow now relies solely on Terraform defaults and removes TF_VAR overrides. This updates the acceptance criteria in Issue #21 (see linked comment) — apply should work out of the box with no additional variables. Requesting re-review based on updated criteria. |
noa-lucent
left a comment
There was a problem hiding this comment.
Re-review complete against the revised issue #21 criteria (documented in the maintainer comment). The workflow now correctly relies on Terraform defaults without TF_VAR overrides, while still meeting the required trigger scope, tool setup, apply order, health verification, PR cleanup, timeout, and concurrency behavior. Approving.
| - name: Export kubeconfig for kubectl | ||
| run: | | ||
| echo "KUBECONFIG=${{ github.workspace }}/stacks/k8s/.kube/agyn-local-kubeconfig.yaml" >> "$GITHUB_ENV" | ||
|
|
There was a problem hiding this comment.
This can be used only inside health check. The rest of the flow should work without any extra env variables!
|
CI run https://github.com/agynio/bootstrap_v2/actions/runs/22628550932 finished green after removing all workflow env exports. @rowan-stein this is ready for re-review. |
* ci: expand bootstrap pipeline * ci: fix concurrency format * ci: simplify concurrency group * ci: align concurrency naming * ci: fix platform rollout loop * ci: enforce issue-21 concurrency * feat(ci): harden bootstrap health checks * fix(ci): rely on terraform defaults * fix(ci): drop workflow env exports
Summary
Testing
Fixes #21