Skip to content

cluster: migrate registry from host container to in-cluster deployment#3856

Open
lkingland wants to merge 1 commit into
knative:mainfrom
lkingland:push-qtztqtnuqqqy
Open

cluster: migrate registry from host container to in-cluster deployment#3856
lkingland wants to merge 1 commit into
knative:mainfrom
lkingland:push-qtztqtnuqqqy

Conversation

@lkingland
Copy link
Copy Markdown
Member

@lkingland lkingland commented May 26, 2026

Summary

Migrates func cluster create from running a standalone func-registry container on the host to deploying the registry as in-cluster Kubernetes resources, matching the approach already taken in hack/cluster.sh (PR #3718).

What changed:

  • Registry is now a Deployment + ClusterIP Service + Contour Ingress inside the Kind cluster (was a host-side docker run container with ExternalName Service)
  • registryAddr is registry.localtest.me (was localhost:50000)
  • Containerd mirrors point at http://localhost:5000 via hostPort (was http://func-registry:5000 via Docker network DNS)
  • Each cluster owns its own registry, destroyed automatically by kind delete cluster
  • Delete flow simplified: no shared container teardown needed
  • Removed: ensureRegistry, registryStatus, teardownRegistry, setupPodmanMacOSForwarding, and all host-container lifecycle constants

What's preserved:

  • Host trust config (Docker daemon.json / Podman registries.conf) — now uses registry.localtest.me
  • --skip-registry-config flag behavior unchanged
  • Last-cluster check for host trust revert
  • Kubeconfig stat guard in Delete() to suppress noisy warnings on orphan-cleanup path

@knative-prow knative-prow Bot requested review from dsimansk and jrangelramos May 26, 2026 07:53
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented May 26, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lkingland

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow Bot added approved 🤖 PR has been approved by an approver from all required OWNERS files. size/L 🤖 PR changes 100-499 lines, ignoring generated files. labels May 26, 2026
@lkingland lkingland force-pushed the push-qtztqtnuqqqy branch from f96a48b to 0470fa1 Compare May 26, 2026 08:00
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 1.06383% with 93 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.89%. Comparing base (da6327e) to head (0470fa1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/cluster/registry.go 0.00% 71 Missing ⚠️
pkg/cluster/delete.go 0.00% 14 Missing ⚠️
cmd/ci/workflow.go 12.50% 7 Missing ⚠️
pkg/cluster/kubernetes.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3856      +/-   ##
==========================================
- Coverage   53.43%   52.89%   -0.54%     
==========================================
  Files         200      200              
  Lines       23405    23388      -17     
==========================================
- Hits        12506    12372     -134     
- Misses       9649     9766     +117     
  Partials     1250     1250              
Flag Coverage Δ
e2e 37.17% <0.00%> (+3.47%) ⬆️
e2e go 29.61% <0.00%> (+<0.01%) ⬆️
e2e node 25.90% <0.00%> (+<0.01%) ⬆️
e2e python 29.94% <0.00%> (+<0.01%) ⬆️
e2e quarkus 26.00% <0.00%> (+0.02%) ⬆️
e2e rust 25.47% <0.00%> (+<0.01%) ⬆️
e2e springboot 24.14% <0.00%> (+<0.01%) ⬆️
e2e typescript 25.99% <0.00%> (+<0.01%) ⬆️
e2e-config-ci 26.84% <1.19%> (+<0.01%) ⬆️
integration 15.77% <0.00%> (-0.02%) ⬇️
unit macos-14 42.25% <0.00%> (+<0.01%) ⬆️
unit macos-latest 42.25% <0.00%> (+<0.01%) ⬆️
unit ubuntu-24.04-arm 42.57% <0.00%> (+0.03%) ⬆️
unit ubuntu-latest 43.12% <0.00%> (+<0.01%) ⬆️
unit windows-latest 42.32% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread CLAUDE.local.md Outdated
Comment thread pkg/cluster/registry.go

if err := ensureRegistry(ctx, cfg, out); err != nil {
return err
registryManifest := `apiVersion: apps/v1
Copy link
Copy Markdown
Contributor

@matejvasek matejvasek May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible improvement: make this Go struct instead of string literal.

Comment thread pkg/cluster/registry.go
return fmt.Errorf("applying registry service: %w", err)
}

success(out, "Registry", time.Since(start))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potential improvement: we could wait until http://registry.localtest.me/v2/ returns 200.

Comment thread pkg/cluster/registry.go
// installRegistry deploys the container registry as in-cluster Kubernetes
// resources (Deployment + ClusterIP Service + Contour Ingress), configures
// host-side trust, and applies the local-registry-hosting ConfigMap.
func installRegistry(ctx context.Context, cfg ClusterConfig, out io.Writer) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure when/how installRegistry() is called but it preferably should be called after Contour installation (or in parallel to it). But it's not necessary.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's not necessary.

Because we use generic k8s ingress not some specific Contour resource.

@matejvasek
Copy link
Copy Markdown
Contributor

matejvasek commented May 26, 2026

unrelated to this PR: why we use exec() instead of using Go code, kind/kubectl is Go code. Also why call docker via CLI exec()? Why not API? We already extensively use k8s and docker API already.

Replace the shared host-side func-registry container with Kubernetes-native
resources deployed inside each Kind cluster:

- Deployment (registry:2 with hostPort 5000 + emptyDir volume)
- ClusterIP Service (port 5000)
- Contour Ingress at registry.localtest.me

Key changes:
- registryAddr is now "registry.localtest.me" (was "localhost:50000")
- containerd mirrors point at http://localhost:5000 via hostPort (was
  http://func-registry:5000 via Docker network DNS)
- Each cluster owns its own registry, destroyed with kind delete cluster
- Delete flow simplified: no shared container teardown, just host trust
  revert on last-cluster removal
- Removed: ensureRegistry, registryStatus, teardownRegistry,
  setupPodmanMacOSForwarding, and all host-container lifecycle code
@lkingland lkingland force-pushed the push-qtztqtnuqqqy branch from 0470fa1 to 05b13f2 Compare May 26, 2026 12:45
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented May 26, 2026

@lkingland: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
unit-tests_func_main 05b13f2 link true /test unit-tests

Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@lkingland
Copy link
Copy Markdown
Member Author

unrelated to this PR: why we use exec() instead of using Go code, kind/kubectl is Go code. Also why call docker via CLI exec()? Why not API? We already extensively use k8s and docker API already.

Exactly right! This is simply a stepping-stone. Translating script->Go code with this structure is much less error-prone. Next step is to replace the exec's with code wherever possible! Same way we did with the builders: wrap the cli, get it working, replace with the stdlib 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved 🤖 PR has been approved by an approver from all required OWNERS files. size/L 🤖 PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants