diff --git a/test/e2e/agent_upgrade_fixture_helper_test.go b/test/e2e/agent_upgrade_fixture_helper_test.go new file mode 100644 index 000000000..c026294c6 --- /dev/null +++ b/test/e2e/agent_upgrade_fixture_helper_test.go @@ -0,0 +1,165 @@ +// Copyright 2026 Platform9, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// nolint: testpackage +package e2e + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + goruntime "runtime" + "strings" + + "github.com/docker/docker/client" + "github.com/onsi/gomega/gexec" +) + +const ( + fixtureAgentDebPackageName = "pf9-byohost-agent-fixture" + fixtureAgentDebFileName = "pf9-byohost-agent.deb" +) + +// buildFixtureAgentBinary builds a real BYOH host agent binary with version.GitVersion baked in +// as gitVersion -- for use as a self-upgrade source/target in the agent-upgrade e2e specs. Reuses +// the same build flags e2e_suite_test.go's SynchronizedBeforeSuite uses for the suite's own main +// agent binary, just with a different, test-chosen version string, so the installed fixture is a +// fully working agent, not a stand-in. +func buildFixtureAgentBinary(gitVersion string) (string, error) { + return gexec.BuildWithEnvironment( + "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent", + []string{"CGO_ENABLED=0", "GOOS=linux", "GOARCH=" + goruntime.GOARCH}, + "-ldflags", "-X github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/agent/version.GitVersion="+gitVersion, + ) +} + +// buildFixtureAgentDeb packages binaryPath as a minimal .deb that installs it at +// systemdAgentBinaryPath -- the same path the real production package and this suite's systemd +// harness both use. Returns the directory containing the built .deb (imgpkg push -f wants a +// directory, not a single file). +// +// Deliberately hand-built with dpkg-deb rather than fpm: fpm/ruby are only installed in this +// repo's CI for the real build-host-agent-deb pipeline, gated on workflow_dispatch (see +// .github/workflows/e2e.yml and the CLAUDE.md gotcha about that target's skip/fail split) -- not +// present for every e2e run. dpkg-deb ships with any Debian-family base (confirmed present in +// golang:1.26.4, the base this suite's own linux-test-runner image builds from), so this needs no +// new dependency at all. +// +func buildFixtureAgentDeb(ctx context.Context, binaryPath, gitVersion string) (debDir string, err error) { + stageDir, err := os.MkdirTemp("", "agent-fixture-deb-*") + if err != nil { + return "", err + } + defer os.RemoveAll(stageDir) //nolint:errcheck // best-effort temp dir cleanup + + debianDir := filepath.Join(stageDir, "DEBIAN") + if mkdirErr := os.MkdirAll(debianDir, 0755); mkdirErr != nil { + return "", mkdirErr + } + // systemdAgentBinaryPath is "/binary/pf9-byoh-hostagent" -- dpkg-deb packs stageDir's tree + // onto the target filesystem's root, so the payload lives at stageDir+that same path. + payloadDir := filepath.Join(stageDir, filepath.Dir(systemdAgentBinaryPath)) + if mkdirErr := os.MkdirAll(payloadDir, 0755); mkdirErr != nil { + return "", mkdirErr + } + + binaryData, err := os.ReadFile(binaryPath) //nolint:gosec // binaryPath is a suite-built local binary path, not user input + if err != nil { + return "", err + } + if writeErr := os.WriteFile(filepath.Join(payloadDir, filepath.Base(systemdAgentBinaryPath)), binaryData, 0755); writeErr != nil { //nolint:gosec // the payload must be executable + return "", writeErr + } + + // GOARCH happens to already match Debian's architecture naming for both values this repo's + // Makefile packages for (amd64/arm64) -- see Makefile's PACKAGE_GOARCH. + control := fmt.Sprintf("Package: %s\nVersion: %s\nArchitecture: %s\nMaintainer: byoh-e2e\nDescription: agent-upgrade e2e fixture package\n", + fixtureAgentDebPackageName, strings.TrimPrefix(gitVersion, "v"), goruntime.GOARCH) + if writeErr := os.WriteFile(filepath.Join(debianDir, "control"), []byte(control), 0644); writeErr != nil { + return "", writeErr + } + + postinst := "#!/bin/sh\nset -e\nchmod +x " + systemdAgentBinaryPath + "\n" + if writeErr := os.WriteFile(filepath.Join(debianDir, "postinst"), []byte(postinst), 0755); writeErr != nil { //nolint:gosec // dpkg requires postinst to be executable + return "", writeErr + } + + outDir, err := os.MkdirTemp("", "agent-fixture-bundle-*") + if err != nil { + return "", err + } + outPath := filepath.Join(outDir, fixtureAgentDebFileName) + buildCmd := exec.CommandContext(ctx, "dpkg-deb", "--build", "--root-owner-group", stageDir, outPath) // #nosec G204 -- fixed args, stageDir/outPath are our own temp dirs + if output, buildErr := buildCmd.CombinedOutput(); buildErr != nil { + return "", fmt.Errorf("dpkg-deb --build failed: %w\n%s", buildErr, output) + } + + return outDir, nil +} + +// pushFixtureAgentBundle imgpkg-pushes the .deb in debDir to the shared local bundle registry +// (see e2e_agent_bundle_registry.go) under tag, returning the address containers on dockerNetwork +// can pull it from. Starts the registry itself if it isn't already running -- a typical local +// `make test-e2e-linux-vm` run never builds the real agent bundle +// (build/pf9-byohost/debsrc/pf9-byohost-agent.deb), so ensureLocalAgentBundleRegistry may never +// have started it. +// +// Returns the registry's container IP on dockerNetwork, not its container-name alias: imgpkg +// (via go-containerregistry's name.Registry.Scheme) only skips TLS automatically for RFC1918/ +// loopback addresses, not arbitrary hostnames -- pointing agent/cloudinit/cmd_runner.go's +// unconfigurable `imgpkg pull` at the real private IP gets that for free, with no wrapper script +// or registry-insecure flag needed anywhere. +func pushFixtureAgentBundle(ctx context.Context, dockerClient *client.Client, dockerNetwork, debDir, tag string) (string, error) { + repoRoot, err := resolveRepoRoot(ctx) + if err != nil { + return "", err + } + if startErr := startLocalBundleRegistry(ctx, dockerClient, dockerNetwork); startErr != nil { + return "", startErr + } + imgpkgPath, err := downloadImgpkg(ctx, repoRoot) + if err != nil { + return "", err + } + + hostAddr := "localhost:" + localBundleRegistryHostPort + pushCmd := exec.CommandContext(ctx, imgpkgPath, "push", "-f", debDir, "-i", hostAddr+"/"+tag) // #nosec G204 -- fixed args, no user input + if output, pushErr := pushCmd.CombinedOutput(); pushErr != nil { + return "", fmt.Errorf("failed to push fixture agent bundle: %w\n%s", pushErr, output) + } + + registryIP, err := localBundleRegistryIP(ctx, dockerClient, dockerNetwork) + if err != nil { + return "", err + } + return registryIP + ":5000/" + tag, nil +} + +// localBundleRegistryIP returns the shared local bundle registry's own IP on dockerNetwork. +func localBundleRegistryIP(ctx context.Context, dockerClient *client.Client, dockerNetwork string) (string, error) { + inspect, err := dockerClient.ContainerInspect(ctx, localBundleRegistryContainerName) + if err != nil { + return "", err + } + endpoint, ok := inspect.NetworkSettings.Networks[dockerNetwork] + if !ok || endpoint.IPAddress == "" { + return "", fmt.Errorf("local bundle registry has no IP on network %q", dockerNetwork) + } + return endpoint.IPAddress, nil +} + +// installImgpkgOnHost puts imgpkg on containerID's PATH -- production hosts get it via the +// k8s-installer's self-install fallback, which this suite's hosts skip (never join a cluster). +func installImgpkgOnHost(ctx context.Context, dockerClient *client.Client, containerID, repoRoot string) error { + imgpkgPath, err := downloadImgpkg(ctx, repoRoot) + if err != nil { + return err + } + return copyToContainer(ctx, dockerClient, cpConfig{ + sourcePath: imgpkgPath, + destPath: "/usr/local/bin/imgpkg", + container: containerID, + }) +} diff --git a/test/e2e/agent_upgrade_rollout_test.go b/test/e2e/agent_upgrade_rollout_test.go new file mode 100644 index 000000000..f1be89f8c --- /dev/null +++ b/test/e2e/agent_upgrade_rollout_test.go @@ -0,0 +1,167 @@ +// Copyright 2026 Platform9, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// nolint: testpackage +package e2e + +import ( + "context" + "fmt" + "time" + + "github.com/docker/docker/client" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + infrastructurev1beta1 "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/apis/infrastructure/v1beta1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/cluster-api/test/framework/clusterctl" + "sigs.k8s.io/cluster-api/util" +) + +// agentUpgradeRolloutFleetSize is the ADR's own §5.3 example size for this scenario. +const agentUpgradeRolloutFleetSize = 4 + +// agentUpgradeRolloutTimeout/Poll bound the full rollout: MaxUnavailable=1 mostly serializes the +// 4 hosts, and each one's cycle is imgpkg pull (local registry, fast) + dpkg -i + os.Exit(0) + +// systemd relaunch (RestartSec=5s) + the rollout controller's own 15s reconcile tick -- a few +// minutes of headroom for 4 hosts. +const ( + agentUpgradeRolloutTimeout = 5 * time.Minute + agentUpgradeRolloutPoll = 5 * time.Second +) + +var _ = Describe("When an agent upgrade rollout stages across a fleet [AgentUpgrade]", func() { + + var ( + ctx context.Context + specName = "agent-upgrade-rollout" + namespace *corev1.Namespace + cancelWatches context.CancelFunc + clusterResources *clusterctl.ApplyClusterTemplateAndWaitResult + hosts []byoHostHandle + ) + + BeforeEach(func() { + ctx, namespace, cancelWatches, clusterResources = commonSpecSetup(specName) + }) + + It("Should stage an agent upgrade across the fleet respecting maxUnavailable", func() { + const ( + oldVersion = "v9.9.8" + newVersion = "v9.9.9" + ) + + dc, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) + Expect(err).NotTo(HaveOccurred()) + setDockerClient(dc) + + By("Building the old and new agent binaries") + oldBinary, err := buildFixtureAgentBinary(oldVersion) + Expect(err).NotTo(HaveOccurred()) + newBinary, err := buildFixtureAgentBinary(newVersion) + Expect(err).NotTo(HaveOccurred()) + + By("Packaging the new version as a .deb fixture and pushing it to the local registry") + newDebDir, err := buildFixtureAgentDeb(ctx, newBinary, newVersion) + Expect(err).NotTo(HaveOccurred()) + packageURL, err := pushFixtureAgentBundle(ctx, dockerClient, dockerNetworkInterfaceKind, newDebDir, + fmt.Sprintf("agent-fixture-%s:e2e", util.RandomString(6))) + Expect(err).NotTo(HaveOccurred()) + + By("Spinning up a 4-host fleet running the old version under systemd") + hosts, err = spinUpByoHostsWithSystemdAgent(ctx, dockerClient, namespace.Name, agentUpgradeRolloutFleetSize, oldBinary) + Expect(err).NotTo(HaveOccurred()) + for _, host := range hosts { + defer host.StopLog() + } + + By("Installing imgpkg on each host (stands in for the k8s-installer's self-install fallback)") + repoRoot, err := resolveRepoRoot(ctx) + Expect(err).NotTo(HaveOccurred()) + for _, host := range hosts { + Expect(installImgpkgOnHost(ctx, dockerClient, host.ContainerID, repoRoot)).To(Succeed()) + } + + By("Waiting for all 4 hosts to connect") + for _, host := range hosts { + AssertByoHostConditionsTrue(ctx, bootstrapClusterProxy, namespace.Name, host.Name, specName, infrastructurev1beta1.AgentConnected) + } + + // Captured before the rollout starts -- proof, once the rollout completes, that each + // host's agent process actually restarted (a new PID) inside the same container (an + // unchanged container ID), not that the container itself was recreated. + originalPIDs := make(map[string]int, len(hosts)) + for _, host := range hosts { + originalPIDs[host.Name] = mainPID(ctx, dockerClient, host.ContainerID) + Expect(originalPIDs[host.Name]).To(BeNumerically(">", 0)) + } + + By("Creating a ByoHostAgentUpgrade targeting all 4 hosts with MaxUnavailable=1") + maxUnavailable := intstr.FromInt(1) + upgrade := &infrastructurev1beta1.ByoHostAgentUpgrade{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-%s", specName, util.RandomString(6)), + Namespace: namespace.Name, + }, + Spec: infrastructurev1beta1.ByoHostAgentUpgradeSpec{ + Selector: metav1.LabelSelector{}, + TargetVersion: newVersion, + PackageURL: packageURL, + MaxUnavailable: &maxUnavailable, + }, + } + Expect(bootstrapClusterProxy.GetClient().Create(ctx, upgrade)).To(Succeed()) + upgradeKey := k8stypes.NamespacedName{Name: upgrade.Name, Namespace: upgrade.Namespace} + + By("Polling the rollout to completion, asserting it never exceeds MaxUnavailable or fails") + Eventually(func(g Gomega) infrastructurev1beta1.ByoHostAgentUpgradePhase { + got := &infrastructurev1beta1.ByoHostAgentUpgrade{} + g.Expect(bootstrapClusterProxy.GetClient().Get(ctx, upgradeKey, got)).To(Succeed()) + g.Expect(got.Status.UnavailableCount).To(BeNumerically("<=", 1), + "UnavailableCount exceeded MaxUnavailable mid-rollout") + g.Expect(got.Status.Phase).NotTo(Equal(infrastructurev1beta1.ByoHostAgentUpgradePhaseFailed), + "rollout failed: FailedHosts=%v", got.Status.FailedHosts) + return got.Status.Phase + }, agentUpgradeRolloutTimeout, agentUpgradeRolloutPoll).Should(Equal(infrastructurev1beta1.ByoHostAgentUpgradePhaseCompleted)) + + By("Asserting full convergence") + converged := &infrastructurev1beta1.ByoHostAgentUpgrade{} + Expect(bootstrapClusterProxy.GetClient().Get(ctx, upgradeKey, converged)).To(Succeed()) + Expect(converged.Status.Upgraded).To(Equal(int32(agentUpgradeRolloutFleetSize))) + Expect(converged.Status.FailedHosts).To(BeEmpty()) + + By("Asserting each host's agent process restarted inside its unchanged container") + for _, host := range hosts { + h := &infrastructurev1beta1.ByoHost{} + Expect(bootstrapClusterProxy.GetClient().Get(ctx, k8stypes.NamespacedName{Name: host.Name, Namespace: namespace.Name}, h)).To(Succeed()) + Expect(h.Status.AgentVersion).To(Equal(newVersion), "host %s never reported the new version", host.Name) + + newPID := mainPID(ctx, dockerClient, host.ContainerID) + Expect(newPID).To(SatisfyAll(BeNumerically(">", 0), Not(BeNumerically("==", originalPIDs[host.Name]))), + "expected host %s's agent process to have restarted with a new PID", host.Name) + + inspect, err := dockerClient.ContainerInspect(ctx, host.ContainerID) + Expect(err).NotTo(HaveOccurred(), "expected host %s's original container to still exist", host.Name) + Expect(inspect.State.Running).To(BeTrue(), "expected host %s's container to still be running", host.Name) + } + }) + + JustAfterEach(func() { + if CurrentGinkgoTestDescription().Failed { + logFiles := make([]string, 0, len(hosts)) + for _, host := range hosts { + logFiles = append(logFiles, host.LogFilePath) + } + ShowInfo(logFiles) + } + }) + + AfterEach(func() { + dumpSpecResourcesAndCleanup(ctx, specName, bootstrapClusterProxy, artifactFolder, namespace, cancelWatches, clusterResources.Cluster, e2eConfig.GetIntervals, skipCleanup) + + teardownByoHosts(ctx, getDockerClient(), hosts) + }) +}) diff --git a/test/e2e/agent_upgrade_systemd_helper_test.go b/test/e2e/agent_upgrade_systemd_helper_test.go new file mode 100644 index 000000000..1d3e6d8f4 --- /dev/null +++ b/test/e2e/agent_upgrade_systemd_helper_test.go @@ -0,0 +1,209 @@ +// Copyright 2026 Platform9, Inc. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// nolint: testpackage +package e2e + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" + "github.com/docker/docker/pkg/stdcopy" + "github.com/pkg/errors" +) + +// The e2e harness's default spinUpByoHosts path (byohost_spinup_helper_test.go) starts the agent +// as a bare, attached `docker exec` child process -- fine for every existing spec, but the +// self-upgrade mechanism's os.Exit(0)-then-relaunch (docs/proposals/agent-self-upgrade-adr.md +// §2.2 step 5) depends on the real pf9-byohostagent.service systemd unit's Restart=always, which +// that path never installs. spinUpByoHostsWithSystemdAgent below is the alternative host +// provisioning path the agent-upgrade specs need instead. +const ( + // systemdAgentBinaryPath matches ConditionPathExists/ExecStart in + // service/pf9-byohostagent.service. + systemdAgentBinaryPath = "/binary/pf9-byoh-hostagent" + // systemdAgentServiceUnitSrcPath is repo-root-relative -- see installSystemdAgentUnit's + // resolveRepoRoot call for how that's resolved. + systemdAgentServiceUnitSrcPath = "service/pf9-byohostagent.service" + // systemdAgentServiceUnitName matches the name the real packaging pipeline installs this same + // unit file under (Makefile's COMMON_SRC_ROOT rule renames it on copy). + systemdAgentServiceUnitName = "pf9-byohost-agent.service" + systemdAgentEnvFileDir = "/etc/pf9-byohost-agent.service.d" + // systemdAgentEnvFilePath matches EnvironmentFile= in service/pf9-byohostagent.service. + systemdAgentEnvFilePath = systemdAgentEnvFileDir + "/pf9-byohost-agent.conf" + systemdAgentLogDir = "/var/log/pf9/byoh" + // systemdAgentLogFile matches the ExecStart redirect in service/pf9-byohostagent.service. + systemdAgentLogFile = systemdAgentLogDir + "/byoh-agent.log" +) + +// spinUpByoHostsWithSystemdAgent creates count BYO hosts whose agent runs under systemd (see the +// package comment above for why). agentBinaryPath is normally pathToHostAgentBinary, but the +// agent-upgrade rollout scenario needs hosts to start on a known, test-controlled version rather +// than whatever this suite's own build produces, so it's a parameter rather than hardcoded. On +// error it returns the handles created so far alongside the error. +func spinUpByoHostsWithSystemdAgent(ctx context.Context, dockerClient *client.Client, namespace string, count int, agentBinaryPath string) ([]byoHostHandle, error) { + return spinUpByoHostsCommon(ctx, dockerClient, namespace, count, agentBinaryPath, + func(runner *ByoHostRunner, byohost *container.CreateResponse) (func(), string, error) { + if err := installSystemdAgentUnit(ctx, dockerClient, byohost.ID, namespace, agentBinaryPath); err != nil { + return nil, "", err + } + logFilePath := fmt.Sprintf("/tmp/host-agent-%s.log", runner.ByoHostName) + return copySystemdAgentLog(ctx, dockerClient, byohost.ID, logFilePath), logFilePath, nil + }) +} + +// installSystemdAgentUnit copies the real agent binary and systemd unit into containerID at the +// paths the unit expects, writes its EnvironmentFile, and enables+starts it. +func installSystemdAgentUnit(ctx context.Context, dockerClient *client.Client, containerID, namespace, agentBinaryPath string) error { + if err := copyToContainer(ctx, dockerClient, cpConfig{ + sourcePath: agentBinaryPath, + destPath: systemdAgentBinaryPath, + container: containerID, + }); err != nil { + return errors.Wrap(err, "copy agent binary to systemd ExecStart path") + } + + // The ginkgo CLI runs the compiled test binary with its cwd set to the package directory + // (test/e2e), not the repo root -- resolveRepoRoot (already used by + // e2e_agent_bundle_registry.go for the same reason) finds the real root regardless. + repoRoot, err := resolveRepoRoot(ctx) + if err != nil { + return errors.Wrap(err, "resolve repo root for systemd unit source path") + } + if copyErr := copyToContainer(ctx, dockerClient, cpConfig{ + sourcePath: filepath.Join(repoRoot, systemdAgentServiceUnitSrcPath), + destPath: "/etc/systemd/system/" + systemdAgentServiceUnitName, + container: containerID, + }); copyErr != nil { + return errors.Wrap(copyErr, "copy pf9-byohost-agent systemd unit") + } + + if mkdirErr := runContainerCommand(ctx, dockerClient, containerID, + "mkdir", "-p", systemdAgentEnvFileDir, systemdAgentLogDir); mkdirErr != nil { + return errors.Wrap(mkdirErr, "create systemd agent config/log directories") + } + + envFileLocal, err := uniqueTempFilePath("pf9-byohost-agent-*.conf") + if err != nil { + return errors.Wrap(err, "allocate local temp path for systemd agent EnvironmentFile") + } + defer os.Remove(envFileLocal) //nolint:errcheck // best-effort local temp file cleanup + + // BOOTSTRAP_KUBECONFIG points at the same in-container path SetupByoDockerHost already wrote + // the kubeconfig to (bootstrapConfPath) -- no need for a second copy at the path + // docs/agent-upgrade.md's onboarding flow uses, since this harness controls both sides. + // REGION must be "key=value" -- the unit's ExecStart passes it straight through as + // --label "$REGION", and agent/main.go's labelFlags.Set rejects anything without an "=". + // PATH is set explicitly (covering /usr/local/bin, where installImgpkgOnHost places its + // imgpkg wrapper) since the real agent finds it via a plain exec.LookPath("imgpkg"), and + // systemd's own default PATH for services isn't guaranteed to include it. + envContent := fmt.Sprintf("NAMESPACE=%s\nBOOTSTRAP_KUBECONFIG=%s\nREGION=region=e2e\nPATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\n", namespace, bootstrapConfPath) + if err := os.WriteFile(envFileLocal, []byte(envContent), 0600); err != nil { + return errors.Wrap(err, "write local systemd agent EnvironmentFile") + } + + if err := copyToContainer(ctx, dockerClient, cpConfig{ + sourcePath: envFileLocal, + destPath: systemdAgentEnvFilePath, + container: containerID, + }); err != nil { + return errors.Wrap(err, "copy systemd agent EnvironmentFile") + } + + return runContainerCommand(ctx, dockerClient, containerID, "sh", "-c", + "systemctl daemon-reload && systemctl enable "+systemdAgentServiceUnitName+" && systemctl start "+systemdAgentServiceUnitName) +} + +// runContainerCommand execs cmd inside containerID and returns an error including captured +// output if it exits non-zero -- unlike raiseInotifyInstanceLimit's fire-and-forget ExecStart, +// callers here need to know if e.g. `systemctl start` actually failed. +func runContainerCommand(ctx context.Context, dockerClient *client.Client, containerID string, cmd ...string) error { + _, err := containerCommandOutput(ctx, dockerClient, containerID, cmd...) + return err +} + +// containerCommandOutput is runContainerCommand's sibling for callers that need the command's +// stdout/stderr, not just success/failure (e.g. reading back a MainPID). +func containerCommandOutput(ctx context.Context, dockerClient *client.Client, containerID string, cmd ...string) (string, error) { + execCmd, err := dockerClient.ContainerExecCreate(ctx, containerID, container.ExecOptions{ + AttachStdout: true, + AttachStderr: true, + Cmd: cmd, + }) + if err != nil { + return "", err + } + resp, err := dockerClient.ContainerExecAttach(ctx, execCmd.ID, container.ExecAttachOptions{}) + if err != nil { + return "", err + } + // ContainerExecCreate above didn't set Tty, so the attach stream is stdcopy's multiplexed + // stdout/stderr framing, not plain bytes -- read it raw and any output we try to parse (e.g. + // MainPID) silently corrupts. Demux both into one buffer; ordering between the two doesn't + // matter for the diagnostic-string/parsed-value use cases here. + var output bytes.Buffer + _, _ = stdcopy.StdCopy(&output, &output, resp.Reader) //nolint:errcheck // best-effort diagnostic output, exit-code check below is authoritative + resp.Close() + + inspect, err := dockerClient.ContainerExecInspect(ctx, execCmd.ID) + if err != nil { + return "", err + } + if inspect.ExitCode != 0 { + return "", fmt.Errorf("command %v exited %d: %s", cmd, inspect.ExitCode, output.String()) + } + return output.String(), nil +} + +// copySystemdAgentLog returns a byoHostHandle.StopLog-shaped closer that, unlike the live stream +// StreamDockerLog attaches for the bare-exec path, reads the systemd-supervised agent's log file +// (ExecStart redirects there, not to a docker exec stream) once, at call time -- typically +// deferred to right before teardown, same calling convention. +func copySystemdAgentLog(ctx context.Context, dockerClient *client.Client, containerID, localPath string) func() { + return func() { + execCmd, err := dockerClient.ContainerExecCreate(ctx, containerID, container.ExecOptions{ + AttachStdout: true, + AttachStderr: true, + Cmd: []string{"cat", systemdAgentLogFile}, + }) + if err != nil { + return + } + resp, err := dockerClient.ContainerExecAttach(ctx, execCmd.ID, container.ExecAttachOptions{}) + if err != nil { + return + } + defer resp.Close() + + f, err := os.Create(localPath) //nolint:gosec // localPath is test-generated (fmt.Sprintf with a random suffix), not user input + if err != nil { + return + } + defer f.Close() + // Same multiplexed-stream caveat as containerCommandOutput -- demux, don't read raw. + _, _ = stdcopy.StdCopy(f, f, resp.Reader) //nolint:errcheck // best-effort log snapshot for test diagnostics + } +} + +// mainPID reads the current MainPID systemd reports for the agent unit inside containerID, +// returning 0 if the unit isn't running or the read fails (e.g. mid-restart) rather than erroring +// -- callers poll this via Eventually, where a transient 0 is expected, not a failure. +func mainPID(ctx context.Context, dockerClient *client.Client, containerID string) int { + out, err := containerCommandOutput(ctx, dockerClient, containerID, + "systemctl", "show", "-p", "MainPID", "--value", systemdAgentServiceUnitName) + if err != nil { + return 0 + } + pid, err := strconv.Atoi(strings.TrimSpace(out)) + if err != nil { + return 0 + } + return pid +} diff --git a/test/e2e/byohost_spinup_helper_test.go b/test/e2e/byohost_spinup_helper_test.go index 1c38361a5..70d2e50b2 100644 --- a/test/e2e/byohost_spinup_helper_test.go +++ b/test/e2e/byohost_spinup_helper_test.go @@ -8,6 +8,7 @@ import ( "context" "fmt" + "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" "sigs.k8s.io/cluster-api/util" ) @@ -21,9 +22,30 @@ type byoHostHandle struct { LogFilePath string } -// spinUpByoHosts creates count BYO hosts. On error it returns the handles created so -// far alongside the error. +// spinUpByoHosts creates count BYO hosts, each running agentBinaryPath as a bare, attached +// `docker exec` process. On error it returns the handles created so far alongside the error. func spinUpByoHosts(ctx context.Context, dockerClient *client.Client, namespace string, count int) ([]byoHostHandle, error) { + return spinUpByoHostsCommon(ctx, dockerClient, namespace, count, pathToHostAgentBinary, + func(runner *ByoHostRunner, byohost *container.CreateResponse) (func(), string, error) { + output, _, err := runner.ExecByoDockerHost(byohost) + if err != nil { + return nil, "", err + } + logFilePath := fmt.Sprintf("/tmp/host-agent-%s.log", runner.ByoHostName) + return StreamDockerLog(output, logFilePath), logFilePath, nil + }) +} + +// spinUpByoHostsCommon creates count BYO hosts and, for each, calls startAgent once the container +// exists -- shared between spinUpByoHosts (bare docker exec) and +// spinUpByoHostsWithSystemdAgent (systemd unit), which differ only in how the agent process +// actually gets started. A host is always appended to the returned slice before startAgent runs, +// so a failure partway through it still leaves the container tracked for the caller's +// teardownByoHosts -- otherwise its ID is lost and the container leaks. +func spinUpByoHostsCommon(ctx context.Context, dockerClient *client.Client, namespace string, count int, agentBinaryPath string, + startAgent func(runner *ByoHostRunner, byohost *container.CreateResponse) (stopLog func(), logFilePath string, err error), +) ([]byoHostHandle, error) { + hosts := make([]byoHostHandle, 0, count) for i := 0; i < count; i++ { @@ -34,7 +56,7 @@ func spinUpByoHosts(ctx context.Context, dockerClient *client.Client, namespace clusterConName: clusterConName, ByoHostName: byoHostName, Namespace: namespace, - PathToHostAgentBinary: pathToHostAgentBinary, + PathToHostAgentBinary: agentBinaryPath, DockerClient: dockerClient, NetworkInterface: dockerNetworkInterfaceKind, Env: byoHostRunnerEnv(), @@ -59,13 +81,11 @@ func spinUpByoHosts(ctx context.Context, dockerClient *client.Client, namespace LogFilePath: "", }) - output, _, err := runner.ExecByoDockerHost(byohost) + stopLog, logFilePath, err := startAgent(&runner, byohost) if err != nil { return hosts, err } - - logFilePath := fmt.Sprintf("/tmp/host-agent-%s.log", byoHostName) - hosts[len(hosts)-1].StopLog = StreamDockerLog(output, logFilePath) + hosts[len(hosts)-1].StopLog = stopLog hosts[len(hosts)-1].LogFilePath = logFilePath }