[history server][e2e] Remove duplicate test support functions

Refactor ApplyHistoryServer to accept an optional manifest path parameter,
eliminating the need for a separate ApplyAzureHistoryServer function.
Remove PrepareAzureHistoryServerTestEnv as it was identical to
PrepareAzureBlobTestEnv.

Author: ikchifo

historyserver/test/e2e/historyserver_azureblob_test.go

@@ -36,9 +36,9 @@ func TestAzureHistoryServer(t *testing.T) {
 }
 
 func testAzureLiveClusters(test Test, g *WithT, namespace *corev1.Namespace, azureClient *azblob.Client) {
-	rayCluster := PrepareAzureHistoryServerTestEnv(test, g, namespace, azureClient)
+	rayCluster := PrepareAzureBlobTestEnv(test, g, namespace, azureClient)
 	ApplyRayJobAndWaitForCompletion(test, g, namespace, rayCluster)
-	ApplyAzureHistoryServer(test, g, namespace)
+	ApplyHistoryServer(test, g, namespace, AzureHistoryServerManifestPath)
 	historyServerURL := GetHistoryServerURL(test, g, namespace)
 
 	clusterInfo := getClusterFromList(test, g, historyServerURL, rayCluster.Name, namespace.Name)

historyserver/test/e2e/historyserver_test.go

@@ -46,7 +46,7 @@ func TestHistoryServer(t *testing.T) {
 func testLiveClusters(test Test, g *WithT, namespace *corev1.Namespace, s3Client *s3.S3) {
 	rayCluster := PrepareTestEnv(test, g, namespace, s3Client)
 	ApplyRayJobAndWaitForCompletion(test, g, namespace, rayCluster)
-	ApplyHistoryServer(test, g, namespace)
+	ApplyHistoryServer(test, g, namespace, "")
 	historyServerURL := GetHistoryServerURL(test, g, namespace)
 
 	clusterInfo := getClusterFromList(test, g, historyServerURL, rayCluster.Name, namespace.Name)

historyserver/test/support/azureblob.go

@@ -8,7 +8,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
@@ -157,58 +156,3 @@ func ApplyAzureRayClusterWithCollector(test Test, g *WithT, namespace *corev1.Na
 	return rayCluster
 }
 
-// ApplyAzureHistoryServer deploys the HistoryServer configured for Azure Blob Storage.
-func ApplyAzureHistoryServer(test Test, g *WithT, namespace *corev1.Namespace) {
-	sa, clusterRole, clusterRoleBinding := DeserializeRBACFromYAML(test, ServiceAccountManifestPath)
-	sa.Namespace = namespace.Name
-	clusterRoleBinding.Name = fmt.Sprintf("historyserver-%s", namespace.Name)
-	clusterRoleBinding.Subjects[0].Namespace = namespace.Name
-
-	_, err := test.Client().Core().CoreV1().ServiceAccounts(namespace.Name).Create(test.Ctx(), sa, metav1.CreateOptions{})
-	g.Expect(err).NotTo(HaveOccurred())
-
-	_, err = test.Client().Core().RbacV1().ClusterRoles().Create(test.Ctx(), clusterRole, metav1.CreateOptions{})
-	if err != nil && !k8serrors.IsAlreadyExists(err) {
-		g.Expect(err).NotTo(HaveOccurred())
-	}
-
-	_, err = test.Client().Core().RbacV1().ClusterRoleBindings().Create(test.Ctx(), clusterRoleBinding, metav1.CreateOptions{})
-	g.Expect(err).NotTo(HaveOccurred())
-
-	test.T().Cleanup(func() {
-		_ = test.Client().Core().RbacV1().ClusterRoleBindings().Delete(
-			context.Background(), clusterRoleBinding.Name, metav1.DeleteOptions{})
-	})
-
-	KubectlApplyYAML(test, AzureHistoryServerManifestPath, namespace.Name)
-
-	LogWithTimestamp(test.T(), "Waiting for Azure HistoryServer to be ready")
-	g.Eventually(func(gg Gomega) {
-		pods, err := test.Client().Core().CoreV1().Pods(namespace.Name).List(
-			test.Ctx(), metav1.ListOptions{
-				LabelSelector: "app=historyserver",
-			},
-		)
-		gg.Expect(err).NotTo(HaveOccurred())
-		gg.Expect(pods.Items).NotTo(BeEmpty())
-		gg.Expect(AllPodsRunningAndReady(pods.Items)).To(BeTrue())
-	}, TestTimeoutMedium).Should(Succeed())
-	LogWithTimestamp(test.T(), "Azure HistoryServer is ready")
-}
-
-// PrepareAzureHistoryServerTestEnv prepares test environment for Azure History Server tests.
-func PrepareAzureHistoryServerTestEnv(test Test, g *WithT, namespace *corev1.Namespace, azureClient *azblob.Client) *rayv1.RayCluster {
-	rayCluster := ApplyAzureRayClusterWithCollector(test, g, namespace)
-
-	headPod, err := GetHeadPod(test, rayCluster)
-	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(headPod.Spec.Containers).To(ContainElement(
-		WithTransform(func(c corev1.Container) string { return c.Name }, Equal("collector")),
-	))
-
-	containerClient := azureClient.ServiceClient().NewContainerClient(AzureContainerName)
-	_, err = containerClient.GetProperties(context.Background(), nil)
-	g.Expect(err).NotTo(HaveOccurred())
-
-	return rayCluster
-}

historyserver/test/support/historyserver.go

@@ -49,32 +49,34 @@ var HistoryServerEndpoints = []string{
 }
 
 // ApplyHistoryServer deploys the HistoryServer and RBAC resources.
-func ApplyHistoryServer(test Test, g *WithT, namespace *corev1.Namespace) {
-	// Read RBAC resources from YAML and modify namespace fields.
+// If manifestPath is empty, the default HistoryServerManifestPath is used.
+func ApplyHistoryServer(test Test, g *WithT, namespace *corev1.Namespace, manifestPath string) {
+	if manifestPath == "" {
+		manifestPath = HistoryServerManifestPath
+	}
+
 	sa, clusterRole, clusterRoleBinding := DeserializeRBACFromYAML(test, ServiceAccountManifestPath)
 	sa.Namespace = namespace.Name
 	clusterRoleBinding.Name = fmt.Sprintf("historyserver-%s", namespace.Name)
 	clusterRoleBinding.Subjects[0].Namespace = namespace.Name
 
-	// Create RBAC resources
 	_, err := test.Client().Core().CoreV1().ServiceAccounts(namespace.Name).Create(test.Ctx(), sa, metav1.CreateOptions{})
 	g.Expect(err).NotTo(HaveOccurred())
-	// ClusterRole is shared across tests, ignore if already exists.
+
 	_, err = test.Client().Core().RbacV1().ClusterRoles().Create(test.Ctx(), clusterRole, metav1.CreateOptions{})
 	if err != nil && !k8serrors.IsAlreadyExists(err) {
 		g.Expect(err).NotTo(HaveOccurred())
 	}
+
 	_, err = test.Client().Core().RbacV1().ClusterRoleBindings().Create(test.Ctx(), clusterRoleBinding, metav1.CreateOptions{})
 	g.Expect(err).NotTo(HaveOccurred())
 
-	// ClusterRoleBinding is cluster-scoped and won't be deleted when the namespace is cleaned up.
-	// Register cleanup to prevent accumulation across test runs.
 	test.T().Cleanup(func() {
 		_ = test.Client().Core().RbacV1().ClusterRoleBindings().Delete(
 			context.Background(), clusterRoleBinding.Name, metav1.DeleteOptions{})
 	})
 
-	KubectlApplyYAML(test, HistoryServerManifestPath, namespace.Name)
+	KubectlApplyYAML(test, manifestPath, namespace.Name)
 
 	LogWithTimestamp(test.T(), "Waiting for HistoryServer to be ready")
 	g.Eventually(func(gg Gomega) {