Pr/deploy on kind fixes - #4390
Open
cyrano-janus wants to merge 9 commits into
Open
Conversation
…ILD block helm upgrade referenced $chart_dir and $values_file unconditionally, but both were only set inside the docker-build branch. Running with SKIP_DOCKER_BUILD=1 crashed with an empty chart path. Now the chart dir is always prepared; SKIP_DOCKER_BUILD just skips kbld image resolution and kind load. helm dependency update now runs against the copied chart dir instead of helm/korifi. Signed-off-by: CyEs <cyrano.janus@gmail.com>
git describe --tags --long fails on shallow clones or repos without tags, killing the whole script under set -e. Fall back to the short commit SHA as chart version. Signed-off-by: CyEs <cyrano.janus@gmail.com>
- trap: single-quote the cleanup command so $chart_dir expands at signal time, not at trap registration (SC2064) - VERSION: declare and assign separately so a failing git describe cannot be masked by export's return code (SC2155) Signed-off-by: CyEs <cyrano.janus@gmail.com>
The SC2064 fix made the trap expand $chart_dir at signal time, but the variable was local to deploy_korifi — by the time the RETURN trap fires the local is gone and set -u aborts with 'chart_dir: unbound'. Drop 'local' so the quoted expansion inside the trap still resolves. Signed-off-by: CyEs <cyrano.janus@gmail.com>
A broken or auth-misconfigured registry only surfaced much later as cryptic kpack build failures (EOF, 401). Poll the registry from inside the kind node (curl against 127.0.0.1:30050, node has no cluster DNS) and fail fast with a clear message if it is not reachable or the user/password credentials are rejected. Signed-off-by: CyEs <cyrano.janus@gmail.com>
api.apiServer.url feeds three places: the Gateway https-api listener hostname, the korifi-api externalFQDN and the generated ingress cert SAN. Hardcoding 'localhost' causes SNI mismatches (connection EOF) for anyone reaching the API under a different name such as api.korifi.local. Default remains 'localhost' so upstream CI behaviour is unchanged; override with API_SERVER_FQDN=api.korifi.local to match an /etc/hosts entry. Signed-off-by: CyEs <cyrano.janus@gmail.com>
A missing /etc/hosts entry for the API FQDN surfaces to users as a confusing connection EOF from cf/curl. Verify resolution via getent at the end of the deploy and print the exact fix instead. Skipped for the default localhost setup. Signed-off-by: CyEs <cyrano.janus@gmail.com>
After deploying, poll https://<fqdn>:<port>/v3/info through the kind hostPort mapping and fail with concrete troubleshooting hints if the API is not reachable. On success print the cf api / cf auth commands. FQDN and port default to localhost:443 and are read from the same API_SERVER_FQDN / API_SERVER_PORT env vars used elsewhere. Signed-off-by: CyEs <cyrano.janus@gmail.com>
kind's --wait 5m flag times out intermittently in our test environment, leaving the script in a half-deployed state. Replace it with a 0m wait plus a manual loop that polls for node Ready state with 10s intervals up to ~5min. This is functionally equivalent but exits cleanly on success and on timeout, so the rest of the script can proceed or abort. Refs: tracking issue with kind timeout on Korifi deploys.
Member
|
Korifi project is currently being archived, see RFC 0060. The repo archival is pending. Provided that the CI infrastructure is already down, I am afraid your PR cannot be merged. FWIW, we have never advertised deploy-on-kind to be the official way of installing korifi on kind. It is just a dev tool we use daily to bring up local test/dev environment. Therefore we have always tried to keep it lean (even at the cost of cryptic errors) and just good enough for our daily job. We never meant the script to be robust in any way |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
+# PR: fix(deploy-on-kind): make fresh deployments work reliably
+
+Title:
fix(deploy-on-kind): make fresh deployments work reliably+
+Base:
main← Compare:cyrano-janus:pr/deploy-on-kind-fixes+
+---
+
+## Description
+
+While deploying Korifi on a fresh kind cluster we hit several issues in
+
scripts/deploy-on-kind.sh. This PR fixes them incrementally (8 small+commits) and adds fail-fast preflight checks plus an end-to-end smoke test,
+so failures surface with actionable messages instead of cryptic kpack/EOF
+errors minutes later.
+
+## Fixes
+
+1. chart_dir scope bug:
helm upgradereferenced$chart_dir/$values_fileunconditionally, but both were only set inside theSKIP_DOCKER_BUILD=1.SKIP_DOCKER_BUILDonly skips kbldkind load.+2. VERSION fallback:
git describe --tags --longfails on shallowset -e.+3. shellcheck SC2064: single-quote the RETURN-trap command so
$chart_direxpands at signal time, not at trap registration.+4. set -u vs RETURN trap: the trap variable must not be
local— by theset -uaborts cleanup withchart_dir: unbound.+5. shellcheck SC2155: declare-and-assign separately for
VERSIONso agit describecannot be masked byexport's exit code.+6. Registry preflight: after deploying the local registry, poll it from
curl -u user:password http://127.0.0.1:30050/v2/)127.0.0.1.+7. Configurable API FQDN (
API_SERVER_FQDN, defaultlocalhost):/etc/hostsline when the name does not resolve.+8. API smoke test: at the end of the deploy, poll
https://<fqdn>:<port>/v3/infothrough the kind hostPort mapping; oncf api/cf authcommands.+## Testing
+
+-
bash -n, shellcheck 0.10 (-S warning),shfmt -d -i 2 -ci: clean+- Fresh kind cluster from scratch with `SKIP_DOCKER_BUILD=1
+- Full workflow verified end to end:
cf api→cf auth cf-admin admin→cf create-org/space→cf pushof a Go app → app reachable via route+- Idempotent re-run of the script against an existing cluster: exit 0
+## Notes
+
+- Default behaviour (
API_SERVER_FQDN=localhost) is unchanged, so CI flows+- Happy to split this into separate PRs if preferred — commits are already