Skip to content

log relative paths for yconverge dependencies#5

Merged
solsson merged 1 commit intomainfrom
yconverge-dep-relative-paths
Apr 29, 2026
Merged

log relative paths for yconverge dependencies#5
solsson merged 1 commit intomainfrom
yconverge-dep-relative-paths

Conversation

@solsson
Copy link
Copy Markdown
Contributor

@solsson solsson commented Apr 29, 2026

Previously we'd log the path from the cue module root, but we kubectl yconverge relative paths to other repositories so that could be confusing.

…ines

The four progress headers reported the CUE-module-root-relative
form, which surprised when the user's CWD is upstream of the
module root. Example before this change:

  $ cd y-cluster
  $ y-cluster yconverge -k ../ystack/yconverge/itest/example-replace-dependent
  yconverge dependency yconverge/itest/example-replace
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  cannot `cd` to that path from the user's shell -- it's
  relative to a module root the user didn't name.

After:

  $ cd y-cluster
  $ y-cluster yconverge -k ../ystack/yconverge/itest/example-replace-dependent
  yconverge dependency ../ystack/yconverge/itest/example-replace
                       (same shape as the -k arg; `cd` works)

Implementation: a small userPath() helper that wraps
filepath.Rel against os.Getwd(). Falls back to the absolute
path on lookup failure (rare; cross-drive paths on Windows).

The diagnostic zap.Debug log lines keep RelPath(cueRoot, step) --
structured fields meant for log aggregation are more useful
in module-relative form, which doesn't depend on cwd-at-log-time.
The CUE-module-root projection there carries significance
(matches CUE's import-path semantics) and stays.

Test coverage:
- pkg/yconverge/userpath_test.go: relative-to-cwd common case
  and the traverse-up `..` case.
- e2e/converge_progress_test.go: assertions switched from full
  string match to substring match on the user-meaningful suffix
  (/base, /dependent), because tests run from the e2e/ dir and
  fixtures live in t.TempDir() so the path traverses up
  through ../../tmp/.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@solsson solsson merged commit afc0cb5 into main Apr 29, 2026
11 checks passed
solsson added a commit that referenced this pull request May 1, 2026
Address review nit #5 from #12: lock in the deadline path and the
ctx-cancel path with a fake-kubectl-on-PATH harness.

To run the loop in milliseconds rather than the production 60s,
the body of waitForHostAPIServer is split into pollHostAPIServerReadyz
which takes the timeout and interval as arguments. Production keeps
the same effective behaviour via const wrappers; the test drives the
helper directly with sub-second knobs.

Three cases:
- success: kubectl exits 0, returns nil immediately.
- deadline-honored: always-failing kubectl, expect the wrapped
  "/readyz never returned 200" error (not ctx.Err()) and the context
  name in the message.
- ctx-cancelled: 50ms ctx vs 10s loop timeout, expect ctx.Err()
  (not the deadline message) -- guards against a refactor that
  drops the select { <-ctx.Done() } branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant