From 1ecc6c8b75041a875ac07e8ad614b0b8b54fad68 Mon Sep 17 00:00:00 2001 From: wangke19 Date: Mon, 13 Oct 2025 13:51:55 +0800 Subject: [PATCH 1/3] fix(disruption): Using correct internal LB of apiserver for monitor test on ARO and Baremetal Hypershift --- .../monitortest.go | 108 ++++++++++++++++-- test/extended/util/managed_services.go | 38 ++++++ 2 files changed, 137 insertions(+), 9 deletions(-) diff --git a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go index 6bb241f56287..b5e6607d7b1c 100644 --- a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go +++ b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go @@ -75,8 +75,10 @@ type InvariantInClusterDisruption struct { notSupportedReason string replicas int32 controlPlaneNodes int32 - - adminRESTConfig *rest.Config + isHypershift bool + isAROHCPCluster bool + isBareMetalHypershift bool + adminRESTConfig *rest.Config kubeClient kubernetes.Interface } @@ -86,6 +88,68 @@ func NewInvariantInClusterDisruption(info monitortestframework.MonitorTestInitia } } +// parseAdminRESTConfigHost parses the adminRESTConfig.Host URL and returns hostname and port +func (i *InvariantInClusterDisruption) parseAdminRESTConfigHost() (hostname, port string, err error) { + parsedURL, err := url.Parse(i.adminRESTConfig.Host) + if err != nil { + return "", "", fmt.Errorf("failed to parse adminRESTConfig.Host %q: %v", i.adminRESTConfig.Host, err) + } + + hostname = parsedURL.Hostname() + if hostname == "" { + return "", "", fmt.Errorf("no hostname found in adminRESTConfig.Host %q", i.adminRESTConfig.Host) + } + + port = parsedURL.Port() + if port == "" { + port = "6443" // default port + } + + return hostname, port, nil +} + +// setKubernetesServiceEnvVars sets the KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT environment variables +// based on the cluster type (ARO HCP, bare metal HyperShift, or standard) +func (i *InvariantInClusterDisruption) setKubernetesServiceEnvVars(envVars []corev1.EnvVar, apiIntHost, apiIntPort string) []corev1.EnvVar { + // Parse adminRESTConfig.Host once for bare metal HyperShift + var bareMetalHost, bareMetalPort string + var bareMetalErr error + if i.isHypershift && i.isBareMetalHypershift { + bareMetalHost, bareMetalPort, bareMetalErr = i.parseAdminRESTConfigHost() + if bareMetalErr != nil { + logrus.WithError(bareMetalErr).Errorf("Failed to parse adminRESTConfig.Host for bare metal HyperShift") + } + } + + for j, env := range envVars { + switch env.Name { + case "KUBERNETES_SERVICE_HOST": + if i.isHypershift && i.isBareMetalHypershift { + if bareMetalErr != nil { + envVars[j].Value = apiIntHost + } else { + envVars[j].Value = bareMetalHost + } + } else { + envVars[j].Value = apiIntHost + } + case "KUBERNETES_SERVICE_PORT": + if i.isHypershift && i.isAROHCPCluster { + envVars[j].Value = "7443" + } else if i.isHypershift && i.isBareMetalHypershift { + if bareMetalErr != nil { + envVars[j].Value = apiIntPort + } else { + envVars[j].Value = bareMetalPort + } + } else { + envVars[j].Value = apiIntPort + } + } + } + return envVars +} + func (i *InvariantInClusterDisruption) createDeploymentAndWaitToRollout(ctx context.Context, deploymentObj *appsv1.Deployment) error { deploymentID := uuid.New().String() deploymentObj = disruptionlibrary.UpdateDeploymentENVs(deploymentObj, deploymentID, "") @@ -114,15 +178,18 @@ func (i *InvariantInClusterDisruption) createDeploymentAndWaitToRollout(ctx cont return nil } -func (i *InvariantInClusterDisruption) createInternalLBDeployment(ctx context.Context, apiIntHost string) error { +func (i *InvariantInClusterDisruption) createInternalLBDeployment(ctx context.Context, apiIntHost, apiIntPort string) error { deploymentObj := resourceread.ReadDeploymentV1OrDie(internalLBDeploymentYaml) deploymentObj.SetNamespace(i.namespaceName) - deploymentObj.Spec.Template.Spec.Containers[0].Env[0].Value = apiIntHost // set amount of deployment replicas to make sure it runs on all nodes deploymentObj.Spec.Replicas = &i.replicas // we need to use the openshift-tests image of the destination during an upgrade. deploymentObj.Spec.Template.Spec.Containers[0].Image = i.openshiftTestsImagePullSpec + // Set the correct host and port for internal API server based on cluster type + deploymentObj.Spec.Template.Spec.Containers[0].Env = i.setKubernetesServiceEnvVars( + deploymentObj.Spec.Template.Spec.Containers[0].Env, apiIntHost, apiIntPort) + err := i.createDeploymentAndWaitToRollout(ctx, deploymentObj) if err != nil { return err @@ -321,6 +388,7 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi oc := exutil.NewCLI("apiserver-incluster-availability").AsAdmin() var isAROHCPcluster bool isHypershift, _ := exutil.IsHypershift(ctx, oc.AdminConfigClient()) + i.isHypershift = isHypershift if isHypershift { _, hcpNamespace, err := exutil.GetHypershiftManagementClusterConfigAndNamespace() if err != nil { @@ -336,6 +404,14 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi i.notSupportedReason = "platform Hypershift - ARO HCP not supported" return nil } + i.isAROHCPCluster = isAROHCPcluster + + // Check if this is a bare metal HyperShift cluster + i.isBareMetalHypershift, err = exutil.IsBareMetalHyperShiftCluster(ctx, managementOC) + if err != nil { + logrus.WithError(err).Warning("Failed to check if bare metal HyperShift, assuming it's not") + i.isBareMetalHypershift = false // Assume not bare metal HyperShift on error + } } // runImageExtract extracts src from specified image to dst @@ -379,11 +455,25 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi return fmt.Errorf("error getting openshift infrastructure: %v", err) } - internalAPI, err := url.Parse(infra.Status.APIServerInternalURL) - if err != nil { - return fmt.Errorf("error parsing api int url: %v", err) + var apiIntHost string + var apiIntPort string + if i.isHypershift { + apiIntHost, apiIntPort, err = i.parseAdminRESTConfigHost() + if err != nil { + return fmt.Errorf("failed to parse adminRESTConfig.Host: %v", err) + } + } else { + internalAPI, err := url.Parse(infra.Status.APIServerInternalURL) + if err != nil { + return fmt.Errorf("error parsing api int url: %v", err) + } + apiIntHost = internalAPI.Hostname() + if internalAPI.Port() != "" { + apiIntPort = internalAPI.Port() + } else { + apiIntPort = "6443" // default port + } } - apiIntHost := internalAPI.Hostname() allNodes, err := i.kubeClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { @@ -439,7 +529,7 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi if err != nil { return fmt.Errorf("error creating localhost: %v", err) } - err = i.createInternalLBDeployment(ctx, apiIntHost) + err = i.createInternalLBDeployment(ctx, apiIntHost, apiIntPort) if err != nil { return fmt.Errorf("error creating internal LB: %v", err) } diff --git a/test/extended/util/managed_services.go b/test/extended/util/managed_services.go index 37e08171b85c..89b630a4eee8 100644 --- a/test/extended/util/managed_services.go +++ b/test/extended/util/managed_services.go @@ -2,6 +2,8 @@ package util import ( "context" + "fmt" + "strings" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -95,3 +97,39 @@ func IsAroHCP(ctx context.Context, namespace string, kubeClient kubernetes.Inter logrus.Infof("No deployment found with control-plane-operator container in namespace %s", namespace) return false, nil } + +// IsBareMetalHyperShiftCluster checks if the HyperShift cluster is running on bare metal +// by checking the platform type of the hosted cluster. It uses kubectl commands to query +// the hosted cluster's platform type and returns true if it's "None" or "Agent". +func IsBareMetalHyperShiftCluster(ctx context.Context, managementOC *CLI) (bool, error) { + // Get the hosted cluster namespace + _, hcpNamespace, err := GetHypershiftManagementClusterConfigAndNamespace() + if err != nil { + return false, fmt.Errorf("failed to get hypershift management cluster config and namespace: %v", err) + } + + // Get the first hosted cluster name + clusterNames, err := managementOC.AsAdmin().WithoutNamespace().Run("get").Args( + "-n", hcpNamespace, "hostedclusters", "-o=jsonpath={.items[*].metadata.name}").Output() + if err != nil { + return false, fmt.Errorf("failed to get hosted cluster names: %v", err) + } + + if len(clusterNames) == 0 { + return false, fmt.Errorf("no hosted clusters found") + } + + // Get the first hosted cluster name + clusterName := strings.Split(strings.TrimSpace(clusterNames), " ")[0] + + // Get the platform type of the hosted cluster + platformType, err := managementOC.AsAdmin().WithoutNamespace().Run("get").Args( + "hostedcluster", clusterName, "-n", hcpNamespace, `-ojsonpath={.spec.platform.type}`).Output() + if err != nil { + return false, fmt.Errorf("failed to get hosted cluster platform type: %v", err) + } + + // Check if it's bare metal (None or Agent platform) + platformTypeStr := strings.TrimSpace(platformType) + return platformTypeStr == "None" || platformTypeStr == "Agent", nil +} From e1bd2a153de59ce20e6e473ba34e9f3340109821 Mon Sep 17 00:00:00 2001 From: wangke19 Date: Thu, 16 Oct 2025 12:35:56 +0800 Subject: [PATCH 2/3] make update-gofmt --- .../kubeapiserver/disruptioninclusterapiserver/monitortest.go | 2 +- test/extended/util/managed_services.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go index b5e6607d7b1c..c025b65245ee 100644 --- a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go +++ b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go @@ -79,7 +79,7 @@ type InvariantInClusterDisruption struct { isAROHCPCluster bool isBareMetalHypershift bool adminRESTConfig *rest.Config - kubeClient kubernetes.Interface + kubeClient kubernetes.Interface } func NewInvariantInClusterDisruption(info monitortestframework.MonitorTestInitializationInfo) monitortestframework.MonitorTest { diff --git a/test/extended/util/managed_services.go b/test/extended/util/managed_services.go index 89b630a4eee8..0bdda1a2242b 100644 --- a/test/extended/util/managed_services.go +++ b/test/extended/util/managed_services.go @@ -114,14 +114,14 @@ func IsBareMetalHyperShiftCluster(ctx context.Context, managementOC *CLI) (bool, if err != nil { return false, fmt.Errorf("failed to get hosted cluster names: %v", err) } - + if len(clusterNames) == 0 { return false, fmt.Errorf("no hosted clusters found") } // Get the first hosted cluster name clusterName := strings.Split(strings.TrimSpace(clusterNames), " ")[0] - + // Get the platform type of the hosted cluster platformType, err := managementOC.AsAdmin().WithoutNamespace().Run("get").Args( "hostedcluster", clusterName, "-n", hcpNamespace, `-ojsonpath={.spec.platform.type}`).Output() From 17013e816065463e9411688734e637d9f3f44f28 Mon Sep 17 00:00:00 2001 From: wangke19 Date: Mon, 27 Oct 2025 11:02:31 +0800 Subject: [PATCH 3/3] refactor(disruption): Replace multiple boolean flags with HostedClusterType enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced three boolean flags (isHypershift, isAROHCPCluster, isBareMetalHypershift) with a single HostedClusterType enum field for better code maintainability and extensibility. Also added a comment explaining why ARO HCP uses port 7443. Changes: - Introduced HostedClusterType enum with values: Standalone, AROHCP, BareMetal, Other - Replaced three boolean fields with single hostedClusterType field - Updated all conditional logic to use the enum pattern - Added comment for ARO HCP port 7443 usage - Improved semantic clarity by aligning with HyperShift terminology This refactoring makes it easier to add new hosted cluster types in the future and prevents contradictory states from occurring. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../monitortest.go | 63 +++++++++++++------ 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go index c025b65245ee..4ac00564183b 100644 --- a/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go +++ b/pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/monitortest.go @@ -68,6 +68,20 @@ var ( rbacMonitorCRBName string ) +// HostedClusterType represents the type of cluster hosting model +type HostedClusterType string + +const ( + // HostedClusterTypeStandalone represents a standard OpenShift cluster with self-hosted control plane + HostedClusterTypeStandalone HostedClusterType = "Standalone" + // HostedClusterTypeAROHCP represents an ARO HCP (Azure Red Hat OpenShift Hosted Control Plane) cluster + HostedClusterTypeAROHCP HostedClusterType = "AROHCP" + // HostedClusterTypeBareMetal represents a bare metal HyperShift hosted cluster + HostedClusterTypeBareMetal HostedClusterType = "BareMetal" + // HostedClusterTypeOther represents other HyperShift hosted cluster types + HostedClusterTypeOther HostedClusterType = "Other" +) + type InvariantInClusterDisruption struct { namespaceName string openshiftTestsImagePullSpec string @@ -75,9 +89,7 @@ type InvariantInClusterDisruption struct { notSupportedReason string replicas int32 controlPlaneNodes int32 - isHypershift bool - isAROHCPCluster bool - isBareMetalHypershift bool + hostedClusterType HostedClusterType adminRESTConfig *rest.Config kubeClient kubernetes.Interface } @@ -109,12 +121,12 @@ func (i *InvariantInClusterDisruption) parseAdminRESTConfigHost() (hostname, por } // setKubernetesServiceEnvVars sets the KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT environment variables -// based on the cluster type (ARO HCP, bare metal HyperShift, or standard) +// based on the hosted cluster type func (i *InvariantInClusterDisruption) setKubernetesServiceEnvVars(envVars []corev1.EnvVar, apiIntHost, apiIntPort string) []corev1.EnvVar { // Parse adminRESTConfig.Host once for bare metal HyperShift var bareMetalHost, bareMetalPort string var bareMetalErr error - if i.isHypershift && i.isBareMetalHypershift { + if i.hostedClusterType == HostedClusterTypeBareMetal { bareMetalHost, bareMetalPort, bareMetalErr = i.parseAdminRESTConfigHost() if bareMetalErr != nil { logrus.WithError(bareMetalErr).Errorf("Failed to parse adminRESTConfig.Host for bare metal HyperShift") @@ -124,7 +136,7 @@ func (i *InvariantInClusterDisruption) setKubernetesServiceEnvVars(envVars []cor for j, env := range envVars { switch env.Name { case "KUBERNETES_SERVICE_HOST": - if i.isHypershift && i.isBareMetalHypershift { + if i.hostedClusterType == HostedClusterTypeBareMetal { if bareMetalErr != nil { envVars[j].Value = apiIntHost } else { @@ -134,9 +146,10 @@ func (i *InvariantInClusterDisruption) setKubernetesServiceEnvVars(envVars []cor envVars[j].Value = apiIntHost } case "KUBERNETES_SERVICE_PORT": - if i.isHypershift && i.isAROHCPCluster { + if i.hostedClusterType == HostedClusterTypeAROHCP { + // ARO HCP uses port 7443 for the internal API server load balancer envVars[j].Value = "7443" - } else if i.isHypershift && i.isBareMetalHypershift { + } else if i.hostedClusterType == HostedClusterTypeBareMetal { if bareMetalErr != nil { envVars[j].Value = apiIntPort } else { @@ -384,11 +397,11 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi i.payloadImagePullSpec = clusterVersion.Status.History[0].Image } - // Check for ARO HCP and skip if detected + // Determine hosted cluster type oc := exutil.NewCLI("apiserver-incluster-availability").AsAdmin() - var isAROHCPcluster bool + i.hostedClusterType = HostedClusterTypeStandalone // Default to standalone + isHypershift, _ := exutil.IsHypershift(ctx, oc.AdminConfigClient()) - i.isHypershift = isHypershift if isHypershift { _, hcpNamespace, err := exutil.GetHypershiftManagementClusterConfigAndNamespace() if err != nil { @@ -398,19 +411,28 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi // For Hypershift, only skip if it's specifically ARO HCP // Use management cluster client to check the control-plane-operator deployment managementOC := exutil.NewHypershiftManagementCLI(hcpNamespace) + var isAROHCPcluster bool if isAROHCPcluster, err = exutil.IsAroHCP(ctx, hcpNamespace, managementOC.AdminKubeClient()); err != nil { logrus.WithError(err).Warning("Failed to check if ARO HCP, assuming it's not") } else if isAROHCPcluster { i.notSupportedReason = "platform Hypershift - ARO HCP not supported" return nil } - i.isAROHCPCluster = isAROHCPcluster - // Check if this is a bare metal HyperShift cluster - i.isBareMetalHypershift, err = exutil.IsBareMetalHyperShiftCluster(ctx, managementOC) - if err != nil { - logrus.WithError(err).Warning("Failed to check if bare metal HyperShift, assuming it's not") - i.isBareMetalHypershift = false // Assume not bare metal HyperShift on error + // Determine the specific HyperShift variant + if isAROHCPcluster { + i.hostedClusterType = HostedClusterTypeAROHCP + } else { + // Check if this is a bare metal HyperShift cluster + isBareMetalHypershift, err := exutil.IsBareMetalHyperShiftCluster(ctx, managementOC) + if err != nil { + logrus.WithError(err).Warning("Failed to check if bare metal HyperShift, assuming other HyperShift type") + i.hostedClusterType = HostedClusterTypeOther + } else if isBareMetalHypershift { + i.hostedClusterType = HostedClusterTypeBareMetal + } else { + i.hostedClusterType = HostedClusterTypeOther + } } } @@ -457,7 +479,12 @@ func (i *InvariantInClusterDisruption) StartCollection(ctx context.Context, admi var apiIntHost string var apiIntPort string - if i.isHypershift { + // Hosted clusters use adminRESTConfig.Host, standalone clusters use APIServerInternalURL + isHostedCluster := i.hostedClusterType == HostedClusterTypeAROHCP || + i.hostedClusterType == HostedClusterTypeBareMetal || + i.hostedClusterType == HostedClusterTypeOther + + if isHostedCluster { apiIntHost, apiIntPort, err = i.parseAdminRESTConfigHost() if err != nil { return fmt.Errorf("failed to parse adminRESTConfig.Host: %v", err)