Skip to content

Commit 2bb75d6

Browse files
Yolean k8s-qaclaude
andcommitted
Address PR review: error handling, DX, cleanup
- Remove workflow test changes from images.yaml - Remove --dry-run from provisioners (use y-cluster-converge-ystack directly) - Remove kubie null workaround from qemu teardown - Use absolute paths for yconverge calls (no cd to YSTACK_HOME) - y-image-list-ystack: let kustomize errors propagate - kubectl-yconverge: replace grep -c with wc -l, guard file existence in _find_imports, use || : for legitimate empty-string fallbacks - y-cluster-converge-ystack: use absolute paths in _resolve_target Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fe5e0f8 commit 2bb75d6

6 files changed

Lines changed: 20 additions & 60 deletions

File tree

.github/workflows/images.yaml

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,40 +4,12 @@ on:
44
push:
55
branches:
66
- main
7-
- y-converge-checks-dag
87

98
jobs:
10-
checks:
11-
uses: ./.github/workflows/checks.yaml
12-
y-kustomize:
13-
needs: checks
14-
runs-on: ubuntu-latest
15-
permissions:
16-
packages: write
17-
steps:
18-
- uses: actions/checkout@v4
19-
- uses: actions/setup-go@v5
20-
with:
21-
go-version: '1.26'
22-
- name: Login to GitHub Container Registry
23-
uses: docker/login-action@v3
24-
with:
25-
registry: ghcr.io
26-
username: ${{ github.repository_owner }}
27-
password: ${{ secrets.GITHUB_TOKEN }}
28-
- name: Build Go binary
29-
working-directory: y-kustomize/cmd
30-
run: CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags='-s -w' -o target/linux/amd64/y-kustomize .
31-
- name: Build and push image
32-
working-directory: y-kustomize/cmd
33-
env:
34-
YSTACK_HOME: ${{ github.workspace }}
35-
PATH: ${{ github.workspace }}/bin:/usr/local/bin:/usr/bin:/bin
36-
run: |
37-
IMAGE=ghcr.io/yolean/y-kustomize:${{ github.sha }} \
38-
y-contain build --push
9+
lint:
10+
uses: ./.github/workflows/lint.yaml
3911
docker:
40-
needs: checks
12+
needs: lint
4113
runs-on: ubuntu-latest
4214
permissions:
4315
packages: write

bin/kubectl-yconverge

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,11 @@ _find_cue_dirs() {
148148
_DEP_VISITED=""
149149

150150
_find_imports() {
151-
grep '"yolean.se/ystack/' "$1" 2>/dev/null \
151+
[ -f "$1" ] || return 0
152+
grep '"yolean.se/ystack/' "$1" \
152153
| grep -v '"yolean.se/ystack/yconverge/verify"' \
153154
| sed 's|.*"yolean.se/ystack/\([^":]*\).*|\1|' \
154-
|| true # y-script-lint:disable=or-true # no imports is valid
155+
|| :
155156
}
156157

157158
_resolve_deps() {
@@ -182,14 +183,14 @@ ${1%/}"
182183

183184
if [ -z "$_YCONVERGE_RESOLVING" ] && [ -n "$KUSTOMIZE_DIR" ]; then
184185
deps=$(_resolve_deps "$KUSTOMIZE_DIR")
185-
dep_count=$(printf '%s\n' "$deps" | grep -c . 2>/dev/null) || true # y-script-lint:disable=or-true # grep -c . exit 1 = zero matches
186+
dep_count=$(printf '%s\n' "$deps" | wc -l)
186187

187188
if [ "$MODE" = "print-deps" ]; then
188189
printf '%s\n' "$deps"
189190
exit 0
190191
fi
191192

192-
if [ "$dep_count" -gt 1 ] 2>/dev/null; then
193+
if [ "$dep_count" -gt 1 ]; then
193194
echo "=== Converge plan (context=$CONTEXT, mode=$MODE) ==="
194195
echo "Steps ($dep_count):"
195196
for d in $deps; do echo " $d"; done
@@ -237,7 +238,7 @@ if [ -z "$NS_GUESS" ] && [ -n "$KUSTOMIZE_DIR" ]; then
237238
NS_GUESS=$(y-kustomize-traverse -q -o namespace "$KUSTOMIZE_DIR")
238239
fi
239240
if [ -z "$NS_GUESS" ]; then
240-
NS_GUESS=$(kubectl config view --minify --context="$CONTEXT" -o jsonpath='{.contexts[0].context.namespace}' 2>/dev/null) || true # y-script-lint:disable=or-true # context may not exist in kubeconfig
241+
NS_GUESS=$(kubectl config view --minify --context="$CONTEXT" -o jsonpath='{.contexts[0].context.namespace}' 2>/dev/null) || :
241242
fi
242243
[ -z "$NS_GUESS" ] && NS_GUESS="default"
243244
export NAMESPACE="$NS_GUESS"

bin/y-cluster-converge-ystack

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,10 @@ done
3737

3838
export OVERRIDE_IP
3939

40-
cd "$YSTACK_HOME"
41-
4240
_resolve_target() {
43-
for d in k3s/*/; do
44-
local base="${d#k3s/}" # strip k3s/ prefix
45-
base="${base%%/}" # strip trailing /
41+
for d in "$YSTACK_HOME"/k3s/*/; do
42+
local base="${d%/}"
43+
base="${base##*/}" # strip path prefix
4644
base="${base#[0-9][0-9]-}" # strip number prefix (e.g. 40-)
4745
if [ "$base" = "$1" ]; then
4846
echo "$d"

bin/y-cluster-provision-k3d

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ K3D_DOCKER_UPDATE="--cpuset-cpus=3 --cpus=3"
1515
SKIP_CONVERGE=false
1616
SKIP_IMAGE_LOAD=false
1717
CONVERGE_TARGETS="y-kustomize,blobs,builds-registry"
18-
DRY_RUN=""
1918

2019
while [ $# -gt 0 ]; do
2120
case "$1" in
@@ -32,7 +31,6 @@ Flags:
3231
--converge=LIST comma-separated k3s bases to converge (default: y-kustomize,blobs,builds-registry)
3332
--skip-converge skip converge, validate, and post-provision steps
3433
--skip-image-load skip image cache and load into containerd
35-
--dry-run=MODE forward to kubectl-yconverge (server|none)
3634
--teardown delete existing cluster and exit
3735
-h, --help show this help
3836
EOF
@@ -45,7 +43,6 @@ EOF
4543
--converge=*) CONVERGE_TARGETS="${1#*=}"; shift ;;
4644
--skip-converge) SKIP_CONVERGE=true; shift ;;
4745
--skip-image-load) SKIP_IMAGE_LOAD=true; shift ;;
48-
--dry-run=*) DRY_RUN="$1"; shift ;;
4946
--teardown) TEARDOWN=true; shift ;;
5047
*) echo "Unknown flag: $1" >&2; exit 1 ;;
5148
esac
@@ -124,8 +121,8 @@ until kubectl --context=$CTX get nodes >/dev/null 2>&1; do sleep 2; done
124121

125122
# Gateway API is always set up, even with --skip-converge.
126123
export OVERRIDE_IP=${YSTACK_PORTS_IP:-127.0.0.1}
127-
(cd "$YSTACK_HOME" && kubectl-yconverge --context=$CTX -k k3s/10-gateway-api/)
128-
(cd "$YSTACK_HOME" && kubectl-yconverge --context=$CTX -k k3s/20-gateway/)
124+
kubectl yconverge --context=$CTX -k "$YSTACK_HOME/k3s/10-gateway-api/"
125+
kubectl yconverge --context=$CTX -k "$YSTACK_HOME/k3s/20-gateway/"
129126

130127
if [ "$SKIP_CONVERGE" = "true" ]; then
131128
echo "# --skip-converge: skipping converge, validate, and post-provision steps"
@@ -142,7 +139,7 @@ else
142139
y-image-cache-load-all </dev/null || true
143140
fi
144141

145-
y-cluster-converge-ystack --context=$CTX --converge=$CONVERGE_TARGETS $DRY_RUN --override-ip=${YSTACK_PORTS_IP:-127.0.0.1}
142+
y-cluster-converge-ystack --context=$CTX --converge=$CONVERGE_TARGETS --override-ip=${YSTACK_PORTS_IP:-127.0.0.1}
146143

147144
# Registry resolution uses magic ClusterIPs in registries.yaml — no /etc/hosts needed.
148145
# TODO: add containerd registry access verification (like qemu provisioner)

bin/y-cluster-provision-qemu

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ VM_SSH_PORT="2222"
1616
SKIP_CONVERGE=false
1717
SKIP_IMAGE_LOAD=false
1818
CONVERGE_TARGETS="y-kustomize,blobs,builds-registry"
19-
DRY_RUN=""
2019

2120
while [ $# -gt 0 ]; do
2221
case "$1" in
@@ -37,7 +36,6 @@ Flags:
3736
--converge=LIST comma-separated k3s bases to converge (default: y-kustomize,blobs,builds-registry)
3837
--skip-converge skip converge and post-provision steps
3938
--skip-image-load skip image cache and load into containerd
40-
--dry-run=MODE forward to kubectl-yconverge (server|none)
4139
--teardown stop and delete the VM (removes disk by default)
4240
--keep-disk preserve disk image on teardown for faster re-provision
4341
--export-vmdk=PATH export disk as streamOptimized VMDK
@@ -55,7 +53,6 @@ EOF
5553
--converge=*) CONVERGE_TARGETS="${1#*=}"; shift ;;
5654
--skip-converge) SKIP_CONVERGE=true; shift ;;
5755
--skip-image-load) SKIP_IMAGE_LOAD=true; shift ;;
58-
--dry-run=*) DRY_RUN="$1"; shift ;;
5956
--teardown) TEARDOWN=true; shift ;;
6057
--keep-disk) KEEP_DISK=true; shift ;;
6158
--export-vmdk=*) EXPORT_VMDK="${1#*=}"; shift ;;
@@ -127,10 +124,6 @@ if [ "$TEARDOWN" = "true" ]; then
127124
rm -f "$VM_DISK"
128125
echo "[y-cluster-provision-qemu] Teardown complete. Disk deleted."
129126
fi
130-
# Fix kubectl writing null instead of [] when last item is removed
131-
sed -i 's/^contexts: null$/contexts: []/' "$KUBECONFIG" 2>/dev/null
132-
sed -i 's/^clusters: null$/clusters: []/' "$KUBECONFIG" 2>/dev/null
133-
sed -i 's/^users: null$/users: []/' "$KUBECONFIG" 2>/dev/null
134127
exit 0
135128
fi
136129

@@ -252,8 +245,8 @@ y-kubeconfig-import "$KUBECONFIG.tmp"
252245
# Gateway API is always set up, even with --skip-converge.
253246
# Services are reachable via port-forward at 127.0.0.1.
254247
export OVERRIDE_IP=127.0.0.1
255-
(cd "$YSTACK_HOME" && kubectl-yconverge --context=$CTX -k k3s/10-gateway-api/)
256-
(cd "$YSTACK_HOME" && kubectl-yconverge --context=$CTX -k k3s/20-gateway/)
248+
kubectl yconverge --context=$CTX -k "$YSTACK_HOME/k3s/10-gateway-api/"
249+
kubectl yconverge --context=$CTX -k "$YSTACK_HOME/k3s/20-gateway/"
257250

258251
if [ "$SKIP_CONVERGE" = "true" ]; then
259252
echo "[y-cluster-provision-qemu] --skip-converge: done"
@@ -270,7 +263,7 @@ fi
270263

271264
# Converge ystack infrastructure (includes Gateway API and /etc/hosts via y-k8s-ingress-hosts)
272265
# Use 127.0.0.1 as override IP since services are reachable via port-forward
273-
y-cluster-converge-ystack --context=$CTX --converge=$CONVERGE_TARGETS $DRY_RUN --override-ip=127.0.0.1
266+
y-cluster-converge-ystack --context=$CTX --converge=$CONVERGE_TARGETS --override-ip=127.0.0.1
274267

275268
# Verify containerd can reach registries via mirror config (magic ClusterIPs)
276269
echo "[y-cluster-provision-qemu] Verifying containerd registry access ..."

bin/y-image-list-ystack

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,9 @@ for target in $(echo "$CONVERGE_TARGETS" | tr ',' ' '); do
2020
base="${base##*/}"
2121
base="${base#[0-9][0-9]-}"
2222
[ "$base" = "$target" ] || continue
23-
kubectl kustomize "$d" 2>/dev/null \
23+
kubectl kustomize "$d" \
2424
| grep -oE 'image:\s*\S+' \
25-
| sed 's/image:[[:space:]]*//' \
26-
|| true # y-script-lint:disable=or-true # kustomize may fail for bases requiring y-kustomize HTTP
25+
| sed 's/image:[[:space:]]*//'
2726
break
2827
done
2928
done | sort -u

0 commit comments

Comments
 (0)