From 3a595387589e1d6bd4c732ea47c381f78c92357e Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Fri, 7 Aug 2026 16:34:31 -0300 Subject: [PATCH 1/3] Add HAProxy version upgrade tests The IngressControllerMultipleHAProxyVersions feature allows selecting HAProxy versions per IngressController. During y-stream upgrades (e.g. 4.22 to 5.0), the default HAProxy version may change (2.8 to 3.2), and versions can be deprecated ahead of removal in a later release. Add HAProxyVersionUpgradeTest, a single upgrade test parameterized by Mode and registered three times to cover the scenarios that matter across an upgrade: - Unset: no HAProxyVersion set; the running version must follow whatever the new release's default becomes after the upgrade. - Default: pinned to the current default version; must retain that exact version after upgrade. - NonDefault: pinned to a supported, non-default, non-deprecated version; must retain that exact version after upgrade. Skip() avoids exercising unsafe scenarios: it skips when the IngressController API lacks the haproxyVersion field, when a pinned mode would run against a multi-hop upgrade chain (a version could be deprecated and removed between hops, with no reliable way to predict that beforehand), and when NonDefault mode has no non-deprecated non-default version available to test with. This favors skipping over risking the shared upgrade job, since a pinned version that blocks the CVO would fail every other upgrade test running alongside it. Setup creates a custom IngressController with the version implied by Mode and confirms the runtime HAProxy version matches before the upgrade starts. Test waits for the upgrade to complete, resolves the expected version (the post-upgrade default for Unset, the original pin otherwise), and validates it via both the IngressController status (EffectiveHAProxyVersion) and the HAProxy runtime socket. Also refactors multi-haproxy.go: extracts shared helpers (apiHasHAProxyVersionField, getHAProxyVersionConfig and its non-default/upgradeable version derivation) used by both the day-2 tests and the new upgrade tests, and makes teardown delete IngressControllers concurrently instead of sequentially. https://redhat.atlassian.net/browse/NE-2839 Co-Authored-By: Claude Opus 4.6 --- test/e2e/upgrade/upgrade.go | 3 + .../extended/router/haproxyversion_upgrade.go | 330 ++++++++++++++++++ test/extended/router/multi-haproxy.go | 224 +++++++----- 3 files changed, 467 insertions(+), 90 deletions(-) create mode 100644 test/extended/router/haproxyversion_upgrade.go diff --git a/test/e2e/upgrade/upgrade.go b/test/e2e/upgrade/upgrade.go index fd408beee0ed..c0e5c6a67b27 100644 --- a/test/e2e/upgrade/upgrade.go +++ b/test/e2e/upgrade/upgrade.go @@ -72,6 +72,9 @@ func AllTests() []upgrades.Test { &prometheus.MetricsAvailableAfterUpgradeTest{}, &dns.UpgradeTest{}, &router.GatewayAPIUpgradeTest{}, + &router.HAProxyVersionUpgradeTest{Mode: router.HAProxyUpgradeModeUnset}, + &router.HAProxyVersionUpgradeTest{Mode: router.HAProxyUpgradeModeNonDefault}, + &router.HAProxyVersionUpgradeTest{Mode: router.HAProxyUpgradeModeDefault}, } } diff --git a/test/extended/router/haproxyversion_upgrade.go b/test/extended/router/haproxyversion_upgrade.go new file mode 100644 index 000000000000..47deb8084512 --- /dev/null +++ b/test/extended/router/haproxyversion_upgrade.go @@ -0,0 +1,330 @@ +package router + +import ( + "context" + "fmt" + "slices" + "strings" + "time" + + g "github.com/onsi/ginkgo/v2" + o "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/kubernetes/test/e2e/framework" + "k8s.io/kubernetes/test/e2e/upgrades" + + operatorv1 "github.com/openshift/api/operator/v1" + operatorv1client "github.com/openshift/client-go/operator/clientset/versioned" + exutil "github.com/openshift/origin/test/extended/util" +) + +// HAProxyVersionUpgradeTest verifies that HAProxy version selection behaves +// as expected during upgrades. +// Mode is a test parameter that should define how the HAProxy version +// should be configured before the upgrade. +type HAProxyVersionUpgradeTest struct { + Mode HAProxyUpgradeMode + + // internal state + oc *exutil.CLI + operatorClient operatorv1client.Interface + controllers *ingressControllers + versionConfig haproxyVersionConfig + pinnedVersion operatorv1.HAProxyVersion + precheckErr error + ic types.NamespacedName +} + +// HAProxyUpgradeMode is the mode of the HAProxy upgrade. +type HAProxyUpgradeMode string + +const ( + // HAProxyUpgradeModeUnset defines the HAProxy version as unpinned before the upgrade. + HAProxyUpgradeModeUnset HAProxyUpgradeMode = "unset" + // HAProxyUpgradeModeDefault defines the HAProxy version with the default version before the upgrade. + HAProxyUpgradeModeDefault HAProxyUpgradeMode = "default" + // HAProxyUpgradeModeNonDefault defines the HAProxy version with a non default but supported version before the upgrade. + HAProxyUpgradeModeNonDefault HAProxyUpgradeMode = "non-default" +) + +func (h *HAProxyVersionUpgradeTest) Name() string { + return "haproxy-version-upgrade-" + string(h.Mode) +} + +func (h *HAProxyVersionUpgradeTest) DisplayName() string { + return fmt.Sprintf("[sig-network-edge][Feature:Router][apigroup:route.openshift.io] Verify HAProxy %s version state during upgrade", h.Mode) +} + +// Skip returns true when the test cannot safely run: the API lacks the haproxyVersion field, the upgrade +// is a multi-hop chain with a pinned version, or (for NonDefault) no safe non-default version is available. +func (h *HAProxyVersionUpgradeTest) Skip(upgctx upgrades.UpgradeContext) bool { + framework.Logf("Upgrade config: %+v", upgctx) + + if h.Mode != HAProxyUpgradeModeUnset && len(upgctx.Versions) > 2 { + // We could have a deprecation and dropping version in the middle of a + // multi-hop upgrade, so we cannot safely run the test having HAProxy pinned. + framework.Logf("skipping: cannot test a multi-hop upgrade with HAProxy pinned. mode=%q, versions=%d", h.Mode, len(upgctx.Versions)) + return true + } + + ctx := context.Background() + + oc := exutil.NewCLIForMonitorTest(h.Name() + "-skip").AsAdmin() + hasField, err := apiHasHAProxyVersionField(ctx, oc) + if err != nil { + h.precheckErr = fmt.Errorf("error checking for HAProxy version API: %w", err) + return false + } + if !hasField { + framework.Logf("skipping: IngressController API is missing the haproxyVersion field") + return true + } + + versions, err := getHAProxyVersionConfig(ctx, oc) + if err != nil { + h.precheckErr = fmt.Errorf("error getting HAProxy version config: %w", err) + return false + } + framework.Logf("HAProxy version config: %+v", versions) + + if h.Mode == HAProxyUpgradeModeNonDefault && len(versions.getNonDefaultVersions()) == 0 { + framework.Logf("skipping: cannot use non default: there are no non default versions") + return true + } + + if h.Mode == HAProxyUpgradeModeNonDefault && len(versions.getNonDefaultUpgradeableVersions()) == 0 { + // Strictly, only a y-stream upgrade could drop this version, but Skip() cannot + // reliably tell y-stream from z-stream before the upgrade completes (the target + // may be given as a pull-spec, not a parseable Version), so we skip conservatively + // regardless of upgrade type. + framework.Logf("skipping: cannot use non default: the only available non default version is deprecated") + return true + } + + h.versionConfig = versions + h.precheckErr = nil + return false +} + +// Setup configures all the test attributes and creates an IngressController +// resource that should be verified after the upgrade. +func (h *HAProxyVersionUpgradeTest) Setup(ctx context.Context, f *framework.Framework) { + o.Expect(h.precheckErr).NotTo(o.HaveOccurred(), "Skip() precheck failed: could not determine if HAProxy version upgrade test should run") + + g.By("Setting up HAProxy version test") + + h.oc = exutil.NewCLIWithFramework(f).AsAdmin() + h.operatorClient = h.oc.AdminOperatorClient() + h.controllers = &ingressControllers{} + + var haproxyVersion operatorv1.HAProxyVersion + // Default and NonDefault cases should have their versions inverted in case + // the default version is overridden, see getHAProxyVersionConfig(). + switch h.Mode { + case HAProxyUpgradeModeUnset: + haproxyVersion = "" + case HAProxyUpgradeModeDefault: + haproxyVersion = h.versionConfig.defaultVersion + case HAProxyUpgradeModeNonDefault: + haproxyVersion = h.versionConfig.getNonDefaultUpgradeableVersions()[0] + default: + framework.Failf("unsupported test mode: %q", h.Mode) + } + + g.By("Creating the IngressController resource") + + ic, err := h.controllers.createIngressController(ctx, h.oc, func(controller *operatorv1.IngressController) { + controller.Spec.HAProxyVersion = haproxyVersion + }) + o.Expect(err).NotTo(o.HaveOccurred(), "error creating IngressController resource") + h.ic = types.NamespacedName{ + Namespace: ic.Namespace, + Name: ic.Name, + } + h.pinnedVersion = haproxyVersion + + framework.Logf("Created IngressController %s with spec.haproxyVersion=%q", h.ic.String(), haproxyVersion) + + g.By("Checking HAProxy version for Ingress " + h.ic.String()) + + waitingVersion := haproxyVersion + if waitingVersion == "" { + waitingVersion = h.versionConfig.defaultVersion + } + err = waitForHAProxyVersion(ctx, h.oc, ic.Name, waitingVersion) + o.Expect(err).NotTo(o.HaveOccurred(), "error getting HAProxy version from runtime API") +} + +// Test verifies that the expected HAProxy version is found after the upgrade. +// Current version is read from the IngressController status and from the +// HAProxy's runtime API. +func (h *HAProxyVersionUpgradeTest) Test(ctx context.Context, f *framework.Framework, done <-chan struct{}, upgrade upgrades.UpgradeType) { + defer g.GinkgoRecover() + + g.By("Waiting for upgrade to complete") + <-done + + err := waitForIngressControllerReady(h.oc, h.ic) + o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("error waiting for IngressController %s to be ready", h.ic.String())) + + g.By("Validating HAProxy version after upgrade") + + versions, err := getHAProxyVersionConfig(ctx, h.oc) + o.Expect(err).NotTo(o.HaveOccurred(), "error getting HAProxy version config") + framework.Logf("HAProxy version config: %+v", versions) + + var expectedVersion operatorv1.HAProxyVersion + switch h.Mode { + case HAProxyUpgradeModeUnset: + expectedVersion = versions.defaultVersion + case HAProxyUpgradeModeDefault, HAProxyUpgradeModeNonDefault: + expectedVersion = h.pinnedVersion + default: + framework.Failf("unsupported test mode: %q", h.Mode) + } + + framework.Logf("Post-upgrade HAProxy version check: expected=%s", expectedVersion) + + const rollingOutTimeout = 15 * time.Minute + err = waitForEffectiveHAProxyVersion(ctx, h.operatorClient, h.ic, expectedVersion, rollingOutTimeout) + o.Expect(err).NotTo(o.HaveOccurred(), "error waiting for EffectiveHAProxyVersion") + + g.By("Validating HAProxy version from runtime API") + + err = waitForHAProxyVersion(ctx, h.oc, h.ic.Name, expectedVersion) + o.Expect(err).NotTo(o.HaveOccurred(), "error getting HAProxy version from runtime API") +} + +// Teardown removes the configured IngressController after the test runs. +func (h *HAProxyVersionUpgradeTest) Teardown(ctx context.Context, f *framework.Framework) { + if h.operatorClient == nil { + framework.Logf("Skipping cleanup because setup did not initialize test resources") + return + } + if err := h.controllers.deleteAll(ctx, h.operatorClient); err != nil { + framework.Logf("error deleting IngressController resource: %s", err.Error()) + } +} + +// haproxyVersionConfig has HAProxy version configuration from the Ingress operator. +type haproxyVersionConfig struct { + defaultVersion operatorv1.HAProxyVersion + deprecatedVersion operatorv1.HAProxyVersion + availableVersions []operatorv1.HAProxyVersion +} + +// getHAProxyVersionConfig parses the current Ingress operator configuration and extracts +// the HAProxy version configuration. The default version can be overridden if the cluster +// has an unsupported HAProxy version override annotation set. +func getHAProxyVersionConfig(ctx context.Context, oc *exutil.CLI) (haproxyVersionConfig, error) { + operatorNamespace := "openshift-ingress-operator" + operatorName := "ingress-operator" + deploy, err := oc.AdminKubeClient().AppsV1().Deployments(operatorNamespace).Get(ctx, operatorName, metav1.GetOptions{}) + if err != nil { + return haproxyVersionConfig{}, err + } + + containers := deploy.Spec.Template.Spec.Containers + if len(containers) < 1 { + return haproxyVersionConfig{}, fmt.Errorf("ingress-operator deployment is missing the operator container") + } + + operator := containers[0] + if operator.Name != operatorName { + return haproxyVersionConfig{}, fmt.Errorf("ingress-operator deployment has an unexpected container name: %s", operator.Name) + } + + // Checking if HAProxy version is being overridden: + // + // oc annotate ingress.config cluster \ + // unsupported.ingress.openshift.io/default-haproxy-version="${HAPROXY_VERSION}" \ + // --overwrite + // + // https://github.com/openshift/release/blob/6833a2362a7e48156c4872d669a018b06123da2c/ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.sh#L17-L19 + ingCluster, err := oc.AdminConfigClient().ConfigV1().Ingresses().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return haproxyVersionConfig{}, fmt.Errorf("error reading Ingress cluster config for HAProxy version override check: %w", err) + } + + // missing ingress.config/cluster annotation means not overridden, so defaultVersion remains empty. + var defaultVersion operatorv1.HAProxyVersion + if overrideVersion := ingCluster.Annotations["unsupported.ingress.openshift.io/default-haproxy-version"]; overrideVersion != "" { + framework.Logf("HAProxy default version overridden to %q via ingress.config/cluster annotation", overrideVersion) + defaultVersion = operatorv1.HAProxyVersion(overrideVersion) + } + + // Read default and deprecated versions from Env + var deprecatedVersion operatorv1.HAProxyVersion + for _, env := range operator.Env { + switch env.Name { + case "DEFAULT_HAPROXY_VERSION": + if defaultVersion == "" { + // update only if not being overridden. + defaultVersion = operatorv1.HAProxyVersion(env.Value) + } + case "DEPRECATED_HAPROXY_VERSION": + deprecatedVersion = operatorv1.HAProxyVersion(env.Value) + } + } + if defaultVersion == "" { + // envvar not found and version not overridden, so this is pre 4.23/5.0, assume "2.8" + defaultVersion = "2.8" + } + + // Read available versions from Command. + // The available versions are configured this way: + // + // command: + // - ... + // - --haproxy-image + // - "2.8=$(HAPROXY_28_IMAGE)" + // - --haproxy-image + // - "3.2=$(HAPROXY_32_IMAGE)" + // + var availableVersions []operatorv1.HAProxyVersion + cmds := operator.Command + for i := range cmds { + if cmds[i] == "--haproxy-image" && len(cmds) > i+1 { + // "2.8=$(HAPROXY_28_IMAGE)" + value := cmds[i+1] + // ["2.8", "$(HAPROXY_28_IMAGE)"] + version := strings.Split(value, "=") + availableVersions = append(availableVersions, operatorv1.HAProxyVersion(version[0])) + } + } + if len(availableVersions) == 0 { + // --haproxy-image not configured, so this is pre 4.23/5.0, assume [defaultVersion] + availableVersions = []operatorv1.HAProxyVersion{defaultVersion} + } + + // a sanity check ensures default and deprecated are valid versions. + if !slices.Contains(availableVersions, defaultVersion) { + return haproxyVersionConfig{}, + fmt.Errorf("the available versions list %v does not include the default version %q", availableVersions, defaultVersion) + } + if deprecatedVersion != "" && !slices.Contains(availableVersions, deprecatedVersion) { + return haproxyVersionConfig{}, + fmt.Errorf("the available versions list %v does not include the deprecated version %q", availableVersions, deprecatedVersion) + } + + return haproxyVersionConfig{ + defaultVersion: defaultVersion, + deprecatedVersion: deprecatedVersion, + availableVersions: availableVersions, + }, nil +} + +// getNonDefaultVersions creates a list of non default versions, derived from the default and the available ones. +func (h *haproxyVersionConfig) getNonDefaultVersions() []operatorv1.HAProxyVersion { + return slices.DeleteFunc(slices.Clone(h.availableVersions), func(v operatorv1.HAProxyVersion) bool { + return v == h.defaultVersion + }) +} + +// getNonDefaultUpgradeableVersions creates a list of non default and upgradeable versions, derived from the default, the deprecated, and the available ones. +func (h *haproxyVersionConfig) getNonDefaultUpgradeableVersions() []operatorv1.HAProxyVersion { + return slices.DeleteFunc(slices.Clone(h.availableVersions), func(v operatorv1.HAProxyVersion) bool { + return v == h.defaultVersion || v == h.deprecatedVersion + }) +} diff --git a/test/extended/router/multi-haproxy.go b/test/extended/router/multi-haproxy.go index 7536293414a2..53ed561381f8 100644 --- a/test/extended/router/multi-haproxy.go +++ b/test/extended/router/multi-haproxy.go @@ -5,25 +5,28 @@ import ( "errors" "fmt" "strings" + "sync" "time" g "github.com/onsi/ginkgo/v2" o "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/storage/names" + e2e "k8s.io/kubernetes/test/e2e/framework" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" operatorv1 "github.com/openshift/api/operator/v1" - apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + operatorv1client "github.com/openshift/client-go/operator/clientset/versioned" "github.com/openshift/origin/test/extended/router/shard" exutil "github.com/openshift/origin/test/extended/util" - e2e "k8s.io/kubernetes/test/e2e/framework" ) var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift.io][OCPFeatureGate:IngressControllerMultipleHAProxyVersions]", func() { @@ -32,10 +35,10 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. // testsTimeout defines the maximum amount of time to wait for test operations to complete. const testsTimeout = 5 * time.Minute - // defaultHAProxyVersion is the default HAProxy version for the current release. - var defaultHAProxyVersion operatorv1.HAProxyVersion + // versionConfig is the HAProxy version configuration in the current release. + var versionConfig haproxyVersionConfig - //alternateHAProxyVersion is the other accepted HAProxyVersion accepted in the current release + // alternateHAProxyVersion is one of the non default accepted HAProxyVersions in the current release. var alternateHAProxyVersion operatorv1.HAProxyVersion // controllers is used to create new ingress controllers, and stores their reference so they can be removed after the test runs @@ -51,46 +54,29 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. exutil.DumpPodLogsStartingWithInNamespace(ic.controller.Name, ic.controller.Namespace, oc) } } - var errs []error - for _, ic := range controllers.items { - err := operatorClient.OperatorV1().IngressControllers(ic.controller.Namespace).Delete(ctx, ic.controller.Name, *metav1.NewDeleteOptions(1)) - errs = append(errs, client.IgnoreNotFound(err)) - } - o.Expect(errors.Join(errs...)).NotTo(o.HaveOccurred()) + err := controllers.deleteAll(ctx, operatorClient) + o.Expect(err).NotTo(o.HaveOccurred()) controllers.items = nil }) g.BeforeEach(func() { - - apiExtClient, err := apiextensionsclient.NewForConfig(oc.AdminConfig()) - o.Expect(err).NotTo(o.HaveOccurred()) - - crd, err := apiExtClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, "ingresscontrollers.operator.openshift.io", metav1.GetOptions{}) + hasField, err := apiHasHAProxyVersionField(ctx, oc) o.Expect(err).NotTo(o.HaveOccurred()) - - // Check if haproxyVersion field exists in the CRD schema - hasField := false - for _, v := range crd.Spec.Versions { - if v.Name == "v1" && v.Schema != nil && v.Schema.OpenAPIV3Schema != nil { - if _, ok := v.Schema.OpenAPIV3Schema.Properties["spec"].Properties["haproxyVersion"]; ok { - hasField = true - } - } - } if !hasField { g.Skip("IngressController CRD does not have haproxyVersion field — operator not yet updated") } - defaultIC, err := operatorClient.OperatorV1().IngressControllers("openshift-ingress-operator").Get(ctx, "default", metav1.GetOptions{}) + versions, err := getHAProxyVersionConfig(ctx, oc) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(defaultIC.Status.EffectiveHAProxyVersion).NotTo(o.BeEmpty()) - defaultHAProxyVersion = defaultIC.Status.EffectiveHAProxyVersion - if defaultHAProxyVersion == operatorv1.HAProxyVersion28 { - alternateHAProxyVersion = operatorv1.HAProxyVersion32 - } else { - alternateHAProxyVersion = operatorv1.HAProxyVersion28 + nonDefaultVersions := versions.getNonDefaultVersions() + if len(nonDefaultVersions) == 0 { + g.Skip("IngressController has no non default versions available") } + + // update shared vars + versionConfig = versions + alternateHAProxyVersion = nonDefaultVersions[0] }) g.Describe("The HAProxy router with version selection", func() { @@ -98,15 +84,15 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. // Ensure that the haproxyVersion field in the IngressController API does not accept unknown versions g.It("should reject invalid HAProxy versions", func() { versions := []operatorv1.HAProxyVersion{ - " ", // Empty becomes unset, but space is invalid - "2.6", // one LTS before the oldest supported version, so always invalid - "v" + defaultHAProxyVersion, // v prefix is invalid - defaultHAProxyVersion + ".0", // .z suffix is invalid, only x.y is supported - " " + defaultHAProxyVersion, // leading space is invalid - defaultHAProxyVersion + " ", // trailing space is invalid + " ", // Empty becomes unset, but space is invalid + "2.6", // one LTS before the oldest supported version, so always invalid + "v" + versionConfig.defaultVersion, // v prefix is invalid + versionConfig.defaultVersion + ".0", // .z suffix is invalid, only x.y is supported + " " + versionConfig.defaultVersion, // leading space is invalid + versionConfig.defaultVersion + " ", // trailing space is invalid } for _, version := range versions { - _, err := controllers.createIngressController(ctx, oc, testsTimeout, func(ic *operatorv1.IngressController) { + _, err := controllers.createIngressController(ctx, oc, func(ic *operatorv1.IngressController) { ic.Spec.HAProxyVersion = version }) o.Expect(err).To(o.Not(o.Succeed())) @@ -116,7 +102,7 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. // Ensure that the ingress controller reverts back to the default version after unsetting the field with null g.It("should revert to default HAProxy version when field is cleared", func() { - ingress, err := controllers.createIngressController(ctx, oc, testsTimeout, func(ic *operatorv1.IngressController) { + ingress, err := controllers.createIngressController(ctx, oc, func(ic *operatorv1.IngressController) { ic.Spec.HAProxyVersion = alternateHAProxyVersion }) o.Expect(err).NotTo(o.HaveOccurred()) @@ -131,35 +117,21 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. o.Expect(err).NotTo(o.HaveOccurred()) g.By("Confirm that the HAProxy version shows default version") - err = waitForHAProxyVersion(ctx, oc, ingress.Name, defaultHAProxyVersion) + err = waitForHAProxyVersion(ctx, oc, ingress.Name, versionConfig.defaultVersion) o.Expect(err).NotTo(o.HaveOccurred()) }) // Ensure that the running HAProxy version matches the version configured in the IngressController API g.It("should configure the same HAProxy version defined in the API", func() { - versions := []operatorv1.HAProxyVersion{ - defaultHAProxyVersion, - alternateHAProxyVersion, - } - for _, version := range versions { - ingress, err := controllers.createIngressController(ctx, oc, testsTimeout, func(ic *operatorv1.IngressController) { + for _, version := range versionConfig.availableVersions { + ingress, err := controllers.createIngressController(ctx, oc, func(ic *operatorv1.IngressController) { ic.Spec.HAProxyVersion = version }) o.Expect(err).To(o.Succeed()) - errPoll := wait.PollUntilContextTimeout(ctx, 2*time.Second, testsTimeout, true, func(ctx context.Context) (bool, error) { - ic, err := operatorClient.OperatorV1().IngressControllers(ingress.Namespace).Get(ctx, ingress.Name, metav1.GetOptions{}) - if err != nil { - e2e.Logf("Failed to get the IngressController %s", ingress.Name) - return false, nil - } - if ic.Status.EffectiveHAProxyVersion == version { - e2e.Logf("EffectiveHAProxyVersion shows the expected version: %q", version) - return true, nil - } - e2e.Logf("EffectiveHAProxyVersion: %q does not match the expected version %q", ic.Status.EffectiveHAProxyVersion, version) - return false, nil - }) + errPoll := waitForEffectiveHAProxyVersion(ctx, operatorClient, types.NamespacedName{Namespace: ingress.Namespace, Name: ingress.Name}, version, testsTimeout) o.Expect(errPoll).NotTo(o.HaveOccurred(), "Timed out waiting for EffectiveHAProxyVersion") + err = waitForHAProxyVersion(ctx, oc, ingress.Name, version) + o.Expect(err).NotTo(o.HaveOccurred(), "error getting HAProxy version from runtime API") e2e.Logf("IngressController: %s matches the expected HAProxyVersion: %s", ingress.Name, string(version)) } }) @@ -167,31 +139,17 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. // Ensure that if the version is unset, its value in the IngressController API remains undeclared, and the running HAProxy matches the default version g.It("should configure the default HAProxy if the version is unset", func() { // create a custom ingress controller with an unset HAProxyVersion - ingress, err := controllers.createIngressController(ctx, oc, testsTimeout, nil) + ingress, err := controllers.createIngressController(ctx, oc, nil) o.Expect(err).To(o.Succeed()) //confirm that the ingresscontroller is unset o.Expect(ingress.Spec.HAProxyVersion).To(o.BeEmpty()) - var effectiveVersion operatorv1.HAProxyVersion - err = wait.PollUntilContextTimeout(ctx, 2*time.Second, testsTimeout, true, func(ctx context.Context) (bool, error) { - ic, err := operatorClient.OperatorV1().IngressControllers(ingress.Namespace).Get(ctx, ingress.Name, metav1.GetOptions{}) - if err != nil { - e2e.Logf("Failed to get the IngressController %s", ingress.Name) - return false, nil - } - if ic.Status.EffectiveHAProxyVersion == "" { - e2e.Logf("IngressController %s: EffectiveHAProxyVersion not yet set, waiting...", ingress.Name) - return false, nil - } - effectiveVersion = ic.Status.EffectiveHAProxyVersion - return true, nil - }) + err = waitForEffectiveHAProxyVersion(ctx, operatorClient, types.NamespacedName{Namespace: ingress.Namespace, Name: ingress.Name}, versionConfig.defaultVersion, testsTimeout) o.Expect(err).NotTo(o.HaveOccurred(), "Timed out waiting for EffectiveHAProxyVersion") - o.Expect(effectiveVersion).To(o.Equal(defaultHAProxyVersion)) - e2e.Logf("IngressController: %s has the expected HAProxyVersion: %s", ingress.Name, string(defaultHAProxyVersion)) - err = waitForHAProxyVersion(ctx, oc, ingress.Name, defaultHAProxyVersion) - o.Expect(err).NotTo(o.HaveOccurred()) + e2e.Logf("IngressController: %s has the expected HAProxyVersion: %s", ingress.Name, string(versionConfig.defaultVersion)) + err = waitForHAProxyVersion(ctx, oc, ingress.Name, versionConfig.defaultVersion) + o.Expect(err).NotTo(o.HaveOccurred(), "error getting HAProxy version from runtime API") }) // Ensure that changing the HAProxy version of a custom IngressController does not affect the running HAProxy of the default router @@ -202,14 +160,14 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. ingressVersion := ingressDefault.Status.EffectiveHAProxyVersion g.By("Create a custom controller and patch it to an older version") - ingress, err := controllers.createIngressController(ctx, oc, testsTimeout, nil) + ingress, err := controllers.createIngressController(ctx, oc, nil) o.Expect(err).To(o.Succeed()) patch := []byte(fmt.Sprintf(`{"spec":{"haproxyVersion":"%s"}}`, alternateHAProxyVersion)) _, err = operatorClient.OperatorV1().IngressControllers(ingress.Namespace).Patch(ctx, ingress.Name, types.MergePatchType, patch, metav1.PatchOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - g.By("Confirm the HaProxy version matches") + g.By("Confirm the HAProxy version matches") err = waitForHAProxyVersion(ctx, oc, ingress.Name, alternateHAProxyVersion) o.Expect(err).NotTo(o.HaveOccurred()) @@ -229,7 +187,7 @@ type ingressController struct { controller types.NamespacedName } -func (i *ingressControllers) createIngressController(ctx context.Context, oc *exutil.CLI, readyTimeout time.Duration, custom func(ic *operatorv1.IngressController)) (*operatorv1.IngressController, error) { +func (i *ingressControllers) createIngressController(ctx context.Context, oc *exutil.CLI, custom func(ic *operatorv1.IngressController)) (*operatorv1.IngressController, error) { operatorClient := oc.AdminOperatorClient() // ingress controller need to be created in operator's namespace, ... @@ -272,20 +230,62 @@ func (i *ingressControllers) createIngressController(ctx context.Context, oc *ex } i.items = append(i.items, &ictr) + return ingress, waitForIngressControllerReady(oc, controller) +} + +func (i *ingressControllers) deleteAll(ctx context.Context, operatorClient operatorv1client.Interface) error { + errs := make([]error, len(i.items)) + wg := sync.WaitGroup{} + for idx, ic := range i.items { + wg.Go(func() { + if err := deleteIngressControllerAndWait(ctx, operatorClient, ic.controller); err != nil { + errs[idx] = fmt.Errorf("error during IngressController %s deletion: %w", ic.controller.String(), err) + } + }) + } + wg.Wait() + return errors.Join(errs...) +} + +// waitForIngressControllerReady waits for the provided IngressController to be ready. +func waitForIngressControllerReady(oc *exutil.CLI, ic types.NamespacedName) error { ingressControllerReady := []operatorv1.OperatorCondition{ {Type: operatorv1.IngressControllerAvailableConditionType, Status: operatorv1.ConditionTrue}, {Type: operatorv1.LoadBalancerManagedIngressConditionType, Status: operatorv1.ConditionFalse}, {Type: operatorv1.DNSManagedIngressConditionType, Status: operatorv1.ConditionFalse}, {Type: operatorv1.OperatorStatusTypeProgressing, Status: operatorv1.ConditionFalse}, } + return shard.WaitForIngressControllerCondition(oc, 5*time.Minute, ic, ingressControllerReady...) +} - // wait for the controller to be available - err = shard.WaitForIngressControllerCondition(oc, readyTimeout, controller, ingressControllerReady...) - if err != nil { - return nil, err - } +// waitForIngressControllerDeletion waits for an IngressController to be removed. +func waitForIngressControllerDeletion(ctx context.Context, operatorClient operatorv1client.Interface, ic types.NamespacedName) error { + return wait.PollUntilContextTimeout(ctx, 2*time.Second, 5*time.Minute, false, func(ctx context.Context) (done bool, err error) { + _, err = operatorClient.OperatorV1().IngressControllers(ic.Namespace).Get(ctx, ic.Name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + e2e.Logf("IngressController %s has been deleted", ic.String()) + return true, nil + } + if err != nil { + e2e.Logf("error reading IngressController %s: %s", ic.String(), err.Error()) + } else { + e2e.Logf("waiting IngressController %s to be deleted", ic.String()) + } + return false, nil + }) +} - return ingress, nil +// deleteIngressControllerAndWait deletes an IngressController and waits for it to be removed. +func deleteIngressControllerAndWait(ctx context.Context, operatorClient operatorv1client.Interface, ic types.NamespacedName) error { + e2e.Logf("Deleting IngressController %s", ic.String()) + err := operatorClient.OperatorV1().IngressControllers(ic.Namespace).Delete(ctx, ic.Name, *metav1.NewDeleteOptions(1)) + if client.IgnoreNotFound(err) != nil { + return fmt.Errorf("error deleting IngressController %s: %w", ic.String(), err) + } + if err := waitForIngressControllerDeletion(ctx, operatorClient, ic); err != nil { + return fmt.Errorf("IngressController %s was not deleted: %w", ic.String(), err) + } + return nil } // poll the router pods HAProxy Container to check that the version is correctly asserted @@ -294,7 +294,9 @@ func waitForHAProxyVersion(ctx context.Context, oc *exutil.CLI, ingressName stri return fmt.Errorf("desiredVersion must not be empty") } err := wait.PollUntilContextTimeout(ctx, 2*time.Second, 2*time.Minute, false, func(ctx context.Context) (bool, error) { - haproxy, err := oc.Run("exec").Args("-n", "openshift-ingress", "-c", "haproxy", "deploy/router-"+ingressName, "--", "bash", "-c", "echo 'show version' | socat - /var/lib/haproxy/run/haproxy.sock").Output() + // running in the `router` container - the only one for 4.22 and earlier used on upgrade tests, + // and a valid one for 4.23/5.0 and newer since it also has socat and the haproxy socket. + haproxy, err := oc.Run("exec").Args("-n", "openshift-ingress", "-c", "router", "deploy/router-"+ingressName, "--", "bash", "-c", "echo 'show version' | socat - /var/lib/haproxy/run/haproxy.sock").Output() if err != nil { e2e.Logf("Failed to extract the HAProxy Version from IngressController %s", ingressName) return false, nil @@ -308,3 +310,45 @@ func waitForHAProxyVersion(ctx context.Context, oc *exutil.CLI, ingressName stri }) return err } + +// waitForEffectiveHAProxyVersion polls the provided IngressController until its EffectiveHAProxyVersion equals expectedVersion. +func waitForEffectiveHAProxyVersion(ctx context.Context, operatorClient operatorv1client.Interface, ingress types.NamespacedName, expectedVersion operatorv1.HAProxyVersion, testsTimeout time.Duration) error { + err := wait.PollUntilContextTimeout(ctx, 2*time.Second, testsTimeout, true, func(ctx context.Context) (bool, error) { + ic, err := operatorClient.OperatorV1().IngressControllers(ingress.Namespace).Get(ctx, ingress.Name, metav1.GetOptions{}) + if err != nil { + e2e.Logf("Failed to get the IngressController %s: %s", ingress.String(), err.Error()) + return false, nil + } + if ic.Status.EffectiveHAProxyVersion != expectedVersion { + e2e.Logf("IngressController %s: HAProxy version %q does not match expected value %q", ingress.String(), ic.Status.EffectiveHAProxyVersion, expectedVersion) + return false, nil + } + return true, nil + }) + if err != nil { + return fmt.Errorf("error waiting for EffectiveHAProxyVersion to match expected version: %s", err.Error()) + } + return nil +} + +func apiHasHAProxyVersionField(ctx context.Context, oc *exutil.CLI) (bool, error) { + apiExtClient, err := apiextensionsclient.NewForConfig(oc.AdminConfig()) + if err != nil { + return false, err + } + + crd, err := apiExtClient.ApiextensionsV1().CustomResourceDefinitions().Get(ctx, "ingresscontrollers.operator.openshift.io", metav1.GetOptions{}) + if err != nil { + return false, err + } + + // Check if haproxyVersion field exists in the CRD schema + for _, v := range crd.Spec.Versions { + if v.Name == "v1" && v.Schema != nil && v.Schema.OpenAPIV3Schema != nil { + if _, ok := v.Schema.OpenAPIV3Schema.Properties["spec"].Properties["haproxyVersion"]; ok { + return true, nil + } + } + } + return false, nil +} From 165153d71c73e53eb0093d186ab36eb5a2f5b6ed Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Wed, 2 Sep 2026 21:15:13 -0300 Subject: [PATCH 2/3] Add support to Hypershift --- .../extended/router/haproxyversion_upgrade.go | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/extended/router/haproxyversion_upgrade.go b/test/extended/router/haproxyversion_upgrade.go index 47deb8084512..7ab429ec38e5 100644 --- a/test/extended/router/haproxyversion_upgrade.go +++ b/test/extended/router/haproxyversion_upgrade.go @@ -11,9 +11,11 @@ import ( o "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/upgrades" + configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" operatorv1client "github.com/openshift/client-go/operator/clientset/versioned" exutil "github.com/openshift/origin/test/extended/util" @@ -218,9 +220,29 @@ type haproxyVersionConfig struct { // the HAProxy version configuration. The default version can be overridden if the cluster // has an unsupported HAProxy version override annotation set. func getHAProxyVersionConfig(ctx context.Context, oc *exutil.CLI) (haproxyVersionConfig, error) { + kubeClient := oc.AdminKubeClient() operatorNamespace := "openshift-ingress-operator" operatorName := "ingress-operator" - deploy, err := oc.AdminKubeClient().AppsV1().Deployments(operatorNamespace).Get(ctx, operatorName, metav1.GetOptions{}) + topology, err := exutil.GetControlPlaneTopology(oc) + if err != nil { + return haproxyVersionConfig{}, fmt.Errorf("error getting control plane topology: %w", err) + } + if *topology == configv1.ExternalTopologyMode { + mgmtKubeconfig, hcpNamespace, err := exutil.GetHypershiftManagementClusterConfigAndNamespace() + if err != nil { + return haproxyVersionConfig{}, fmt.Errorf("error getting HyperShift management cluster config: %w", err) + } + mgmtConfig, err := exutil.GetClientConfig(mgmtKubeconfig) + if err != nil { + return haproxyVersionConfig{}, fmt.Errorf("error loading HyperShift management cluster config: %w", err) + } + if kubeClient, err = kubernetes.NewForConfig(mgmtConfig); err != nil { + return haproxyVersionConfig{}, fmt.Errorf("error building HyperShift management cluster client: %w", err) + } + operatorNamespace = hcpNamespace + } + + deploy, err := kubeClient.AppsV1().Deployments(operatorNamespace).Get(ctx, operatorName, metav1.GetOptions{}) if err != nil { return haproxyVersionConfig{}, err } @@ -271,6 +293,11 @@ func getHAProxyVersionConfig(ctx context.Context, oc *exutil.CLI) (haproxyVersio // envvar not found and version not overridden, so this is pre 4.23/5.0, assume "2.8" defaultVersion = "2.8" } + if deprecatedVersion == "" { + // envvar/flag not configured (e.g. HyperShift's asset doesn't set it at all), + // so fall back to the operator's own compiled default. + deprecatedVersion = operatorv1.HAProxyVersion28 + } // Read available versions from Command. // The available versions are configured this way: From e14c8b724f5967a725e49741fbfa398fa3356166 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Thu, 3 Sep 2026 10:51:50 -0300 Subject: [PATCH 3/3] adding context and timeout from review --- test/extended/router/config_manager_ingress.go | 2 +- test/extended/router/haproxyversion_upgrade.go | 7 +++++-- test/extended/router/multi-haproxy.go | 6 +++--- test/extended/router/shard/shard.go | 8 ++++---- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/test/extended/router/config_manager_ingress.go b/test/extended/router/config_manager_ingress.go index e20790e62f2e..b342af23e235 100644 --- a/test/extended/router/config_manager_ingress.go +++ b/test/extended/router/config_manager_ingress.go @@ -146,7 +146,7 @@ var _ = g.Describe("[sig-network-edge][Feature:Router][apigroup:route.openshift. } // wait for the controller to be available - err = shard.WaitForIngressControllerCondition(oc, dcmIngressTimeout, controller, ingressControllerReady...) + err = shard.WaitForIngressControllerCondition(ctx, oc, dcmIngressTimeout, controller, ingressControllerReady...) o.Expect(err).NotTo(o.HaveOccurred()) // take the router pod, we need it to send requests to the router diff --git a/test/extended/router/haproxyversion_upgrade.go b/test/extended/router/haproxyversion_upgrade.go index 7ab429ec38e5..f0af891cff1b 100644 --- a/test/extended/router/haproxyversion_upgrade.go +++ b/test/extended/router/haproxyversion_upgrade.go @@ -70,7 +70,10 @@ func (h *HAProxyVersionUpgradeTest) Skip(upgctx upgrades.UpgradeContext) bool { return true } - ctx := context.Background() + // A 5-minute timeout avoids some of the calls hanging forever, + // which would impact all the other tests running in the same suite. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() oc := exutil.NewCLIForMonitorTest(h.Name() + "-skip").AsAdmin() hasField, err := apiHasHAProxyVersionField(ctx, oc) @@ -167,7 +170,7 @@ func (h *HAProxyVersionUpgradeTest) Test(ctx context.Context, f *framework.Frame g.By("Waiting for upgrade to complete") <-done - err := waitForIngressControllerReady(h.oc, h.ic) + err := waitForIngressControllerReady(ctx, h.oc, h.ic) o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("error waiting for IngressController %s to be ready", h.ic.String())) g.By("Validating HAProxy version after upgrade") diff --git a/test/extended/router/multi-haproxy.go b/test/extended/router/multi-haproxy.go index 53ed561381f8..3cc2f6f48e53 100644 --- a/test/extended/router/multi-haproxy.go +++ b/test/extended/router/multi-haproxy.go @@ -230,7 +230,7 @@ func (i *ingressControllers) createIngressController(ctx context.Context, oc *ex } i.items = append(i.items, &ictr) - return ingress, waitForIngressControllerReady(oc, controller) + return ingress, waitForIngressControllerReady(ctx, oc, controller) } func (i *ingressControllers) deleteAll(ctx context.Context, operatorClient operatorv1client.Interface) error { @@ -248,14 +248,14 @@ func (i *ingressControllers) deleteAll(ctx context.Context, operatorClient opera } // waitForIngressControllerReady waits for the provided IngressController to be ready. -func waitForIngressControllerReady(oc *exutil.CLI, ic types.NamespacedName) error { +func waitForIngressControllerReady(ctx context.Context, oc *exutil.CLI, ic types.NamespacedName) error { ingressControllerReady := []operatorv1.OperatorCondition{ {Type: operatorv1.IngressControllerAvailableConditionType, Status: operatorv1.ConditionTrue}, {Type: operatorv1.LoadBalancerManagedIngressConditionType, Status: operatorv1.ConditionFalse}, {Type: operatorv1.DNSManagedIngressConditionType, Status: operatorv1.ConditionFalse}, {Type: operatorv1.OperatorStatusTypeProgressing, Status: operatorv1.ConditionFalse}, } - return shard.WaitForIngressControllerCondition(oc, 5*time.Minute, ic, ingressControllerReady...) + return shard.WaitForIngressControllerCondition(ctx, oc, 5*time.Minute, ic, ingressControllerReady...) } // waitForIngressControllerDeletion waits for an IngressController to be removed. diff --git a/test/extended/router/shard/shard.go b/test/extended/router/shard/shard.go index 3ba2857b888b..7c1ad6dd86e9 100644 --- a/test/extended/router/shard/shard.go +++ b/test/extended/router/shard/shard.go @@ -69,7 +69,7 @@ func DeployNewRouterShard(oc *exutil.CLI, timeout time.Duration, cfg Config) (*o return nil, err } - return ingressCtrl, WaitForIngressControllerCondition(oc, timeout, types.NamespacedName{Namespace: ingressCtrl.Namespace, Name: ingressCtrl.Name}, ingressControllerNonDefaultAvailableConditions...) + return ingressCtrl, WaitForIngressControllerCondition(context.Background(), oc, timeout, types.NamespacedName{Namespace: ingressCtrl.Namespace, Name: ingressCtrl.Name}, ingressControllerNonDefaultAvailableConditions...) } func operatorConditionMap(conditions ...operatorv1.OperatorCondition) map[string]string { @@ -90,9 +90,9 @@ func conditionsMatchExpected(expected, actual map[string]string) bool { return reflect.DeepEqual(expected, filtered) } -func WaitForIngressControllerCondition(oc *exutil.CLI, timeout time.Duration, name types.NamespacedName, conditions ...operatorv1.OperatorCondition) error { - return wait.PollImmediate(3*time.Second, timeout, func() (bool, error) { - ic, err := oc.AdminOperatorClient().OperatorV1().IngressControllers(name.Namespace).Get(context.Background(), name.Name, metav1.GetOptions{}) +func WaitForIngressControllerCondition(ctx context.Context, oc *exutil.CLI, timeout time.Duration, name types.NamespacedName, conditions ...operatorv1.OperatorCondition) error { + return wait.PollUntilContextTimeout(ctx, 3*time.Second, timeout, true, func(ctx context.Context) (done bool, err error) { + ic, err := oc.AdminOperatorClient().OperatorV1().IngressControllers(name.Namespace).Get(ctx, name.Name, metav1.GetOptions{}) if err != nil { e2e.Logf("failed to get ingresscontroller %s/%s: %v, retrying...", name.Namespace, name.Name, err) return false, nil