diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 39eb0fcc7..106f0c8a1 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -14,6 +14,8 @@ on: jobs: e2e-pr-blocking: runs-on: ubuntu-22.04 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} steps: - name: Checkout code uses: actions/checkout@v3 diff --git a/apis/infrastructure/v1beta1/byocluster_types.go b/apis/infrastructure/v1beta1/byocluster_types.go index e128df7d5..1cd2ab593 100644 --- a/apis/infrastructure/v1beta1/byocluster_types.go +++ b/apis/infrastructure/v1beta1/byocluster_types.go @@ -22,7 +22,7 @@ type ByoClusterSpec struct { ControlPlaneEndpoint APIEndpoint `json:"controlPlaneEndpoint"` // BundleLookupBaseRegistry is the base Registry URL that is used for pulling byoh bundle images, - // if not set, the default will be set to https://projects.registry.vmware.com/cluster_api_provider_bringyourownhost + // if not set, the default will be set to https://quay.io/platform9 // +optional BundleLookupBaseRegistry string `json:"bundleLookupBaseRegistry,omitempty"` } diff --git a/apis/infrastructure/v1beta1/byohost_webhook.go b/apis/infrastructure/v1beta1/byohost_webhook.go index 998ccbac2..6e2872f91 100644 --- a/apis/infrastructure/v1beta1/byohost_webhook.go +++ b/apis/infrastructure/v1beta1/byohost_webhook.go @@ -25,8 +25,18 @@ type ByoHostValidator struct { decoder *admission.Decoder } -// To allow byoh manager service account to patch ByoHost CR -const managerServiceAccount = "system:serviceaccount:kaapi:byoh-controller-manager" +// The byoh-controller-manager's namespace differs by deployment: "byoh-system" is the OSS +// default (config/default, e2e), "kaapi" is the PF9 production deployment. Both identities +// are allowlisted to bypass the per-agent host-ownership check below. +const ( + kaapiManagerServiceAccount = "system:serviceaccount:kaapi:byoh-controller-manager" + byohSystemManagerServiceAccount = "system:serviceaccount:byoh-system:byoh-controller-manager" +) + +var managerServiceAccounts = map[string]bool{ + kaapiManagerServiceAccount: true, + byohSystemManagerServiceAccount: true, +} // Precompile email-like regex for efficiency var emailLikeUserRegex = regexp.MustCompile(`^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$`) @@ -55,7 +65,7 @@ func (v *ByoHostValidator) handleCreateUpdate(req *admission.Request) admission. } userName := req.UserInfo.Username // allow manager service account to patch ByoHost - if userName == managerServiceAccount { + if managerServiceAccounts[userName] { return admission.Allowed("") } diff --git a/apis/infrastructure/v1beta1/byohost_webhook_internal_test.go b/apis/infrastructure/v1beta1/byohost_webhook_internal_test.go index e95030653..c3744d00c 100644 --- a/apis/infrastructure/v1beta1/byohost_webhook_internal_test.go +++ b/apis/infrastructure/v1beta1/byohost_webhook_internal_test.go @@ -134,7 +134,7 @@ var _ = Describe("ByohostWebhook/Unit", func() { It("Should allow update request from manager", func() { admissionRequest := admissionv1.AdmissionRequest{ Operation: admissionv1.Update, - UserInfo: v1.UserInfo{Username: managerServiceAccount}, + UserInfo: v1.UserInfo{Username: byohSystemManagerServiceAccount}, Object: runtime.RawExtension{ Raw: byoHostRaw, Object: byoHost, @@ -270,8 +270,13 @@ func TestByoHostValidator_handleCreateUpdate(t *testing.T) { wantMsg string }{ { - name: "manager service account bypasses the ownership check", - userName: managerServiceAccount, + name: "byoh-system manager service account bypasses the ownership check", + userName: byohSystemManagerServiceAccount, + wantAllow: true, + }, + { + name: "kaapi manager service account bypasses the ownership check", + userName: kaapiManagerServiceAccount, wantAllow: true, }, { diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclusters.yaml index c87e67c6a..5fc35a9ca 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclusters.yaml @@ -52,7 +52,7 @@ spec: bundleLookupBaseRegistry: description: |- BundleLookupBaseRegistry is the base Registry URL that is used for pulling byoh bundle images, - if not set, the default will be set to https://projects.registry.vmware.com/cluster_api_provider_bringyourownhost + if not set, the default will be set to https://quay.io/platform9 type: string controlPlaneEndpoint: description: ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclustertemplates.yaml index 343482169..e4dba8e84 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_byoclustertemplates.yaml @@ -81,7 +81,7 @@ spec: bundleLookupBaseRegistry: description: |- BundleLookupBaseRegistry is the base Registry URL that is used for pulling byoh bundle images, - if not set, the default will be set to https://projects.registry.vmware.com/cluster_api_provider_bringyourownhost + if not set, the default will be set to https://quay.io/platform9 type: string controlPlaneEndpoint: description: ControlPlaneEndpoint represents the endpoint used to communicate with the control plane. diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 4579a1590..8a3ee143e 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -28,10 +28,12 @@ spec: env: - name: MANUAL_CSR_APPROVAL value: "${MANUAL_CSR_APPROVAL:=disable}" + - name: BYOH_SKIP_KERNEL_MODULE_CLEANUP + value: "${BYOH_SKIP_KERNEL_MODULE_CLEANUP:=disable}" args: - --enable-leader-election - "--metrics-bind-addr=127.0.0.1:8080" - image: docker.io/psarwate/pf9-cluster-api-byoh-controller:dev + image: gcr.io/k8s-staging-cluster-api/cluster-api-byoh-controller:dev name: manager resources: limits: diff --git a/controllers/infrastructure/k8sinstallerconfig_controller.go b/controllers/infrastructure/k8sinstallerconfig_controller.go index d2d32f539..abd905b3f 100644 --- a/controllers/infrastructure/k8sinstallerconfig_controller.go +++ b/controllers/infrastructure/k8sinstallerconfig_controller.go @@ -35,6 +35,12 @@ import ( type K8sInstallerConfigReconciler struct { client.Client Scheme *runtime.Scheme + // SkipKernelModuleCleanup disables the overlay/br_netfilter kernel module + // unload step in the generated uninstall script. Real BYO hosts own their + // kernel and must unload these modules; e2e's containerized hosts share + // Docker's kernel, and unloading them there breaks Docker's own bridge + // networking and hangs cluster deletion. + SkipKernelModuleCleanup bool } // k8sInstallerConfigScope defines a scope defined around a K8sInstallerConfig and its ByoMachine @@ -149,7 +155,7 @@ func (r *K8sInstallerConfigReconciler) reconcileNormal(ctx context.Context, scop k8sVersion := scope.Config.GetAnnotations()[infrav1.K8sVersionAnnotation] downloader := installer.NewBundleDownloader(scope.Config.Spec.BundleType, scope.Config.Spec.BundleRepo, "{{.BUNDLE_DOWNLOAD_PATH}}", logger) - installerObj, err := installer.NewInstaller(ctx, scope.ByoMachine.Status.HostInfo.OSImage, scope.ByoMachine.Status.HostInfo.Architecture, k8sVersion, downloader) + installerObj, err := installer.NewInstaller(ctx, scope.ByoMachine.Status.HostInfo.OSImage, scope.ByoMachine.Status.HostInfo.Architecture, k8sVersion, downloader, r.SkipKernelModuleCleanup) if err != nil { logger.Error(err, "failed to create installer instance", "osImage", scope.ByoMachine.Status.HostInfo.OSImage, "architecture", scope.ByoMachine.Status.HostInfo.Architecture, "k8sVersion", k8sVersion) return ctrl.Result{}, err diff --git a/go.mod b/go.mod index 8bd1a23f0..15080b887 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ replace golang.org/x/net => golang.org/x/net v0.17.0 require ( github.com/docker/cli v24.0.7+incompatible github.com/docker/docker v24.0.7+incompatible + github.com/docker/go-units v0.5.0 github.com/go-logr/logr v1.4.3 github.com/jackpal/gateway v1.0.7 github.com/kube-vip/kube-vip v0.5.5 @@ -60,7 +61,6 @@ require ( github.com/docker/go v1.5.1-1.0.20160303222718-d30aec9fd63c // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/docker/go-metrics v0.0.1 // indirect - github.com/docker/go-units v0.5.0 // indirect github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 // indirect github.com/emicklei/go-restful/v3 v3.9.0 // indirect github.com/evanphx/json-patch v5.6.0+incompatible // indirect diff --git a/hack/configure-byoh-host-remote.sh b/hack/configure-byoh-host-remote.sh new file mode 100755 index 000000000..65bf851bc --- /dev/null +++ b/hack/configure-byoh-host-remote.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# +# Runs ON a BYO host — piped in over SSH by configure-byoh-host.sh. Loads the +# kernel modules and sets the sysctls kubeadm requires, persisting both across +# reboot. Idempotent. Uses sudo per-command; assumes passwordless sudo. + +set -Eeuo pipefail + +modules_conf=/etc/modules-load.d/byoh.conf +sysctl_conf=/etc/sysctl.d/99-byoh.conf + +# Load the required modules now and persist them for boot. +printf '%s\n' overlay br_netfilter | sudo tee "$modules_conf" >/dev/null +for mod in overlay br_netfilter; do + sudo modprobe "$mod" +done + +# Persist and apply the kubeadm-required sysctls. +# +# net.netfilter.nf_conntrack_max is global, not per-netns: kube-proxy inside +# a privileged byoh host container can read it but gets "permission denied" +# trying to raise it, so the host must already meet or exceed whatever value +# kube-proxy computes (observed 524288; set well above it for headroom). +sudo tee "$sysctl_conf" >/dev/null <<'SYSCTL' +net.bridge.bridge-nf-call-iptables = 1 +net.bridge.bridge-nf-call-ip6tables = 1 +net.ipv4.ip_forward = 1 +net.netfilter.nf_conntrack_max = 1048576 +SYSCTL +sudo sysctl --system >/dev/null + +# Report final state (|| true: display only; the modprobe above is the assertion). +echo "modules:" +lsmod | grep -E '^overlay|^br_netfilter' || true +echo "sysctls:" +sudo sysctl net.bridge.bridge-nf-call-iptables net.bridge.bridge-nf-call-ip6tables net.ipv4.ip_forward net.netfilter.nf_conntrack_max diff --git a/hack/configure-byoh-host.sh b/hack/configure-byoh-host.sh new file mode 100755 index 000000000..63aad8c60 --- /dev/null +++ b/hack/configure-byoh-host.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# +# configure-byoh-host.sh — enable the kernel/network prerequisites kubeadm needs +# on a BYO host, applied over SSH against a target VM. +# +# When the workload cluster runs on a single machine, the privileged byoh/node +# containers mount /lib/modules:ro and share the host kernel, so the host must +# have the overlay + br_netfilter modules and the bridge-netfilter / ip_forward +# sysctls loaded. The same script preps a standalone BYO host VM. +# +# The commands that run on the target live in configure-byoh-host-remote.sh (a +# sibling file) so they can be linted independently; this script just pipes that +# file to the host over SSH. +# +# Usage: +# hack/configure-byoh-host.sh +# +# SSH user defaults to "ubuntu"; override with BYOH_SSH_USER. +# Assumes passwordless sudo on the target. + +set -Eeuo pipefail +shopt -s nullglob + +SSH_USER=${BYOH_SSH_USER:-ubuntu} + +log() { printf '%s %s\n' "$(date -u +%FT%TZ)" "$*"; } + +usage() { + cat >&2 <<'EOF' +Usage: configure-byoh-host.sh + +Loads overlay + br_netfilter and sets the kubeadm-required sysctls on the +target host over SSH, persisting both across reboot. Idempotent. + +SSH user defaults to "ubuntu" (override with BYOH_SSH_USER). +EOF +} + +main() { + if [[ $# -ne 1 || "$1" == "-h" || "$1" == "--help" ]]; then + usage + exit 1 + fi + local vm_ip="$1" + + local script_dir remote_file + script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) + remote_file="${script_dir}/configure-byoh-host-remote.sh" + [[ -f "$remote_file" ]] || { + log "remote script not found: ${remote_file}" + exit 1 + } + + log "configuring byoh host prerequisites on ${SSH_USER}@${vm_ip}" + ssh "${SSH_USER}@${vm_ip}" 'bash -s' <"$remote_file" + log "done on ${vm_ip}: overlay/br_netfilter loaded, sysctls applied and persisted" +} + +main "$@" diff --git a/installer/installer.go b/installer/installer.go index 88d347a26..242cc2241 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -53,7 +53,7 @@ var archOldNameMap = map[string]string{ } // NewInstaller will return a new installer -func NewInstaller(ctx context.Context, osDist, arch, k8sVersion string, downloader *bundleDownloader) (K8sInstaller, error) { +func NewInstaller(ctx context.Context, osDist, arch, k8sVersion string, downloader *bundleDownloader, skipKernelModuleCleanup bool) (K8sInstaller, error) { bundleArchName := arch // replacing the arch name to old name to match with the bundle name if _, exists := archOldNameMap[arch]; exists { @@ -74,9 +74,9 @@ func NewInstaller(ctx context.Context, osDist, arch, k8sVersion string, download var err error if strings.Contains(osbundle, "Ubuntu_22.04") { - installer, err = algo.NewUbuntu22_04Installer(ctx, arch, addrs) + installer, err = algo.NewUbuntu22_04Installer(ctx, arch, addrs, skipKernelModuleCleanup) } else { - installer, err = algo.NewUbuntu20_04Installer(ctx, arch, addrs) + installer, err = algo.NewUbuntu20_04Installer(ctx, arch, addrs, skipKernelModuleCleanup) } if err != nil { diff --git a/installer/installer_test.go b/installer/installer_test.go index 0eae28988..b634e13d8 100644 --- a/installer/installer_test.go +++ b/installer/installer_test.go @@ -28,7 +28,7 @@ var _ = Describe("Byohost Installer Tests", func() { Context("When installer object is created for valid OS and arch", func() { It("should create the object successfully", func() { - _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader) + _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader, false) Expect(err).ShouldNot(HaveOccurred()) }) }) @@ -36,7 +36,7 @@ var _ = Describe("Byohost Installer Tests", func() { Context("When installer object is created for invalid arch", func() { It("should fail create the object", func() { arch = "arm64" - _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader) + _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader, false) Expect(err).To(MatchError(installer.ErrOsK8sNotSupported)) }) }) @@ -44,7 +44,7 @@ var _ = Describe("Byohost Installer Tests", func() { Context("When installer object is created for invalid OS", func() { It("should fail create the object", func() { os = "rhel" - _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader) + _, err := installer.NewInstaller(context.TODO(), os, arch, k8sversion, downloader, false) Expect(err).To(MatchError(installer.ErrOsK8sNotSupported)) }) }) diff --git a/installer/internal/algo/common_ubuntu.go b/installer/internal/algo/common_ubuntu.go index d1ff378bc..7ef610d91 100644 --- a/installer/internal/algo/common_ubuntu.go +++ b/installer/internal/algo/common_ubuntu.go @@ -39,7 +39,7 @@ func (s *BaseUbuntuInstaller) Uninstall() string { } // NewBaseUbuntuInstaller creates a new base Ubuntu installer -func NewBaseUbuntuInstaller(ctx context.Context, arch, bundleAddrs, containerdConfig string) (*BaseUbuntuInstaller, error) { +func NewBaseUbuntuInstaller(ctx context.Context, arch, bundleAddrs, containerdConfig string, skipKernelModuleCleanup bool) (*BaseUbuntuInstaller, error) { // Validate embedded templates if commonUbuntuInstallTemplate == "" { return nil, fmt.Errorf("install template is empty - template file may be missing") @@ -48,12 +48,13 @@ func NewBaseUbuntuInstaller(ctx context.Context, arch, bundleAddrs, containerdCo return nil, fmt.Errorf("uninstall template is empty - template file may be missing") } - data := map[string]string{ - "BundleAddrs": bundleAddrs, - "Arch": arch, - "ImgpkgVersion": ImgpkgVersion, - "ContainerdConfig": containerdConfig, - "BundleDownloadPath": "/var/lib/byoh/bundles", + data := map[string]interface{}{ + "BundleAddrs": bundleAddrs, + "Arch": arch, + "ImgpkgVersion": ImgpkgVersion, + "ContainerdConfig": containerdConfig, + "BundleDownloadPath": "/var/lib/byoh/bundles", + "SkipKernelModuleCleanup": skipKernelModuleCleanup, } // Parse and validate templates diff --git a/installer/internal/algo/common_ubuntu_test.go b/installer/internal/algo/common_ubuntu_test.go new file mode 100644 index 000000000..2894f3c88 --- /dev/null +++ b/installer/internal/algo/common_ubuntu_test.go @@ -0,0 +1,46 @@ +// Copyright 2022 VMware, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package algo_test + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/installer/internal/algo" +) + +func TestBaseUbuntuInstallerUninstallKernelModuleCleanup(t *testing.T) { + testCases := []struct { + name string + skipKernelModuleCleanup bool + wantModprobeLine bool + }{ + { + name: "kernel modules unloaded when cleanup is not skipped", + skipKernelModuleCleanup: false, + wantModprobeLine: true, + }, + { + name: "kernel modules left alone when cleanup is skipped", + skipKernelModuleCleanup: true, + wantModprobeLine: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + installer, err := algo.NewBaseUbuntuInstaller(context.Background(), "amd64", "test-bundle", "", tc.skipKernelModuleCleanup) + require.NoError(t, err) + + uninstallScript := installer.Uninstall() + + hasModprobeLine := strings.Contains(uninstallScript, "modprobe -rq overlay") + assert.Equal(t, tc.wantModprobeLine, hasModprobeLine) + }) + } +} diff --git a/installer/internal/algo/ubuntu-templates/uninstall.sh.tmpl b/installer/internal/algo/ubuntu-templates/uninstall.sh.tmpl index 82024ba78..3bb66f9c2 100644 --- a/installer/internal/algo/ubuntu-templates/uninstall.sh.tmpl +++ b/installer/internal/algo/ubuntu-templates/uninstall.sh.tmpl @@ -26,7 +26,7 @@ else fi ## remove kernal modules -modprobe -rq overlay || true && modprobe -r br_netfilter || true +{{if not .SkipKernelModuleCleanup}}modprobe -rq overlay || true && modprobe -r br_netfilter || true{{end}} ## restore firewall to its pre-install state if command -v ufw >>/dev/null; then diff --git a/installer/internal/algo/ubuntu20_4k8s.go b/installer/internal/algo/ubuntu20_4k8s.go index 73da2e57d..1a6880523 100644 --- a/installer/internal/algo/ubuntu20_4k8s.go +++ b/installer/internal/algo/ubuntu20_4k8s.go @@ -13,8 +13,8 @@ type Ubuntu20_04Installer struct { } // NewUbuntu20_04Installer will return new Ubuntu20_04Installer instance -func NewUbuntu20_04Installer(ctx context.Context, arch, bundleAddrs string) (*Ubuntu20_04Installer, error) { - base, err := NewBaseUbuntuInstaller(ctx, arch, bundleAddrs, "") // No special containerd config needed for 20.04 +func NewUbuntu20_04Installer(ctx context.Context, arch, bundleAddrs string, skipKernelModuleCleanup bool) (*Ubuntu20_04Installer, error) { + base, err := NewBaseUbuntuInstaller(ctx, arch, bundleAddrs, "", skipKernelModuleCleanup) // No special containerd config needed for 20.04 if err != nil { return nil, err } diff --git a/installer/internal/algo/ubuntu22_04k8s.go b/installer/internal/algo/ubuntu22_04k8s.go index c0ac01168..f1e0befc1 100644 --- a/installer/internal/algo/ubuntu22_04k8s.go +++ b/installer/internal/algo/ubuntu22_04k8s.go @@ -18,8 +18,8 @@ type Ubuntu22_04Installer struct { } // NewUbuntu22_04Installer will return new Ubuntu22_04Installer instance -func NewUbuntu22_04Installer(ctx context.Context, arch, bundleAddrs string) (*Ubuntu22_04Installer, error) { - base, err := NewBaseUbuntuInstaller(ctx, arch, bundleAddrs, systemdCgroupConfig) +func NewUbuntu22_04Installer(ctx context.Context, arch, bundleAddrs string, skipKernelModuleCleanup bool) (*Ubuntu22_04Installer, error) { + base, err := NewBaseUbuntuInstaller(ctx, arch, bundleAddrs, systemdCgroupConfig, skipKernelModuleCleanup) if err != nil { return nil, err } diff --git a/main.go b/main.go index 010949e88..4e7a51070 100644 --- a/main.go +++ b/main.go @@ -140,7 +140,21 @@ func main() { os.Exit(1) } } - if err = (&byohcontrollers.K8sInstallerConfigReconciler{Client: mgr.GetClient(), Scheme: mgr.GetScheme()}).SetupWithManager(mgr); err != nil { + // Set 'BYOH_SKIP_KERNEL_MODULE_CLEANUP=enable' to skip unloading overlay/br_netfilter kernel + // modules during uninstall. Real BYO hosts own their kernel and must unload these modules; + // e2e's containerized hosts share Docker's kernel, so unloading them there breaks Docker's + // own bridge networking and hangs cluster deletion. + // + // Uses 'enable'/'disable' (matching MANUAL_CSR_APPROVAL above), not 'true'/'false': kustomize + // re-serializes manager.yaml and drops the quotes around "${VAR:=default}", so an unquoted + // true/false would parse as a YAML bool instead of a string, breaking clusterctl's conversion + // of the rendered Deployment's env value back into a typed corev1.EnvVar. + skipKernelModuleCleanup := os.Getenv("BYOH_SKIP_KERNEL_MODULE_CLEANUP") == "enable" + if err = (&byohcontrollers.K8sInstallerConfigReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + SkipKernelModuleCleanup: skipKernelModuleCleanup, + }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "K8sInstallerConfig") os.Exit(1) } diff --git a/test/e2e/byohost_reuse_test.go b/test/e2e/byohost_reuse_test.go index d9e8eb826..a19bf86ab 100644 --- a/test/e2e/byohost_reuse_test.go +++ b/test/e2e/byohost_reuse_test.go @@ -93,12 +93,12 @@ var _ = Describe("When BYO Host rejoins the capacity pool", func() { defer output.Close() byohostContainerIDs = append(byohostContainerIDs, byohostContainerID) f := WriteDockerLog(output, agentLogFile1) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("Error closing file %s: %v", agentLogFile1, deferredErr) } - }() + }(f) runner.ByoHostName = byoHostName2 runner.BootstrapKubeconfigData = generateBootstrapKubeconfig(runner.Context, bootstrapClusterProxy, clusterConName) @@ -111,12 +111,12 @@ var _ = Describe("When BYO Host rejoins the capacity pool", func() { // read the log of host agent container in backend, and write it f = WriteDockerLog(output, agentLogFile2) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("Error closing file %s: %v", agentLogFile2, deferredErr) } - }() + }(f) By("Creating a cluster") diff --git a/test/e2e/config/provider.yaml b/test/e2e/config/provider.yaml index 0c14be077..ed9dbb8d1 100644 --- a/test/e2e/config/provider.yaml +++ b/test/e2e/config/provider.yaml @@ -86,7 +86,7 @@ providers: variables: # default variables for the e2e test; those values could be overridden via env variables, thus # allowing the same e2e config file to be re-used in different prow jobs e.g. each one with a K8s version permutation - KUBERNETES_VERSION: "v1.26.6" + KUBERNETES_VERSION: "v1.31.0" ETCD_VERSION_UPGRADE_TO: "3.5.6-0" COREDNS_VERSION_UPGRADE_TO: "1.9.3" KUBERNETES_VERSION_UPGRADE_TO: "v1.22.0" @@ -103,9 +103,10 @@ variables: NODE_DRAIN_TIMEOUT: "60s" # NOTE: INIT_WITH_BINARY is used only by the clusterctl upgrade test to initialize the management cluster to be upgraded INIT_WITH_BINARY: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v1.4.4/clusterctl-{OS}-{ARCH}" - BUNDLE_LOOKUP_TAG: "v1.26.6" + BUNDLE_LOOKUP_TAG: "v1.31.0" CONTROL_PLANE_ENDPOINT_IP: "" MANUAL_CSR_APPROVAL: "disable" + BYOH_SKIP_KERNEL_MODULE_CLEANUP: "enable" intervals: default/wait-controllers: ["3m", "10s"] diff --git a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template-topology.yaml b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template-topology.yaml index 528e613fa..59da11c02 100644 --- a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template-topology.yaml +++ b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template-topology.yaml @@ -24,7 +24,7 @@ spec: replicas: ${CONTROL_PLANE_MACHINE_COUNT} variables: - name: bundleLookupBaseRegistry - value: "projects.registry.vmware.com/cluster_api_provider_bringyourownhost" + value: "quay.io/platform9" - name: controlPlaneIpAddr value: ${CONTROL_PLANE_ENDPOINT_IP} - name: kubeVipPodManifest @@ -75,7 +75,7 @@ spec: ip: 127.0.0.1 volumes: - hostPath: - path: /etc/kubernetes/admin.conf + path: /etc/kubernetes/super-admin.conf type: FileOrCreate name: kubeconfig status: {} diff --git a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template.yaml b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template.yaml index c5ce19a32..94fb1b35d 100644 --- a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template.yaml +++ b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-template.yaml @@ -27,8 +27,13 @@ spec: spec: joinConfiguration: nodeRegistration: + ignorePreflightErrors: + - Swap + - SystemVerification + - DirAvailable--etc-kubernetes-manifests + - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% --- apiVersion: cluster.x-k8s.io/v1beta1 @@ -148,7 +153,7 @@ spec: ip: 127.0.0.1 volumes: - hostPath: - path: /etc/kubernetes/admin.conf + path: /etc/kubernetes/super-admin.conf type: FileOrCreate name: kubeconfig status: {} @@ -159,21 +164,32 @@ spec: criSocket: /var/run/containerd/containerd.sock ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% joinConfiguration: nodeRegistration: criSocket: /var/run/containerd/containerd.sock ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% + preKubeadmCommands: + - | + cat <<'EOF' >> /run/kubeadm/kubeadm.yaml + --- + apiVersion: kubeproxy.config.k8s.io/v1alpha1 + kind: KubeProxyConfiguration + conntrack: + maxPerCore: 0 + EOF machineTemplate: infrastructureRef: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 @@ -188,7 +204,7 @@ kind: ByoCluster metadata: name: ${CLUSTER_NAME} spec: - bundleLookupBaseRegistry: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleLookupBaseRegistry: quay.io/platform9 controlPlaneEndpoint: host: ${CONTROL_PLANE_ENDPOINT_IP} port: 6443 @@ -226,7 +242,7 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s --- apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 @@ -236,5 +252,5 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s diff --git a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-with-kcp.yaml b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-with-kcp.yaml index f5238de8f..1bcd1b43e 100644 --- a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-with-kcp.yaml +++ b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/cluster-with-kcp.yaml @@ -27,7 +27,7 @@ kind: ByoCluster metadata: name: ${CLUSTER_NAME} spec: - bundleLookupBaseRegistry: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleLookupBaseRegistry: quay.io/platform9 controlPlaneEndpoint: host: ${CONTROL_PLANE_ENDPOINT_IP} port: 6443 @@ -60,6 +60,22 @@ spec: name: "${CLUSTER_NAME}-control-plane" namespace: "${NAMESPACE}" kubeadmConfigSpec: + # kube-proxy's conntrack tuning tries to raise net.netfilter.nf_conntrack_max, + # a global (non-namespaced) sysctl a privileged container can read but never + # write -- crash-looping with "permission denied" regardless of the host's + # own value. CAPI's KubeadmControlPlane has no field for KubeProxyConfiguration + # (kubernetes-sigs/cluster-api#4512), so append one to kubeadm's own config + # file before it runs, disabling the tuning kubeadm would otherwise bake into + # the kube-proxy ConfigMap it creates. + preKubeadmCommands: + - | + cat <<'EOF' >> /run/kubeadm/kubeadm.yaml + --- + apiVersion: kubeproxy.config.k8s.io/v1alpha1 + kind: KubeProxyConfiguration + conntrack: + maxPerCore: 0 + EOF clusterConfiguration: controllerManager: extraArgs: {enable-hostpath-provisioner: 'true'} @@ -113,7 +129,7 @@ spec: ip: 127.0.0.1 volumes: - hostPath: - path: /etc/kubernetes/admin.conf + path: /etc/kubernetes/super-admin.conf type: FileOrCreate name: kubeconfig status: {} @@ -123,21 +139,23 @@ spec: nodeRegistration: ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf criSocket: /var/run/containerd/containerd.sock kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% joinConfiguration: nodeRegistration: ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf criSocket: /var/run/containerd/containerd.sock kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% version: ${KUBERNETES_VERSION} --- @@ -148,5 +166,5 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s diff --git a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/clusterclass-quickstart.yaml b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/clusterclass-quickstart.yaml index ad5563eb8..dcf402688 100644 --- a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/clusterclass-quickstart.yaml +++ b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/clusterclass-quickstart.yaml @@ -39,7 +39,7 @@ spec: schema: openAPIV3Schema: type: string - default: "https://projects.registry.vmware.com/cluster_api_provider_bringyourownhost" + default: "https://quay.io/platform9" - name: controlPlaneIpAddr required: true schema: @@ -101,6 +101,22 @@ spec: template: spec: kubeadmConfigSpec: + # kube-proxy's conntrack tuning tries to raise net.netfilter.nf_conntrack_max, + # a global (non-namespaced) sysctl a privileged container can read but never + # write -- crash-looping with "permission denied" regardless of the host's + # own value. CAPI's KubeadmControlPlane has no field for KubeProxyConfiguration + # (kubernetes-sigs/cluster-api#4512), so append one to kubeadm's own config + # file before it runs, disabling the tuning kubeadm would otherwise bake into + # the kube-proxy ConfigMap it creates. + preKubeadmCommands: + - | + cat <<'EOF' >> /run/kubeadm/kubeadm.yaml + --- + apiVersion: kubeproxy.config.k8s.io/v1alpha1 + kind: KubeProxyConfiguration + conntrack: + maxPerCore: 0 + EOF clusterConfiguration: apiServer: certSANs: @@ -119,20 +135,22 @@ spec: criSocket: /var/run/containerd/containerd.sock ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% joinConfiguration: nodeRegistration: criSocket: /var/run/containerd/containerd.sock ignorePreflightErrors: - Swap + - SystemVerification - DirAvailable--etc-kubernetes-manifests - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% --- apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 @@ -155,7 +173,7 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s --- apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 @@ -175,8 +193,13 @@ spec: spec: joinConfiguration: nodeRegistration: + ignorePreflightErrors: + - Swap + - SystemVerification + - DirAvailable--etc-kubernetes-manifests + - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% --- apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 @@ -199,7 +222,7 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s --- apiVersion: v1 diff --git a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/md.yaml b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/md.yaml index 127bd05b8..40a6fb890 100644 --- a/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/md.yaml +++ b/test/e2e/data/infrastructure-provider-byoh/v1beta1/templates/e2e/md.yaml @@ -22,8 +22,13 @@ spec: spec: joinConfiguration: nodeRegistration: + ignorePreflightErrors: + - Swap + - SystemVerification + - DirAvailable--etc-kubernetes-manifests + - FileAvailable--etc-kubernetes-kubelet.conf kubeletExtraArgs: - cgroup-driver: cgroupfs + cgroup-driver: systemd eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0% --- apiVersion: cluster.x-k8s.io/v1beta1 @@ -59,5 +64,5 @@ metadata: spec: template: spec: - bundleRepo: projects.registry.vmware.com/cluster_api_provider_bringyourownhost + bundleRepo: quay.io/platform9 bundleType: k8s diff --git a/test/e2e/docker_helper.go b/test/e2e/docker_helper.go index 7b8487902..0a0b64664 100644 --- a/test/e2e/docker_helper.go +++ b/test/e2e/docker_helper.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/system" + "github.com/docker/go-units" . "github.com/onsi/gomega" //nolint: stylecheck "github.com/pkg/errors" "k8s.io/client-go/tools/clientcmd" @@ -171,11 +172,43 @@ func (r *ByoHostRunner) createDockerContainer() (container.CreateResponse, error Tmpfs: tmpfs, NetworkMode: container.NetworkMode(r.NetworkInterface), Binds: []string{"/var", "/lib/modules:/lib/modules:ro"}, + // kube-proxy's iptables/netlink usage exhausts Docker's default 1024 + // nofile limit almost immediately; match kind's own node ulimit. + Resources: container.Resources{ + Ulimits: []*units.Ulimit{ + { + Name: "nofile", + Soft: 1048576, + Hard: 1048576, + }, + }, + }, }, &network.NetworkingConfig{EndpointsConfig: map[string]*network.EndpointSettings{r.NetworkInterface: {}}}, nil, r.ByoHostName) } +// raiseInotifyInstanceLimit bumps fs.inotify.max_user_instances inside the container. +// +// Docker's --sysctl rejects fs.inotify.* at container-create time (not on its +// namespaced-sysctl allowlist), so it has to be applied with a live write after +// start. Running several byohost containers alongside the management kind +// cluster on one devbox exhausts the host default of 128 instances, which +// makes containerd's CRI plugin fail to load ("too many open files") and +// leaves kubeadm join stuck retrying against a dead CRI socket. +func (r *ByoHostRunner) raiseInotifyInstanceLimit(containerID string) error { + execCommand, err := r.DockerClient.ContainerExecCreate(r.Context, containerID, types.ExecConfig{ + AttachStdout: true, + AttachStderr: true, + Cmd: []string{"sysctl", "-w", "fs.inotify.max_user_instances=8192"}, + }) + if err != nil { + return errors.Wrapf(err, "create exec for raising inotify instance limit in container %q", containerID) + } + return errors.Wrapf(r.DockerClient.ContainerExecStart(r.Context, execCommand.ID, types.ExecStartCheck{}), + "raise inotify instance limit in container %q", containerID) +} + func (r *ByoHostRunner) copyKubeconfig(config cpConfig, listopt types.ContainerListOptions) error { var kubeconfig []byte if r.NetworkInterface == "host" { @@ -261,6 +294,7 @@ func (r *ByoHostRunner) SetupByoDockerHost() (*container.CreateResponse, error) Expect(err).NotTo(HaveOccurred()) Expect(r.DockerClient.ContainerStart(r.Context, byohost.ID, types.ContainerStartOptions{})).NotTo(HaveOccurred()) + Expect(r.raiseInotifyInstanceLimit(byohost.ID)).To(Succeed()) config := cpConfig{ sourcePath: r.PathToHostAgentBinary, diff --git a/test/e2e/e2e_clusterclass_test.go b/test/e2e/e2e_clusterclass_test.go index 6129a4aee..5751e0914 100644 --- a/test/e2e/e2e_clusterclass_test.go +++ b/test/e2e/e2e_clusterclass_test.go @@ -89,12 +89,12 @@ var _ = Describe("When BYOH joins existing cluster [Cluster-Class]", func() { defer output.Close() byohostContainerIDs = append(byohostContainerIDs, byohostContainerID) f := WriteDockerLog(output, agentLogFile1) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile1, deferredErr) } - }() + }(f) runner.ByoHostName = byoHostName2 runner.BootstrapKubeconfigData = generateBootstrapKubeconfig(runner.Context, bootstrapClusterProxy, clusterConName) @@ -107,12 +107,12 @@ var _ = Describe("When BYOH joins existing cluster [Cluster-Class]", func() { // read the log of host agent container in backend, and write it f = WriteDockerLog(output, agentLogFile2) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile2, deferredErr) } - }() + }(f) setControlPlaneIP(context.Background(), dockerClient) clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ diff --git a/test/e2e/e2e_installer_test.go b/test/e2e/e2e_installer_test.go index 68a108355..923759b29 100644 --- a/test/e2e/e2e_installer_test.go +++ b/test/e2e/e2e_installer_test.go @@ -90,12 +90,12 @@ var _ = Describe("When BYOH joins existing cluster [Installer]", func() { defer output.Close() byohostContainerIDs = append(byohostContainerIDs, byohostContainerID) f := WriteDockerLog(output, agentLogFile1) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile1, deferredErr) } - }() + }(f) runner.ByoHostName = byoHostName2 runner.BootstrapKubeconfigData = generateBootstrapKubeconfig(runner.Context, bootstrapClusterProxy, clusterConName) @@ -108,12 +108,12 @@ var _ = Describe("When BYOH joins existing cluster [Installer]", func() { // read the log of host agent container in backend, and write it f = WriteDockerLog(output, agentLogFile2) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile2, deferredErr) } - }() + }(f) setControlPlaneIP(context.Background(), dockerClient) clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index cc5273276..cc2c11fad 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -89,12 +89,12 @@ var _ = Describe("When BYOH joins existing cluster [PR-Blocking]", func() { defer output.Close() byohostContainerIDs = append(byohostContainerIDs, byohostContainerID) f := WriteDockerLog(output, agentLogFile1) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile1, deferredErr) } - }() + }(f) runner.ByoHostName = byoHostName2 runner.BootstrapKubeconfigData = generateBootstrapKubeconfig(runner.Context, bootstrapClusterProxy, clusterConName) @@ -107,12 +107,12 @@ var _ = Describe("When BYOH joins existing cluster [PR-Blocking]", func() { // read the log of host agent container in backend, and write it f = WriteDockerLog(output, agentLogFile2) - defer func() { + defer func(f *os.File) { deferredErr := f.Close() if deferredErr != nil { Showf("error closing file %s: %v", agentLogFile2, deferredErr) } - }() + }(f) setControlPlaneIP(context.Background(), dockerClient) clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{