From 3711535e995875c4a1802aef6c17989176ae0119 Mon Sep 17 00:00:00 2001 From: Jugal Shah Date: Fri, 19 Sep 2025 13:08:03 -0700 Subject: [PATCH 1/3] Get details of only declarative serve apps Signed-off-by: Jugal Shah --- .../dashboardclient/dashboard_httpclient.go | 18 ++- .../dashboard_httpclient_test.go | 127 +++++++++++++++++- 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go index ddd45aad08b..995985168ca 100644 --- a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go +++ b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "io" + "net/url" "net/http" "strings" @@ -19,6 +20,7 @@ import ( var ( // Multi-application URL paths ServeDetailsPath = "/api/serve/applications/" + APITypeParam = "declarative" DeployPathV2 = "/api/serve/applications/" // Job URL paths JobPath = "/api/jobs/" @@ -85,9 +87,19 @@ func (r *RayDashboardClient) GetMultiApplicationStatus(ctx context.Context) (map return r.ConvertServeDetailsToApplicationStatuses(serveDetails) } -// GetServeDetails gets details on all live applications on the Ray cluster. +// GetServeDetails gets details on all declarative applications on the Ray cluster. func (r *RayDashboardClient) GetServeDetails(ctx context.Context) (*utiltypes.ServeDetails, error) { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, r.dashboardURL+ServeDetailsPath, nil) + + serveDetailsURL, err := url.Parse(r.dashboardURL + ServeDetailsPath) + if err != nil { + return nil, fmt.Errorf("failed to parse dashboard URL: %w", err) + } + q := serveDetailsURL.Query() + q.Set("api_type", APITypeParam) + serveDetailsURL.RawQuery = q.Encode() + + req, err := http.NewRequestWithContext(ctx, "GET", serveDetailsURL.String(), nil) + if err != nil { return nil, err } @@ -352,4 +364,4 @@ func UnmarshalRuntimeEnvYAML(runtimeEnvYAML string) (utiltypes.RuntimeEnvType, e return nil, fmt.Errorf("failed to unmarshal RuntimeEnvYAML: %v: %w", runtimeEnvYAML, err) } return runtimeEnv, nil -} +} \ No newline at end of file diff --git a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go index e5022749781..653a7bf90eb 100644 --- a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go +++ b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go @@ -172,4 +172,129 @@ var _ = Describe("RayFrameworkGenerator", func() { err := rayDashboardClient.StopJob(context.TODO(), "stop-job-1") Expect(err).ToNot(HaveOccurred()) }) -}) + + It("Test GetServeDetails URL construction with query parameters", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Mock the response + expectedResponse := &utiltypes.ServeDetails{ + Applications: map[string]utiltypes.ServeApplicationDetails{ + "app1": { + ServeApplicationStatus: utiltypes.ServeApplicationStatus{ + Name: "app1", + Status: "RUNNING", + }, + Deployments: map[string]utiltypes.ServeDeploymentDetails{}, + }, + }, + DeployMode: "MULTI_APP", + } + responseBytes, _ := json.Marshal(expectedResponse) + + // Register responder that checks the URL includes the query parameter + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + func(req *http.Request) (*http.Response, error) { + // Verify the query parameter is present + Expect(req.URL.Query().Get("api_type")).To(Equal("declarative")) + return httpmock.NewBytesResponse(200, responseBytes), nil + }) + + details, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).ToNot(HaveOccurred()) + Expect(details.Applications).To(HaveKey("app1")) + Expect(details.DeployMode).To(Equal("MULTI_APP")) + }) + + It("Test GetServeDetails with invalid dashboard URL", func() { + invalidClient := &RayDashboardClient{ + dashboardURL: "://invalid-url", + client: &http.Client{}, + } + + _, err := invalidClient.GetServeDetails(context.TODO()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to parse dashboard URL")) + }) + + It("Test GetServeDetails with HTTP 400 Bad Request", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Test 400 Bad Request (might happen with older Ray versions) + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(400, "Bad Request: unknown parameter api_type"), nil + }) + + _, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("GetServeDetails fail: 400")) + Expect(err.Error()).To(ContainSubstring("Bad Request: unknown parameter api_type")) + }) + + It("Test GetServeDetails with HTTP 500 Internal Server Error", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(500, "Internal Server Error"), nil + }) + + _, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("GetServeDetails fail: 500")) + }) + + It("Test GetServeDetails with invalid JSON response", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(200, "invalid json response"), nil + }) + + _, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("GetServeDetails failed. Failed to unmarshal bytes")) + Expect(err.Error()).To(ContainSubstring("invalid json response")) + }) + + It("Test GetServeDetails with empty response", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Test with empty but valid JSON + emptyResponse := &utiltypes.ServeDetails{ + Applications: map[string]utiltypes.ServeApplicationDetails{}, + DeployMode: "", + } + responseBytes, _ := json.Marshal(emptyResponse) + + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + func(req *http.Request) (*http.Response, error) { + // Verify the query parameter is present + Expect(req.URL.Query().Get("api_type")).To(Equal("declarative")) + return httpmock.NewBytesResponse(200, responseBytes), nil + }) + + details, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).ToNot(HaveOccurred()) + Expect(details.Applications).To(BeEmpty()) + Expect(details.DeployMode).To(Equal("")) + }) + + It("Test GetServeDetails with network error", func() { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, + httpmock.NewErrorResponder(context.DeadlineExceeded)) + + _, err := rayDashboardClient.GetServeDetails(context.TODO()) + Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(context.DeadlineExceeded)) + }) +}) \ No newline at end of file From caa1b711a3c5288dcb3ddd12309983023245a8a1 Mon Sep 17 00:00:00 2001 From: Jugal Shah Date: Mon, 22 Sep 2025 14:33:37 -0700 Subject: [PATCH 2/3] Fix CI errors Signed-off-by: Jugal Shah --- .../ray/utils/dashboardclient/dashboard_httpclient.go | 8 +++----- .../utils/dashboardclient/dashboard_httpclient_test.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go index 995985168ca..d360ebc0af9 100644 --- a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go +++ b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient.go @@ -5,8 +5,8 @@ import ( "context" "fmt" "io" - "net/url" "net/http" + "net/url" "strings" "k8s.io/apimachinery/pkg/api/errors" @@ -20,7 +20,7 @@ import ( var ( // Multi-application URL paths ServeDetailsPath = "/api/serve/applications/" - APITypeParam = "declarative" + APITypeParam = "declarative" DeployPathV2 = "/api/serve/applications/" // Job URL paths JobPath = "/api/jobs/" @@ -89,7 +89,6 @@ func (r *RayDashboardClient) GetMultiApplicationStatus(ctx context.Context) (map // GetServeDetails gets details on all declarative applications on the Ray cluster. func (r *RayDashboardClient) GetServeDetails(ctx context.Context) (*utiltypes.ServeDetails, error) { - serveDetailsURL, err := url.Parse(r.dashboardURL + ServeDetailsPath) if err != nil { return nil, fmt.Errorf("failed to parse dashboard URL: %w", err) @@ -99,7 +98,6 @@ func (r *RayDashboardClient) GetServeDetails(ctx context.Context) (*utiltypes.Se serveDetailsURL.RawQuery = q.Encode() req, err := http.NewRequestWithContext(ctx, "GET", serveDetailsURL.String(), nil) - if err != nil { return nil, err } @@ -364,4 +362,4 @@ func UnmarshalRuntimeEnvYAML(runtimeEnvYAML string) (utiltypes.RuntimeEnvType, e return nil, fmt.Errorf("failed to unmarshal RuntimeEnvYAML: %v: %w", runtimeEnvYAML, err) } return runtimeEnv, nil -} \ No newline at end of file +} diff --git a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go index 653a7bf90eb..24598a68993 100644 --- a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go +++ b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go @@ -297,4 +297,4 @@ var _ = Describe("RayFrameworkGenerator", func() { Expect(err).To(HaveOccurred()) Expect(err).To(Equal(context.DeadlineExceeded)) }) -}) \ No newline at end of file +}) From 893958737d8024b92658cfb6e302e8dac3d67434 Mon Sep 17 00:00:00 2001 From: Jugal Shah Date: Fri, 26 Sep 2025 14:58:51 -0700 Subject: [PATCH 3/3] Fix linting in dashboard_httpclient_test.go Signed-off-by: Jugal Shah --- .../ray/utils/dashboardclient/dashboard_httpclient_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go index 24598a68993..1bdb3a1fb75 100644 --- a/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go +++ b/ray-operator/controllers/ray/utils/dashboardclient/dashboard_httpclient_test.go @@ -223,7 +223,7 @@ var _ = Describe("RayFrameworkGenerator", func() { // Test 400 Bad Request (might happen with older Ray versions) httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, - func(req *http.Request) (*http.Response, error) { + func(_ *http.Request) (*http.Response, error) { return httpmock.NewStringResponse(400, "Bad Request: unknown parameter api_type"), nil }) @@ -238,7 +238,7 @@ var _ = Describe("RayFrameworkGenerator", func() { defer httpmock.DeactivateAndReset() httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, - func(req *http.Request) (*http.Response, error) { + func(_ *http.Request) (*http.Response, error) { return httpmock.NewStringResponse(500, "Internal Server Error"), nil }) @@ -252,7 +252,7 @@ var _ = Describe("RayFrameworkGenerator", func() { defer httpmock.DeactivateAndReset() httpmock.RegisterResponder(http.MethodGet, rayDashboardClient.dashboardURL+ServeDetailsPath, - func(req *http.Request) (*http.Response, error) { + func(_ *http.Request) (*http.Response, error) { return httpmock.NewStringResponse(200, "invalid json response"), nil })