fix(e2e): make test-e2e actually pass - #6

config/manager/manager.yaml hardcoded a colleague's personal Docker Hub
image (docker.io/psarwate/pf9-cluster-api-byoh-controller:dev) instead
of the canonical gcr.io/k8s-staging-cluster-api placeholder, introduced
in 42bf3c9 over a year ago. make test-e2e's cluster-templates-e2e target
never runs `kustomize edit set image` (only deploy/build-installer do),
so clusterctl init during e2e has been deploying that stray pre-built
image regardless of what's actually in the source tree -- this also
breaks current upstream CI the same way.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The host-ownership check added in platform9#115 hardcoded a single allowed
identity, system:serviceaccount:kaapi:byoh-controller-manager --
"kaapi" being Platform9's production deployment namespace. The OSS
default (config/default/kustomization.yaml: namespace byoh-system)
resolves to a different service account string, so every legitimate
controller-initiated ByoHost patch was denied with "cannot
create/update resource <host>". Allowlist both identities explicitly
rather than hardcoding one namespace, and cover both in the existing
table-driven test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…v1.31.0

The e2e fixtures pointed at projects.registry.vmware.com/cluster_api_
provider_bringyourownhost, which has zero hosted bundles at any OS or
k8s version -- confirmed by probing the registry directly, and by
current upstream main's own CI failing the same way. Real bundles are
hosted at quay.io/platform9, built via .ci/build-push-bundle.sh; v1.31.0
exists there for both Ubuntu 20.04 and 22.04 and matches what
installer/registry.go already declares support for, so used that
instead of also touching the registry's k8s-version filter. Regenerated
the CRD docs and the e2e cluster-template.yaml kustomize output to
match (via `make manifests` and `make cluster-templates-e2e`).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

kubeadm's SystemVerification check tries to modprobe the "configs"
kernel module to read kernel config flags; on stock Ubuntu generic
kernels CONFIG_IKCONFIG is compiled out entirely (confirmed against
kubernetes/kubeadm#501, kubernetes-sigs/kind#61), and the byoh host
containers don't have /boot bind-mounted for kubernetes/system-
validators' earlier fallback path to find it directly. The existing
templates already ignore other container-specific preflight checks
(Swap, DirAvailable, FileAvailable) for this identical reason -- added
SystemVerification to that precedented list, and to the two worker
join configs that had no ignorePreflightErrors at all.

…strap

kube-vip's static pod couldn't win its leader election to announce the
control-plane VIP: "leases.coordination.k8s.io \"plndr-cp-lock\" is
forbidden: User \"kubernetes-admin\" cannot get resource \"leases\"".
Since Kubernetes 1.29, kubeadm's admin.conf is scoped to a revocable
"kubeadm:cluster-admins" RBAC group instead of the old system:masters
bypass, and that RBAC binding isn't guaranteed to exist yet at the
point kube-vip's static pod starts -- a well-known kube-vip/kubeadm
bootstrap deadlock (kube-vip/kube-vip#684). super-admin.conf (also
generated by kubeadm) keeps the unconditional system:masters group.

kube-vip v0.5.0's manager.New() hardcodes the path
"/etc/kubernetes/admin.conf" as a literal string (verified directly
against pkg/manager/manager.go in the kube-vip source) -- it does not
read whatever path the volume happens to be mounted at. So the
container-side mountPath must stay /etc/kubernetes/admin.conf; only
the host-side hostPath.path is redirected to super-admin.conf, so the
container still finds a file at the path it expects, but with
system:masters permissions instead of the scoped ones.

kube-proxy (iptables mode) crash-looped with "too many open files" on
every byoh host container -- Docker's default nofile ulimit (1024) is
well known to be insufficient for kube-proxy's iptables/netlink usage,
which is exactly why kind explicitly raises it for its own nodes.
createDockerContainer() set no Ulimits at all, so containers inherited
Docker's default. Added an explicit nofile soft/hard limit of 1048576,
matching kind's convention. github.com/docker/go-units was already an
indirect dependency; promoted to direct via go mod tidy.

kube-proxy inside a privileged byoh host container reads the host's
current net.netfilter.nf_conntrack_max fine but gets "permission
denied" trying to raise it -- it's a global, non-namespaced sysctl,
so a container (even privileged) can't increase it, only read it.
kube-proxy computed a target of 524288 on devbox (whose default was
262144) and crashed attempting the write. Persisting a higher value
on the host means kube-proxy's own check (only write if current is
below what it wants) finds the host already sufficient and skips the
write entirely. Set to 1048576 for headroom, alongside this script's
existing bridge-nf/ip_forward sysctls.

Running multiple byohost containers alongside the management kind cluster
on a single devbox host exhausts the host's per-UID kernel limit for inotify
instances (default 128). When exhausted, containerd's CRI plugin fails to load
with "too many open files" (from inotify_init returning EMFILE), which cascades
into kubelet failing with "unknown service runtime.v1.RuntimeService" and
kubeadm join hanging against a dead CRI socket.

Raise the limit to 8192 inside each container after start via sysctl. Docker's
--sysctl flag does not accept fs.inotify.* sysctls at container-create time
(unlike net.* sysctls), so this must be a live exec-based write after the
container starts.

This is one fix in a chain of e2e test-runner improvements.

kube-proxy's conntrack tuning tries to raise net.netfilter.nf_conntrack_max,
which is a global (non-namespaced) Linux sysctl. Containers cannot write this
sysctl even if the host's value is already raised; writes always fail with
"permission denied". This crash-loops kube-proxy indefinitely, leaving the
control-plane Node stuck at NotReady.

CAPI's KubeadmControlPlane and KubeadmConfig types have no field for
KubeProxyConfiguration (kubernetes-sigs/cluster-api#4512). Instead, inject
the configuration directly into kubeadm's config file before kubeadm init/join
runs, so the tuning is disabled from the start.

Verified: docker exec confirmed the KubeProxyConfiguration block lands in
/run/kubeadm/kubeadm.yaml, and the e2e run showed the control plane reaching
ready state within ~1 minute.

cluster-template.yaml is regenerated output from cluster-with-kcp.yaml via
`make cluster-templates-e2e` and is not hand-edited.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Fix Go closure semantics bug in e2e test log file closing. The byohost
agent log files (agentLogFile1 and agentLogFile2) are written to a single
variable `f` that is reassigned between WriteDockerLog calls. The original
closures captured `f` by reference, so both defer blocks read the final
value of `f` (whichever file was assigned last).

This caused the first file (agentLogFile1) to never close — a leaked file
descriptor on every test run — and the second file (agentLogFile2) to be
closed twice, producing a confusing error message where the filename shown
did not match the file actually being closed.

Passing `f` as a parameter to the defer closure forces immediate evaluation,
binding each closure to the correct file.

Note: This bug does not explain the unrelated e2e cluster-deletion timeout
in CI; Docker exec stream closes for both hosts work correctly regardless.
This is a real but independent resource-leak fix.

… hang

TEMPORARY DIAGNOSTIC COMMIT — expected to be reverted after testing.

Temporarily comments out the kernel module cleanup (modprobe -rq overlay
and modprobe -r br_netfilter) in the e2e uninstall script to test whether
this step is causing the CI cluster-deletion hang observed in GitHub Actions
(WaitForClusterDeleted timing out at ~906s, reproduced 2/2 times).

This is not intended to be merged as-is. Once CI testing confirms or rules
out this as the root cause, this change must be reverted before any PR
approval.

The e2e-pr-blocking job was hitting GitHub API rate limits when
clusterctl init fetched cert-manager.yaml unauthenticated. Adding
GITHUB_TOKEN at the job level allows clusterctl to authenticate its
API calls and access the higher rate limit for authenticated requests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

When e2e ByoHosts run in containers, they share the kernel with Docker.
The uninstall script unconditionally unloads overlay and br_netfilter
modules, breaking Docker's bridge networking and causing cluster deletion
hangs with TLS timeouts (~900s wait timeout).

Introduce BYOH_SKIP_KERNEL_MODULE_CLEANUP env var (manager-level only,
not a CRD field, to avoid polluting the public API) to conditionally
guard the modprobe unload. Defaults to false via clusterctl's variable
substitution in manager.yaml — production behavior unchanged. Only e2e's
test config (provider.yaml) sets it to true.

Thread the flag through main.go → K8sInstallerConfigReconciler →
installer.NewInstaller → OS-specific constructors → NewBaseUbuntuInstaller.

Add table-driven unit tests in common_ubuntu_test.go asserting the
modprobe line is present by default and absent when the flag is set.
Update existing test call sites in installer_test.go for the new
NewInstaller signature.

Verified: go build, go vet, go test ./installer/... all pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Replace 'true'/'false' with 'enable'/'disable' strings to match the
existing MANUAL_CSR_APPROVAL convention and prevent YAML boolean parsing
issues in clusterctl image overrides.

When kustomize re-serializes the deployment manifest, unquoted YAML booleans
are parsed as native Go bool types. This breaks clusterctl's Unstructured
conversion when it expects corev1.EnvVar.Value (a string field). Using
'enable'/'disable' ensures the values remain parsed as strings, matching
the convention already established by MANUAL_CSR_APPROVAL.