From d77892188fd88843176e971f095e17ed5e0878fb Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:45:21 -0700 Subject: [PATCH 1/8] fix(rest-api): distinguish a lost proxy result from a closed execution A DeadlineExceeded in the workflow result was read as proof the caller had stopped waiting, but it can also come from inside a live execution. Only wfCtx carries that evidence, and a Temporal timeout is the stronger signal when both hold, so it is classified first. Neither case terminates the execution: the activity does not heartbeat, so cancellation cannot reach an in-flight RPC, and freeing a deterministic ID would let a retry start a duplicate mutation instead of attaching through USE_EXISTING. A caller that goes away during the start now reports 504 rather than 500. Signed-off-by: Kun Zhao --- .../pkg/api/handler/util/common/grpcproxy.go | 55 +++++++- .../api/handler/util/common/grpcproxy_test.go | 121 +++++++++++++++++- 2 files changed, 167 insertions(+), 9 deletions(-) diff --git a/rest-api/api/pkg/api/handler/util/common/grpcproxy.go b/rest-api/api/pkg/api/handler/util/common/grpcproxy.go index 88dc8b0bc8..6943b5510b 100644 --- a/rest-api/api/pkg/api/handler/util/common/grpcproxy.go +++ b/rest-api/api/pkg/api/handler/util/common/grpcproxy.go @@ -13,6 +13,7 @@ import ( "github.com/google/uuid" temporalEnums "go.temporal.io/api/enums/v1" + "go.temporal.io/api/serviceerror" tclient "go.temporal.io/sdk/client" tp "go.temporal.io/sdk/temporal" "google.golang.org/protobuf/encoding/protojson" @@ -62,9 +63,9 @@ func ExecuteCoreGRPC( // Naming secretFields requires a non-empty secretKey; the site has no other way // to recover the redacted values. // -// On timeout it returns StatusGatewayTimeout; handlers that previously -// terminated the workflow on timeout should call TerminateWorkflowOnTimeOut -// with the same workflowID. +// On timeout it returns StatusGatewayTimeout. Do not follow that with +// TerminateWorkflowOnTimeOut; see executeGRPCProxy for why the proxy leaves the +// execution alone. func ExecuteFlowGRPC( ctx context.Context, stc tclient.Client, @@ -79,9 +80,27 @@ func ExecuteFlowGRPC( return executeGRPCProxy(ctx, stc, grpcproxy.Flow, fullMethod, req, resp, workflowID, conflictPolicy, secretKey, secretFields...) } +// proxyTimedOutError reports that no result arrived in time. The client-facing +// message is the same however the request ran out of time, because the client +// learns the same thing either way; detail is what carries the distinction into +// the log, and it matters because only an execution timeout proves the +// execution has stopped. +func proxyTimedOutError(backend grpcproxy.Backend, fullMethod, detail string, cause error) *cutil.APIError { + return cutil.NewAPIError(http.StatusGatewayTimeout, fmt.Sprintf("%s proxy request timed out", backend.Label), fmt.Errorf("%s proxy %s %s: %w", strings.ToLower(backend.Label), fullMethod, detail, cause)) +} + // executeGRPCProxy starts one proxy workflow and translates its result. Both // backends share it; they differ only in the backend descriptor and in who // chooses the workflow ID and conflict policy. +// +// It never terminates the execution, including when the caller stops waiting. +// The activity does not heartbeat, so Temporal cannot deliver cancellation while +// its backend RPC is in progress, and terminating only the workflow would +// discard the result without stopping that RPC. For the Flow callers that pass a +// deterministic ID with USE_EXISTING it would also free that ID, so a retried +// request would start a duplicate mutation instead of attaching to the call +// already in flight. An abandoned execution stays bounded by +// grpcproxy.WorkflowExecutionTimeout and grpcproxy.ActivityStartToCloseTimeout. func executeGRPCProxy( ctx context.Context, stc tclient.Client, @@ -139,16 +158,42 @@ func executeGRPCProxy( EncryptedSecrets: encryptedSecrets, }) if err != nil { + // Either way the start may have reached the server before the wait + // ended, so these are timeouts rather than failures to dispatch. The + // second is not the caller giving up: the SDK caps a start at ten + // seconds regardless of how long the caller is prepared to wait, and + // never retries that deadline, so a slow frontend surfaces here with + // the caller still waiting. + if wfCtx.Err() != nil { + return proxyTimedOutError(backend, fullMethod, "caller stopped waiting", wfCtx.Err()) + } + var startDeadlineErr *serviceerror.DeadlineExceeded + if errors.As(err, &startDeadlineErr) { + return proxyTimedOutError(backend, fullMethod, "start deadline exceeded", err) + } return cutil.NewAPIError(http.StatusInternalServerError, fmt.Sprintf("Failed to execute %s proxy workflow", backend.Label), fmt.Errorf("execute %s workflow: %w", backend.WorkflowName, err)) } var out grpcproxy.Response resultErr := we.Get(wfCtx, &out) if resultErr != nil { + // A Temporal timeout is positive evidence that the execution closed, so + // it is checked first: when the caller also went away, having closed is + // the more useful fact of the two. var timeoutErr *tp.TimeoutError - if errors.As(resultErr, &timeoutErr) || errors.Is(resultErr, context.DeadlineExceeded) || wfCtx.Err() != nil { - return cutil.NewAPIError(http.StatusGatewayTimeout, fmt.Sprintf("%s proxy request timed out", backend.Label), fmt.Errorf("%s proxy %s timed out: %w", strings.ToLower(backend.Label), fullMethod, resultErr)) + if errors.As(resultErr, &timeoutErr) { + return proxyTimedOutError(backend, fullMethod, "execution timed out", resultErr) + } + + // Only wfCtx proves the caller stopped waiting. A DeadlineExceeded in + // resultErr alone does not: it can come from inside the execution, and + // treating it as the caller's would misreport who gave up. Unlike the + // start, the wait for a result runs to wfCtx's deadline, so there is no + // shorter SDK deadline to distinguish from it here. + if wfCtx.Err() != nil { + return proxyTimedOutError(backend, fullMethod, "caller stopped waiting", wfCtx.Err()) } + code, werr := UnwrapWorkflowError(resultErr) return cutil.NewAPIError(code, werr.Error(), nil) } diff --git a/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go b/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go index 05a7a7e1b2..0b620275e6 100644 --- a/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go +++ b/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go @@ -5,6 +5,7 @@ package common import ( "context" + "errors" "net/http" "strings" "testing" @@ -13,8 +14,10 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" temporalEnums "go.temporal.io/api/enums/v1" + "go.temporal.io/api/serviceerror" tclient "go.temporal.io/sdk/client" tmocks "go.temporal.io/sdk/mocks" + tp "go.temporal.io/sdk/temporal" "google.golang.org/protobuf/types/known/emptypb" "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" @@ -27,11 +30,17 @@ type startedProxyWorkflow struct { workflowName string } -// newTimingOutProxyClient returns a Temporal client whose workflow never -// produces a result, so helpers take their timeout path. +// newTimingOutProxyClient returns a Temporal client whose workflow hits its +// execution timeout, so helpers take their timeout path. func newTimingOutProxyClient() (*tmocks.Client, *startedProxyWorkflow) { + return newProxyClient(tp.NewTimeoutError(temporalEnums.TIMEOUT_TYPE_START_TO_CLOSE, nil), nil) +} + +// newProxyClient returns a Temporal client whose start fails with startErr, or, +// when that is nil, whose workflow result is getErr. +func newProxyClient(getErr, startErr error) (*tmocks.Client, *startedProxyWorkflow) { workflowRun := &tmocks.WorkflowRun{} - workflowRun.On("Get", mock.Anything, mock.Anything).Return(context.DeadlineExceeded) + workflowRun.On("Get", mock.Anything, mock.Anything).Return(getErr) started := &startedProxyWorkflow{} temporalClient := &tmocks.Client{} @@ -46,11 +55,115 @@ func newTimingOutProxyClient() (*tmocks.Client, *startedProxyWorkflow) { mock.Anything, ).Run(func(args mock.Arguments) { started.workflowName = args.Get(2).(string) - }).Return(workflowRun, nil) + }).Return(workflowRun, startErr) return temporalClient, started } +// assertNoTermination fails if the proxy tried to terminate the execution. It +// matches on the method rather than an argument list so a call that passes +// termination details cannot slip past. +func assertNoTermination(t *testing.T, temporalClient *tmocks.Client) { + t.Helper() + + for _, call := range temporalClient.Calls { + assert.NotEqual(t, "TerminateWorkflow", call.Method, "the proxy must leave the execution alone") + } +} + +// TestExecuteGRPCProxyClassifiesLostResults separates the ways a caller ends up +// without a result. They all answer 504, so only the internal cause tells an +// operator whether the execution closed itself or is still running unobserved. +// +// Neither case terminates: the activity does not heartbeat, so termination +// would discard the result without stopping the RPC it is blocked on, and for a +// deterministic ID it would free the name for a retry to start a duplicate. +func TestExecuteGRPCProxyClassifiesLostResults(t *testing.T) { + cases := []struct { + name string + getErr error + startErr error + cancelCaller bool + expectedCode int + expectedMsg string + expectedCause string + }{ + { + name: "temporal timeout while the caller is still waiting", + getErr: tp.NewTimeoutError(temporalEnums.TIMEOUT_TYPE_START_TO_CLOSE, nil), + expectedCode: http.StatusGatewayTimeout, + expectedMsg: "Flow proxy request timed out", + expectedCause: "execution timed out", + }, + { + name: "caller stops waiting for the result", + getErr: context.Canceled, + cancelCaller: true, + expectedCode: http.StatusGatewayTimeout, + expectedMsg: "Flow proxy request timed out", + expectedCause: "caller stopped waiting", + }, + { + name: "caller stops waiting during the start", + startErr: context.Canceled, + cancelCaller: true, + expectedCode: http.StatusGatewayTimeout, + expectedMsg: "Flow proxy request timed out", + expectedCause: "caller stopped waiting", + }, + { + // The SDK's own start deadline is far shorter than the caller's + // budget, so a slow frontend runs out of time while the caller is + // still waiting. The start may have landed all the same. + name: "start exceeds the SDK deadline", + startErr: serviceerror.NewDeadlineExceeded("context deadline exceeded"), + expectedCode: http.StatusGatewayTimeout, + expectedMsg: "Flow proxy request timed out", + expectedCause: "start deadline exceeded", + }, + { + // The execution's own deadline is not the caller's, so it stays a + // workflow failure rather than being reported as a gateway timeout. + name: "deadline exceeded from inside a live execution", + getErr: context.DeadlineExceeded, + expectedCode: http.StatusInternalServerError, + expectedMsg: context.DeadlineExceeded.Error(), + }, + { + name: "start fails while the caller is still waiting", + startErr: errors.New("namespace not found"), + expectedCode: http.StatusInternalServerError, + expectedMsg: "Failed to execute Flow proxy workflow", + expectedCause: "namespace not found", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + temporalClient, _ := newProxyClient(tc.getErr, tc.startErr) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + if tc.cancelCaller { + cancel() + } + + err := ExecuteFlowGRPC(ctx, temporalClient, "/v1.Flow/CreateOperationRun", &emptypb.Empty{}, nil, "flow-grpc-create-1", temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, "") + + require.NotNil(t, err) + assert.Equal(t, tc.expectedCode, err.Code) + assert.Equal(t, tc.expectedMsg, err.Message) + assertNoTermination(t, temporalClient) + if tc.expectedCause == "" { + return + } + require.NotNil(t, err.Data) + cause, ok := err.Data.(error) + require.True(t, ok, "Data is %T", err.Data) + assert.Contains(t, cause.Error(), tc.expectedCause) + }) + } +} + func TestExecuteCoreGRPC(t *testing.T) { temporalClient, started := newTimingOutProxyClient() From 7bbbe7ef911db5d5712f8337f397dd1426ca2361 Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:45:47 -0700 Subject: [PATCH 2/8] feat(rest-api): add shared dispatch helpers for Flow proxy calls FlowWorkflowID namespaces a derived workflow ID under flow-grpc-. The derivation rules are unchanged, but the resulting string has to differ: a deterministic ID plus USE_EXISTING attaches to whichever execution already holds that name, and the bespoke per-method workflows stay registered during the rollout, so a collision would hand the proxy a payload it cannot decode. ProxyFlowGRPC renders the Echo response so handlers report Flow's own status and message instead of a generic wrapper. The cause is logged and kept out of the body, where an error marshals to an empty object that tells a client nothing and contradicts the null the schema documents. APIError.LogCause chooses what to log, because Data is empty whenever the reason was folded into Message and reading it blind drops the reason entirely; logAPIError now shares that rule instead of keeping a copy. Signed-off-by: Kun Zhao --- rest-api/api/pkg/api/handler/machinepower.go | 6 +- .../pkg/api/handler/util/common/grpcproxy.go | 47 ++++++++++++++ .../api/handler/util/common/grpcproxy_test.go | 64 +++++++++++++++++++ rest-api/common/pkg/util/api.go | 15 +++++ rest-api/common/pkg/util/api_test.go | 37 +++++++++++ 5 files changed, 164 insertions(+), 5 deletions(-) diff --git a/rest-api/api/pkg/api/handler/machinepower.go b/rest-api/api/pkg/api/handler/machinepower.go index 3e4bc8f55a..11e36bcdc9 100644 --- a/rest-api/api/pkg/api/handler/machinepower.go +++ b/rest-api/api/pkg/api/handler/machinepower.go @@ -23,11 +23,7 @@ import ( ) func logAPIError(logger zerolog.Logger, apiErr *cutil.APIError, msg string) { - if apiErr.Data != nil { - logger.Error().Err(apiErr.Data).Msg(msg) - return - } - logger.Error().Err(apiErr).Msg(msg) + logger.Error().Err(apiErr.Diagnosis()).Msg(msg) } type MachinePowerControlHandler struct { diff --git a/rest-api/api/pkg/api/handler/util/common/grpcproxy.go b/rest-api/api/pkg/api/handler/util/common/grpcproxy.go index 6943b5510b..1d20144da4 100644 --- a/rest-api/api/pkg/api/handler/util/common/grpcproxy.go +++ b/rest-api/api/pkg/api/handler/util/common/grpcproxy.go @@ -12,6 +12,8 @@ import ( "strings" "github.com/google/uuid" + "github.com/labstack/echo/v4" + "github.com/rs/zerolog" temporalEnums "go.temporal.io/api/enums/v1" "go.temporal.io/api/serviceerror" tclient "go.temporal.io/sdk/client" @@ -80,6 +82,51 @@ func ExecuteFlowGRPC( return executeGRPCProxy(ctx, stc, grpcproxy.Flow, fullMethod, req, resp, workflowID, conflictPolicy, secretKey, secretFields...) } +// FlowWorkflowID namespaces a derived workflow ID under the Flow gRPC proxy, +// leaving the derivation rules themselves untouched. The namespace is what stops +// a proxy request from attaching, under USE_EXISTING, to a bespoke per-method +// execution of the same derived name: those still run on the site agent, and +// their result is a type this proxy cannot decode. +// +// It can go once no bespoke Flow workflow is left to collide with, at which +// point a shared ID costs duplicated work rather than an undecodable result. +func FlowWorkflowID(derived string) string { + return "flow-grpc-" + derived +} + +// ProxyFlowGRPC dispatches one already-validated request to Flow through the +// generic proxy workflow, decoding the reply into resp, which may be nil for +// methods with an empty response. +// +// Callers pass a deterministic workflow ID with USE_EXISTING where concurrent +// identical requests should coalesce onto one in-flight Flow call, and a fresh +// ID with UNSPECIFIED where they must not. +// +// It returns nil on success and otherwise a rendered Echo response, so handlers +// report failures without replacing Flow's status code and message with a +// generic wrapper. The internal cause stays in the log: an error in the +// response body serializes to an empty object, which tells a client nothing +// and contradicts the null the schema promises. +func ProxyFlowGRPC( + ctx context.Context, + c echo.Context, + logger zerolog.Logger, + stc tclient.Client, + fullMethod string, + req proto.Message, + resp proto.Message, + workflowID string, + conflictPolicy temporalEnums.WorkflowIdConflictPolicy, +) error { + apiErr := ExecuteFlowGRPC(ctx, stc, fullMethod, req, resp, workflowID, conflictPolicy, "") + if apiErr == nil { + return nil + } + + logger.Error().Err(apiErr.Diagnosis()).Str("Method", path.Base(fullMethod)).Msg("failed to proxy request to Flow") + return cutil.NewAPIErrorResponse(c, apiErr.Code, apiErr.Message, nil) +} + // proxyTimedOutError reports that no result arrived in time. The client-facing // message is the same however the request ran out of time, because the client // learns the same thing either way; detail is what carries the distinction into diff --git a/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go b/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go index 0b620275e6..f75ff4d0df 100644 --- a/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go +++ b/rest-api/api/pkg/api/handler/util/common/grpcproxy_test.go @@ -4,12 +4,17 @@ package common import ( + "bytes" "context" + "encoding/json" "errors" "net/http" + "net/http/httptest" "strings" "testing" + "github.com/labstack/echo/v4" + "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -24,6 +29,13 @@ import ( cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" ) +// newProxyEchoContext returns a context for the helpers that render their own +// response, along with the recorder holding what they wrote. +func newProxyEchoContext() (echo.Context, *httptest.ResponseRecorder) { + recorder := httptest.NewRecorder() + return echo.New().NewContext(httptest.NewRequest(http.MethodPost, "/", nil), recorder), recorder +} + // startedProxyWorkflow captures what a proxy helper handed to Temporal. type startedProxyWorkflow struct { options tclient.StartWorkflowOptions @@ -164,6 +176,58 @@ func TestExecuteGRPCProxyClassifiesLostResults(t *testing.T) { } } +// TestProxyFlowGRPCSeparatesDiagnosisFromResponseData keeps the diagnosis in +// the log and out of the body: an error placed in the response serializes to an +// empty object, which tells a client nothing and contradicts the null the +// schema documents. +func TestProxyFlowGRPCSeparatesDiagnosisFromResponseData(t *testing.T) { + cases := []struct { + name string + getErr error + expectedCode int + expectedLog string + }{ + { + // Flow's own rejection arrives with no separate cause, so the + // message is the only diagnosis there is. + name: "flow rejects the request", + getErr: errors.New("rack not found"), + expectedCode: http.StatusInternalServerError, + expectedLog: "rack not found", + }, + { + name: "execution times out", + getErr: tp.NewTimeoutError(temporalEnums.TIMEOUT_TYPE_START_TO_CLOSE, nil), + expectedCode: http.StatusGatewayTimeout, + expectedLog: "execution timed out", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + temporalClient, _ := newProxyClient(tc.getErr, nil) + echoCtx, recorder := newProxyEchoContext() + var logs bytes.Buffer + + err := ProxyFlowGRPC( + context.Background(), echoCtx, zerolog.New(&logs), temporalClient, + "/v1.Flow/GetRackInfoByID", + &emptypb.Empty{}, nil, + "flow-grpc-rack-get-1", temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + + require.NoError(t, err) + assert.Equal(t, tc.expectedCode, recorder.Code) + assert.Contains(t, logs.String(), tc.expectedLog) + + var body map[string]any + require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &body)) + require.Contains(t, body, "data") + assert.Nil(t, body["data"], "data must be null, got %#v", body["data"]) + }) + } +} + func TestExecuteCoreGRPC(t *testing.T) { temporalClient, started := newTimingOutProxyClient() diff --git a/rest-api/common/pkg/util/api.go b/rest-api/common/pkg/util/api.go index 6472726dc8..75221f243f 100644 --- a/rest-api/common/pkg/util/api.go +++ b/rest-api/common/pkg/util/api.go @@ -46,6 +46,21 @@ func (a *APIError) Error() string { return a.Message } +// Unwrap returns the internal error recorded in Data, so errors.Is and +// errors.As reach the cause behind an APIError. +func (a *APIError) Unwrap() error { + return a.Data +} + +// Diagnosis returns the error worth logging: the internal cause, or a itself +// when the cause was folded into Message and Unwrap is therefore nil. +func (a *APIError) Diagnosis() error { + if cause := a.Unwrap(); cause != nil { + return cause + } + return a +} + // NewAPIError returns an API error given appropriate params func NewAPIError(code int, message string, data error) *APIError { return &APIError{ diff --git a/rest-api/common/pkg/util/api_test.go b/rest-api/common/pkg/util/api_test.go index 18464b7a1d..f75231d888 100644 --- a/rest-api/common/pkg/util/api_test.go +++ b/rest-api/common/pkg/util/api_test.go @@ -55,6 +55,43 @@ func TestNewAPIErrorResponse(t *testing.T) { } } +// TestAPIErrorUnwrapAndDiagnosis pins the split between the two accessors: +// Unwrap reports a missing cause as nil, because the errors package walks it +// without detecting a cycle, and Diagnosis is where the fallback to the +// APIError itself belongs. +func TestAPIErrorUnwrapAndDiagnosis(t *testing.T) { + cause := errors.New("flow rejected the request") + + tests := []struct { + name string + apiError *APIError + expectedUnwrap error + expectedLogged string + expectedMatches bool + }{ + { + name: "cause recorded", + apiError: NewAPIError(http.StatusInternalServerError, "Failed to get Rack details", cause), + expectedUnwrap: cause, + expectedLogged: "flow rejected the request", + expectedMatches: true, + }, + { + name: "cause folded into the message", + apiError: NewAPIError(http.StatusNotFound, "Rack not found", nil), + expectedUnwrap: nil, + expectedLogged: "Rack not found", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expectedUnwrap, tt.apiError.Unwrap()) + assert.Equal(t, tt.expectedLogged, tt.apiError.Diagnosis().Error()) + assert.Equal(t, tt.expectedMatches, errors.Is(tt.apiError, cause)) + }) + } +} + func TestDefaultHTTPErrorHandler(t *testing.T) { type args struct { err error From 6eafaf6f3742cecb22d701dbe86e264131436a6f Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:47:29 -0700 Subject: [PATCH 3/8] refactor(rest-api): dispatch task, rule and run handlers through the proxy Replaces sixteen bespoke Temporal workflow types with the generic Flow proxy. Each call site keeps the workflow ID its bespoke workflow derived and the conflict policy that went with it, so read dedup and create freshness are unchanged; only the namespace and the workflow type differ. testFlowProxyReply, testFlowProxyDispatch and testFlowProxyRequest let the handler tests assert the proxied method and decoded request rather than a per-method workflow name. Signed-off-by: Kun Zhao --- .../api/pkg/api/handler/grpcproxy_test.go | 82 +++++++++ rest-api/api/pkg/api/handler/task.go | 137 ++++---------- rest-api/api/pkg/api/handler/task_test.go | 31 ++-- rest-api/api/pkg/api/handler/taskrule.go | 171 +++++------------- rest-api/api/pkg/api/handler/taskrule_test.go | 61 ++++--- rest-api/api/pkg/api/handler/taskrun.go | 170 ++++++----------- rest-api/api/pkg/api/handler/taskrun_test.go | 166 +++++++++++++---- 7 files changed, 387 insertions(+), 431 deletions(-) create mode 100644 rest-api/api/pkg/api/handler/grpcproxy_test.go diff --git a/rest-api/api/pkg/api/handler/grpcproxy_test.go b/rest-api/api/pkg/api/handler/grpcproxy_test.go new file mode 100644 index 0000000000..76ea6b042c --- /dev/null +++ b/rest-api/api/pkg/api/handler/grpcproxy_test.go @@ -0,0 +1,82 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package handler + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + tClient "go.temporal.io/sdk/client" + tmocks "go.temporal.io/sdk/mocks" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + + "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" +) + +// testFlowProxyReply makes run resolve to msg the way the site's Flow proxy +// activity does, as protojson inside a grpcproxy.Response rather than as the +// bare response proto. +func testFlowProxyReply(t *testing.T, run *tmocks.WorkflowRun, msg proto.Message) { + t.Helper() + + responseJSON, err := protojson.Marshal(msg) + require.NoError(t, err) + + run.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + args.Get(1).(*grpcproxy.Response).ResponseJSON = responseJSON + }).Return(nil) +} + +// testFlowProxyDispatch mocks the Flow proxy workflow start, asserting that the +// handler asked for fullMethod, and returns a pointer to the options it was +// started with so a test can also assert the derived workflow ID and conflict +// policy. Checking the method here is what catches an endpoint wired to the +// wrong Flow call, which the workflow ID alone cannot show. +func testFlowProxyDispatch(t *testing.T, mockTC *tmocks.Client, run *tmocks.WorkflowRun, fullMethod string, execErr error) *tClient.StartWorkflowOptions { + t.Helper() + + started := &tClient.StartWorkflowOptions{} + mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, grpcproxy.Flow.WorkflowName, mock.Anything). + Run(func(args mock.Arguments) { + *started = args.Get(1).(tClient.StartWorkflowOptions) + assert.Equal(t, fullMethod, args.Get(3).(grpcproxy.Request).FullMethod) + }). + Return(run, execErr) + return started +} + +// testFlowProxyMethodDispatch mocks the Flow proxy workflow start for one Flow +// method, routing on the method instead of asserting it. A handler that makes +// several Flow calls needs each to resolve to its own reply, which +// testFlowProxyDispatch cannot do because it matches any method. inspect, when +// non-nil, runs on the dispatched arguments so a test can decode the request. +func testFlowProxyMethodDispatch(t *testing.T, mockTC *tmocks.Client, run *tmocks.WorkflowRun, fullMethod string, inspect func(mock.Arguments)) *tClient.StartWorkflowOptions { + t.Helper() + + started := &tClient.StartWorkflowOptions{} + mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, grpcproxy.Flow.WorkflowName, + mock.MatchedBy(func(req grpcproxy.Request) bool { return req.FullMethod == fullMethod })). + Run(func(args mock.Arguments) { + *started = args.Get(1).(tClient.StartWorkflowOptions) + if inspect != nil { + inspect(args) + } + }). + Return(run, nil) + return started +} + +// testFlowProxyRequest decodes the Flow request the handler sent through the +// proxy into req, so tests can assert on the fields the transport carries as +// protojson rather than as a typed proto argument. +func testFlowProxyRequest(t *testing.T, args mock.Arguments, req proto.Message) { + t.Helper() + + proxyReq, ok := args.Get(3).(grpcproxy.Request) + require.True(t, ok, "workflow arg must be a grpcproxy.Request") + require.NoError(t, protojson.Unmarshal(proxyReq.RequestJSON, req)) +} diff --git a/rest-api/api/pkg/api/handler/task.go b/rest-api/api/pkg/api/handler/task.go index 4e2b234274..9f519e0be3 100644 --- a/rest-api/api/pkg/api/handler/task.go +++ b/rest-api/api/pkg/api/handler/task.go @@ -4,7 +4,6 @@ package handler import ( - "context" "encoding/json" "errors" "fmt" @@ -15,7 +14,6 @@ import ( "go.opentelemetry.io/otel/attribute" temporalEnums "go.temporal.io/api/enums/v1" tClient "go.temporal.io/sdk/client" - tp "go.temporal.io/sdk/temporal" "github.com/NVIDIA/infra-controller/rest-api/api/internal/config" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/handler/util/common" @@ -27,7 +25,6 @@ import ( cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" ) // ~~~~~ Get Task Handler ~~~~~ // @@ -149,33 +146,15 @@ func (gth GetTaskHandler) Handle(c echo.Context) error { TaskIds: []*flowv1.UUID{{Id: taskID}}, } - workflowOptions := tClient.StartWorkflowOptions{ - ID: fmt.Sprintf("task-get-%s", taskID), - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "GetTask", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetTask workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Task retrieval workflow", nil) - } - var flowResponse flowv1.GetTasksByIDsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, fmt.Sprintf("task-get-%s", taskID), err, "Task", "GetTask") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetTask workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Task retrieval workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetTasksByIDs_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(fmt.Sprintf("task-get-%s", taskID)), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } tasks := flowResponse.GetTasks() @@ -325,33 +304,15 @@ func (cth CancelTaskHandler) Handle(c echo.Context) error { } workflowID := fmt.Sprintf("task-cancel-%s", taskID) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "CancelTask", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule CancelTask workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Task cancellation workflow", nil) - } - var flowResponse flowv1.CancelTaskResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Task", "CancelTask") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from CancelTask workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Task cancellation workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_CancelTask_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } apiTask := model.NewAPITask(flowResponse.GetTask(), model.WithTaskReport()) @@ -501,32 +462,15 @@ func (h GetRackTasksHandler) Handle(c echo.Context) error { } workflowID := fmt.Sprintf("tasks-rack-get-%s-%s", rackID, common.QueryParamHash(apiRequest.QueryValues(pageRequest))) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "GetTasks", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule workflow to retrieve all Rack Tasks") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule workflow to retrieve all Rack Tasks", nil) - } - var flowResponse flowv1.ListTasksResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Task", "GetTasks") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetTasks workflow for Rack") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute workflow to retrieve all Rack Tasks: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ListTasks_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } taskOpts := apiRequest.TaskOptions() @@ -687,32 +631,15 @@ func (h GetTrayTasksHandler) Handle(c echo.Context) error { } workflowID := fmt.Sprintf("tasks-tray-get-%s-%s", trayID, common.QueryParamHash(apiRequest.QueryValues(pageRequest))) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "GetTasks", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule workflow to retrieve all Tray Tasks") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule workflow to retrieve all Tray Tasks", nil) - } - var flowResponse flowv1.ListTasksResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Task", "GetTasks") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetTasks workflow for Tray") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute workflow to retrieve all Tray Tasks: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ListTasks_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } taskOpts := apiRequest.TaskOptions() diff --git a/rest-api/api/pkg/api/handler/task_test.go b/rest-api/api/pkg/api/handler/task_test.go index 9522a69e63..6f4cadecf2 100644 --- a/rest-api/api/pkg/api/handler/task_test.go +++ b/rest-api/api/pkg/api/handler/task_test.go @@ -26,6 +26,7 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/model" sc "github.com/NVIDIA/infra-controller/rest-api/api/pkg/client/site" authz "github.com/NVIDIA/infra-controller/rest-api/auth/pkg/authorization" + "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" "github.com/NVIDIA/infra-controller/rest-api/common/pkg/otelecho" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" @@ -152,12 +153,9 @@ func TestGetTaskHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockTasks != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetTasksByIDsResponse) - resp.Tasks = tt.mockTasks - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetTasksByIDsResponse{Tasks: tt.mockTasks}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTask", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_GetTasksByIDs_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient q := url.Values{} @@ -224,17 +222,17 @@ func ExecuteGetTasksHandlerTestCases(t *testing.T, pathFmt string, handle func(e mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockTasks != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ListTasksResponse) - resp.Tasks = tt.mockTasks - resp.Total = int32(len(tt.mockTasks)) - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ListTasksResponse{ + Tasks: tt.mockTasks, + Total: int32(len(tt.mockTasks)), + }) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTasks", mock.Anything). + mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, grpcproxy.Flow.WorkflowName, mock.Anything). Run(func(args mock.Arguments) { + assert.Equal(t, flowv1.Flow_ListTasks_FullMethodName, args.Get(3).(grpcproxy.Request).FullMethod) if tt.assertFlowReq != nil { - req, ok := args.Get(3).(*flowv1.ListTasksRequest) - require.True(t, ok, "workflow arg must be *flowv1.ListTasksRequest") + req := &flowv1.ListTasksRequest{} + testFlowProxyRequest(t, args, req) tt.assertFlowReq(t, req, tt.pathParam) } }). @@ -544,12 +542,9 @@ func TestCancelTaskHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockTask != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.CancelTaskResponse) - resp.Task = tt.mockTask - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.CancelTaskResponse{Task: tt.mockTask}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "CancelTask", mock.Anything).Return(mockWorkflowRun, tt.mockExecErr) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_CancelTask_FullMethodName, tt.mockExecErr) scp.IDClientMap[site.ID.String()] = mockTemporalClient path := fmt.Sprintf("/v2/org/%s/nico/rack/task/%s/cancel", tt.reqOrg, tt.taskUUID) diff --git a/rest-api/api/pkg/api/handler/taskrule.go b/rest-api/api/pkg/api/handler/taskrule.go index 998e601367..18729fae2d 100644 --- a/rest-api/api/pkg/api/handler/taskrule.go +++ b/rest-api/api/pkg/api/handler/taskrule.go @@ -16,7 +16,6 @@ import ( "go.opentelemetry.io/otel/attribute" temporalEnums "go.temporal.io/api/enums/v1" tClient "go.temporal.io/sdk/client" - tp "go.temporal.io/sdk/temporal" "github.com/NVIDIA/infra-controller/rest-api/api/internal/config" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/handler/util/common" @@ -28,7 +27,6 @@ import ( cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" ) // prepareTaskRuleHandler runs the auth + site lookup + Flow-enabled check + @@ -164,32 +162,15 @@ func (h CreateTaskRuleHandler) Handle(c echo.Context) error { // Dedicated workflow ID per request so Create is never deduped. workflowID := fmt.Sprintf("task-rule-create-%s", uuid.NewString()) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "CreateTaskRule", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule CreateTaskRule workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Rule creation workflow", nil) - } - var flowResponse flowv1.CreateOperationRuleResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "TaskRule", "CreateTaskRule") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from CreateTaskRule workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Rule creation workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_CreateOperationRule_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, + ) + if proxyErr != nil { + return proxyErr } // Flow's CreateTaskRule returns only the new rule's ID; echo the @@ -273,34 +254,15 @@ func (h GetTaskRuleHandler) Handle(c echo.Context) error { RuleId: &flowv1.UUID{Id: ruleID}, } workflowID := fmt.Sprintf("task-rule-get-%s", ruleID) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "GetTaskRule", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetTaskRule workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Rule retrieval workflow", nil) - } - var flowResponse flowv1.OperationRule - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "TaskRule", "GetTaskRule") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - // Flow returns NotFound as gRPC code 5 → 404; UnwrapWorkflowError - // already maps it for us. Preserve that here. - logger.Error().Err(unwrapErr).Msg("failed to get result from GetTaskRule workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Rule retrieval workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetOperationRule_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } if flowResponse.GetId() == nil || flowResponse.GetId().GetId() == "" { @@ -391,32 +353,15 @@ func (h GetAllTaskRuleHandler) Handle(c echo.Context) error { } workflowID := fmt.Sprintf("task-rule-get-all-%s", common.QueryParamHash(apiRequest.QueryValues(pageRequest))) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "GetAllTaskRules", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetAllTaskRules workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Rule list workflow", nil) - } - var flowResponse flowv1.ListOperationRulesResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "TaskRule", "GetAllTaskRules") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetAllTaskRules workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Rule list workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ListOperationRules_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } apiRules := make([]*model.APITaskRule, 0, len(flowResponse.GetRules())) @@ -507,32 +452,17 @@ func (h UpdateTaskRuleHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, ferr.Error(), nil) } + // Dedicated workflow ID per request so concurrent updates to one rule stay + // separate executions rather than the later one reading the earlier result. workflowID := fmt.Sprintf("task-rule-update-%s-%s", ruleID, uuid.NewString()) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "UpdateTaskRule", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule UpdateTaskRule workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Rule update workflow", nil) - } - - if err := we.Get(wfCtx, nil); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "TaskRule", "UpdateTaskRule") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from UpdateTaskRule workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Rule update workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_UpdateOperationRule_FullMethodName, + flowRequest, nil, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, + ) + if proxyErr != nil { + return proxyErr } logger.Info().Str("RuleID", ruleID).Msg("finishing API handler") @@ -610,31 +540,14 @@ func (h DeleteTaskRuleHandler) Handle(c echo.Context) error { RuleId: &flowv1.UUID{Id: ruleID}, } workflowID := fmt.Sprintf("task-rule-delete-%s", ruleID) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "DeleteTaskRule", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule DeleteTaskRule workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Rule deletion workflow", nil) - } - - if err := we.Get(wfCtx, nil); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "TaskRule", "DeleteTaskRule") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from DeleteTaskRule workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Rule deletion workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_DeleteOperationRule_FullMethodName, + flowRequest, nil, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } logger.Info().Str("RuleID", ruleID).Msg("finishing API handler") diff --git a/rest-api/api/pkg/api/handler/taskrule_test.go b/rest-api/api/pkg/api/handler/taskrule_test.go index edf0a999c7..84ceec13d1 100644 --- a/rest-api/api/pkg/api/handler/taskrule_test.go +++ b/rest-api/api/pkg/api/handler/taskrule_test.go @@ -12,6 +12,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/google/uuid" @@ -20,6 +21,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" oteltrace "go.opentelemetry.io/otel/trace" + temporalEnums "go.temporal.io/api/enums/v1" tmocks "go.temporal.io/sdk/mocks" "google.golang.org/protobuf/types/known/timestamppb" @@ -27,6 +29,7 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/model" sc "github.com/NVIDIA/infra-controller/rest-api/api/pkg/client/site" authz "github.com/NVIDIA/infra-controller/rest-api/auth/pkg/authorization" + "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" "github.com/NVIDIA/infra-controller/rest-api/common/pkg/otelecho" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" @@ -153,12 +156,9 @@ func TestCreateRuleHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockResp != nil { - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.CreateOperationRuleResponse) - resp.Id = tt.mockResp.Id - }).Return(nil) + testFlowProxyReply(t, mockRun, &flowv1.CreateOperationRuleResponse{Id: tt.mockResp.Id}) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "CreateTaskRule", mock.Anything).Return(mockRun, tt.mockExecErr) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_CreateOperationRule_FullMethodName, tt.mockExecErr) scp.IDClientMap[site.ID.String()] = mockTC bodyBytes, err := json.Marshal(tt.body) @@ -182,6 +182,11 @@ func TestCreateRuleHandler_Handle(t *testing.T) { return } + // A per-request ID is what keeps two creates from becoming one + // rule, so the policy that resolves a collision never applies. + assert.True(t, strings.HasPrefix(started.ID, "flow-grpc-task-rule-create-"), "workflow ID = %q", started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, started.WorkflowIDConflictPolicy) + var got model.APITaskRule require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) assert.Equal(t, tt.mockResp.GetId().GetId(), got.ID) @@ -278,21 +283,9 @@ func TestGetRuleHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockRule != nil { - src := tt.mockRule - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.OperationRule) - resp.Id = src.Id - resp.Name = src.Name - resp.Description = src.Description - resp.OperationType = src.OperationType - resp.OperationCode = src.OperationCode - resp.RuleDefinitionJson = src.RuleDefinitionJson - resp.IsDefault = src.IsDefault - resp.CreatedAt = src.CreatedAt - resp.UpdatedAt = src.UpdatedAt - }).Return(nil) + testFlowProxyReply(t, mockRun, tt.mockRule) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTaskRule", mock.Anything).Return(mockRun, nil) + testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_GetOperationRule_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTC q := url.Values{} @@ -414,17 +407,17 @@ func TestListRulesHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockRules != nil { - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ListOperationRulesResponse) - resp.Rules = tt.mockRules - resp.TotalCount = int32(len(tt.mockRules)) - }).Return(nil) + testFlowProxyReply(t, mockRun, &flowv1.ListOperationRulesResponse{ + Rules: tt.mockRules, + TotalCount: int32(len(tt.mockRules)), + }) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetAllTaskRules", mock.Anything). + mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, grpcproxy.Flow.WorkflowName, mock.Anything). Run(func(args mock.Arguments) { + assert.Equal(t, flowv1.Flow_ListOperationRules_FullMethodName, args.Get(3).(grpcproxy.Request).FullMethod) if tt.assertFlowReq != nil { - req, ok := args.Get(3).(*flowv1.ListOperationRulesRequest) - require.True(t, ok) + req := &flowv1.ListOperationRulesRequest{} + testFlowProxyRequest(t, args, req) tt.assertFlowReq(t, req) } }). @@ -540,7 +533,7 @@ func TestUpdateRuleHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") mockRun.Mock.On("Get", mock.Anything, mock.Anything).Return(tt.mockGetErr) - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "UpdateTaskRule", mock.Anything).Return(mockRun, tt.mockExecErr) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_UpdateOperationRule_FullMethodName, tt.mockExecErr) scp.IDClientMap[site.ID.String()] = mockTC bodyBytes, err := json.Marshal(tt.body) @@ -558,6 +551,16 @@ func TestUpdateRuleHandler_Handle(t *testing.T) { _ = handler.Handle(ec) require.Equal(t, tt.expectedStatus, rec.Code, "body=%s", rec.Body.String()) + + if tt.expectedStatus != http.StatusNoContent { + return + } + + // Concurrent updates to one rule must stay separate executions, so + // the ID carries a per-request suffix and never resolves a + // collision by reading another request's result. + assert.True(t, strings.HasPrefix(started.ID, fmt.Sprintf("flow-grpc-task-rule-update-%s-", tt.ruleID)), "workflow ID = %q", started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, started.WorkflowIDConflictPolicy) }) } } @@ -624,7 +627,7 @@ func TestDeleteRuleHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") mockRun.Mock.On("Get", mock.Anything, mock.Anything).Return(nil) - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "DeleteTaskRule", mock.Anything).Return(mockRun, nil) + testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_DeleteOperationRule_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTC q := url.Values{} diff --git a/rest-api/api/pkg/api/handler/taskrun.go b/rest-api/api/pkg/api/handler/taskrun.go index 8bc9ecbfeb..c9ff7b6a05 100644 --- a/rest-api/api/pkg/api/handler/taskrun.go +++ b/rest-api/api/pkg/api/handler/taskrun.go @@ -16,7 +16,7 @@ import ( "go.opentelemetry.io/otel/attribute" temporalEnums "go.temporal.io/api/enums/v1" tClient "go.temporal.io/sdk/client" - tp "go.temporal.io/sdk/temporal" + "google.golang.org/protobuf/proto" "github.com/NVIDIA/infra-controller/rest-api/api/internal/config" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/handler/util/common" @@ -28,7 +28,6 @@ import ( cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" ) // prepareRunHandler runs the auth + site lookup + Flow-enabled check + @@ -104,18 +103,6 @@ func prepareRunHandler( return site, stc, nil } -// runWorkflowOptions returns the standard site-queue workflow options for -// a run action with the given deterministic workflow ID. -func runWorkflowOptions(workflowID string) tClient.StartWorkflowOptions { - return tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } -} - // ~~~~~ Create Run Handler ~~~~~ // // CreateTaskRunHandler is the API Handler for creating a Run. @@ -172,26 +159,17 @@ func (h CreateTaskRunHandler) Handle(c echo.Context) error { flowRequest := apiRequest.ToProto() // Dedicated workflow ID per request so Create is never deduped. - workflowID := fmt.Sprintf("task-run-create-%s", uuid.NewString()) - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, runWorkflowOptions(workflowID), "CreateTaskRun", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule CreateTaskRun workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Run creation workflow", nil) - } + workflowID := common.FlowWorkflowID(fmt.Sprintf("task-run-create-%s", uuid.NewString())) var flowResponse flowv1.CreateOperationRunResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Run", "CreateTaskRun") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from CreateTaskRun workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Run creation workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_CreateOperationRun_FullMethodName, + flowRequest, &flowResponse, + workflowID, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, + ) + if proxyErr != nil { + return proxyErr } // Flow's CreateOperationRun returns only the new run's ID. Echo the known @@ -281,26 +259,17 @@ func (h GetTaskRunHandler) Handle(c echo.Context) error { // IncludeStats is part of the workflow ID because the conflict policy // attaches to an in-flight execution with the same ID, which would // otherwise return a response whose stats presence contradicts the query. - workflowID := fmt.Sprintf("task-run-get-%s-%t", runID, apiRequest.IncludeStats) - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, runWorkflowOptions(workflowID), "GetTaskRun", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetTaskRun workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Run retrieval workflow", nil) - } + workflowID := common.FlowWorkflowID(fmt.Sprintf("task-run-get-%s-%t", runID, apiRequest.IncludeStats)) var flowResponse flowv1.GetOperationRunResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Run", "GetTaskRun") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetTaskRun workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Run retrieval workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetOperationRun_FullMethodName, + flowRequest, &flowResponse, + workflowID, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } run := flowResponse.GetOperationRun() @@ -388,26 +357,17 @@ func (h GetAllTaskRunHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, ferr.Error(), nil) } - workflowID := fmt.Sprintf("task-run-get-all-%s", common.QueryParamHash(apiRequest.QueryValues(pageRequest))) - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, runWorkflowOptions(workflowID), "GetAllTaskRuns", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetAllTaskRuns workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Run list workflow", nil) - } + workflowID := common.FlowWorkflowID(fmt.Sprintf("task-run-get-all-%s", common.QueryParamHash(apiRequest.QueryValues(pageRequest)))) var flowResponse flowv1.ListOperationRunsResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Run", "GetAllTaskRuns") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetAllTaskRuns workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Run list workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ListOperationRuns_FullMethodName, + flowRequest, &flowResponse, + workflowID, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } apiRuns := make([]*model.APITaskRun, 0, len(flowResponse.GetOperationRuns())) @@ -508,26 +468,17 @@ func (h GetAllTaskRunTargetHandler) Handle(c echo.Context) error { } flowRequest := apiRequest.ToProto(runID, pageRequest) - workflowID := fmt.Sprintf("task-run-target-get-all-%s-%s", runID, common.QueryParamHash(apiRequest.QueryValues(pageRequest))) - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, runWorkflowOptions(workflowID), "GetAllTaskRunTargets", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to schedule GetAllTaskRunTargets workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to schedule Run targets workflow", nil) - } + workflowID := common.FlowWorkflowID(fmt.Sprintf("task-run-target-get-all-%s-%s", runID, common.QueryParamHash(apiRequest.QueryValues(pageRequest)))) var flowResponse flowv1.ListOperationRunTargetsResponse - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Run", "GetAllTaskRunTargets") - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msg("failed to get result from GetAllTaskRunTargets workflow") - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Run targets workflow on Site: %s", unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ListOperationRunTargets_FullMethodName, + flowRequest, &flowResponse, + workflowID, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } apiTargets := make([]*model.APITaskRunTarget, 0, len(flowResponse.GetTargets())) @@ -552,16 +503,16 @@ func (h GetAllTaskRunTargetHandler) Handle(c echo.Context) error { // ~~~~~ Lifecycle Handlers (pause/resume/advance/cancel) ~~~~~ // -// executeRunLifecycleWorkflow executes one OperationRun-returning lifecycle workflow -// and renders the resulting run. It centralizes the auth/site prep, -// workflow execution, and error handling shared by pause/resume/advance/cancel. -func executeRunLifecycleWorkflow( +// executeRunLifecycleAction proxies one OperationRun-returning lifecycle call to +// Flow and renders the resulting run. It centralizes the auth/site prep, +// dispatch, and error handling shared by pause/resume/advance/cancel. +func executeRunLifecycleAction( c echo.Context, dbSession *cdb.Session, scp *sc.ClientPool, dbUser *cdbm.User, - org, siteID, runID, action, workflowName string, - flowRequest any, + org, siteID, runID, action, fullMethod string, + flowRequest proto.Message, logger zerolog.Logger, ctx context.Context, ) error { @@ -570,26 +521,17 @@ func executeRunLifecycleWorkflow( return cutil.NewAPIErrorResponse(c, apiErr.Code, apiErr.Message, apiErr.Data) } - workflowID := fmt.Sprintf("task-run-%s-%s", action, runID) - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, runWorkflowOptions(workflowID), workflowName, flowRequest) - if err != nil { - logger.Error().Err(err).Msgf("failed to schedule %s workflow", workflowName) - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to schedule Run %s workflow", action), nil) - } + workflowID := common.FlowWorkflowID(fmt.Sprintf("task-run-%s-%s", action, runID)) var flowResponse flowv1.OperationRun - if err := we.Get(wfCtx, &flowResponse); err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || wfCtx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Run", workflowName) - } - code, unwrapErr := common.UnwrapWorkflowError(err) - logger.Error().Err(unwrapErr).Msgf("failed to get result from %s workflow", workflowName) - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to execute Run %s workflow on Site: %s", action, unwrapErr), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + fullMethod, + flowRequest, &flowResponse, + workflowID, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } apiRun := &model.APITaskRun{} @@ -651,7 +593,7 @@ func (h PauseTaskRunHandler) Handle(c echo.Context) error { } flowRequest := &flowv1.PauseOperationRunRequest{Id: &flowv1.UUID{Id: runID}} - return executeRunLifecycleWorkflow(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "pause", "PauseTaskRun", flowRequest, logger, ctx) + return executeRunLifecycleAction(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "pause", flowv1.Flow_PauseOperationRun_FullMethodName, flowRequest, logger, ctx) } // ResumeTaskRunHandler resumes an operator-paused Run. @@ -707,7 +649,7 @@ func (h ResumeTaskRunHandler) Handle(c echo.Context) error { } flowRequest := &flowv1.ResumeOperationRunRequest{Id: &flowv1.UUID{Id: runID}} - return executeRunLifecycleWorkflow(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "resume", "ResumeTaskRun", flowRequest, logger, ctx) + return executeRunLifecycleAction(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "resume", flowv1.Flow_ResumeOperationRun_FullMethodName, flowRequest, logger, ctx) } // AdvanceTaskRunPhaseHandler opens the next phase of a phase-gated Run. @@ -766,7 +708,7 @@ func (h AdvanceTaskRunPhaseHandler) Handle(c echo.Context) error { Id: &flowv1.UUID{Id: runID}, ExpectedPhaseIndex: apiRequest.ExpectedPhaseIndex, } - return executeRunLifecycleWorkflow(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "advance", "AdvanceTaskRunPhase", flowRequest, logger, ctx) + return executeRunLifecycleAction(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "advance", flowv1.Flow_AdvanceOperationRunPhase_FullMethodName, flowRequest, logger, ctx) } // CancelTaskRunHandler cancels a Run and its in-flight targets. @@ -825,5 +767,5 @@ func (h CancelTaskRunHandler) Handle(c echo.Context) error { Id: &flowv1.UUID{Id: runID}, Reason: apiRequest.Reason, } - return executeRunLifecycleWorkflow(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "cancel", "CancelTaskRun", flowRequest, logger, ctx) + return executeRunLifecycleAction(c, h.dbSession, h.scp, dbUser, org, apiRequest.SiteID, runID, "cancel", flowv1.Flow_CancelOperationRun_FullMethodName, flowRequest, logger, ctx) } diff --git a/rest-api/api/pkg/api/handler/taskrun_test.go b/rest-api/api/pkg/api/handler/taskrun_test.go index 22db06906c..9cbc2b36e0 100644 --- a/rest-api/api/pkg/api/handler/taskrun_test.go +++ b/rest-api/api/pkg/api/handler/taskrun_test.go @@ -12,14 +12,15 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/google/uuid" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" oteltrace "go.opentelemetry.io/otel/trace" + temporalEnums "go.temporal.io/api/enums/v1" tmocks "go.temporal.io/sdk/mocks" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/handler/util/common" @@ -135,12 +136,9 @@ func TestCreateTaskRunHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockResp != nil { - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.CreateOperationRunResponse) - resp.Id = tt.mockResp.Id - }).Return(nil) + testFlowProxyReply(t, mockRun, tt.mockResp) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "CreateTaskRun", mock.Anything).Return(mockRun, tt.mockExecErr) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_CreateOperationRun_FullMethodName, tt.mockExecErr) scp.IDClientMap[site.ID.String()] = mockTC bodyBytes, err := json.Marshal(tt.body) @@ -162,6 +160,12 @@ func TestCreateTaskRunHandler_Handle(t *testing.T) { if tt.expectedStatus != http.StatusCreated { return } + + // Create must never coalesce onto another request's execution, so + // its ID is per-request and it declares no conflict policy. + assert.True(t, strings.HasPrefix(started.ID, "flow-grpc-task-run-create-"), "workflow ID = %q", started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, started.WorkflowIDConflictPolicy) + var got model.APITaskRun require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) assert.Equal(t, tt.mockResp.GetId().GetId(), got.ID) @@ -172,6 +176,60 @@ func TestCreateTaskRunHandler_Handle(t *testing.T) { } } +// TestCreateTaskRunHandler_FreshWorkflowIDPerRequest pins what keeps two +// identical create requests from becoming one run. The ID is the whole +// mechanism: a fixed one would let the second request attach to the first +// execution and report its run as though it had created a second. +func TestCreateTaskRunHandler_FreshWorkflowIDPerRequest(t *testing.T) { + e := echo.New() + dbSession := testRackInitDB(t) + defer dbSession.Close() + + cfg := common.GetTestConfig() + tcfg, _ := cfg.GetTemporalConfig() + scp := sc.NewClientPool(tcfg) + + org := "test-org" + _, site, _ := testRackSetupTestData(t, dbSession, org) + providerUser := testRackBuildUser(t, dbSession, "provider-user-run-create-fresh", org, []string{authz.ProviderAdminRole}) + + handler := NewCreateTaskRunHandler(dbSession, nil, scp, cfg) + tracer := oteltrace.NewNoopTracerProvider().Tracer("test") + + submit := func(t *testing.T) string { + t.Helper() + + mockTC := &tmocks.Client{} + mockRun := &tmocks.WorkflowRun{} + mockRun.On("GetID").Return("test-workflow-id") + testFlowProxyReply(t, mockRun, &flowv1.CreateOperationRunResponse{Id: &flowv1.UUID{Id: uuid.New().String()}}) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_CreateOperationRun_FullMethodName, nil) + scp.IDClientMap[site.ID.String()] = mockTC + + bodyBytes, err := json.Marshal(testRunSampleCreateRequest(site.ID.String())) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/v2/org/%s/nico/task/run", org), bytes.NewReader(bodyBytes)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName") + ec.SetParamValues(org) + ec.Set("user", providerUser) + ec.SetRequest(ec.Request().WithContext(context.WithValue(context.Background(), otelecho.TracerKey, tracer))) + + require.NoError(t, handler.Handle(ec)) + require.Equal(t, http.StatusCreated, rec.Code, "body=%s", rec.Body.String()) + return started.ID + } + + first := submit(t) + second := submit(t) + + assert.NotEqual(t, first, second, "two creates shared a workflow ID") + assert.True(t, strings.HasPrefix(first, "flow-grpc-task-run-create-"), "workflow ID = %q", first) +} + func TestGetTaskRunHandler_Handle(t *testing.T) { e := echo.New() dbSession := testRackInitDB(t) @@ -214,6 +272,14 @@ func TestGetTaskRunHandler_Handle(t *testing.T) { mockRun: found, expectedStatus: http.StatusOK, }, + { + name: "success - includeStats gets its own workflow ID", + user: providerUser, + runID: runID, + queryParams: map[string]string{"siteId": site.ID.String(), "includeStats": "true"}, + mockRun: found, + expectedStatus: http.StatusOK, + }, { name: "failure - run not found", user: providerUser, @@ -251,13 +317,9 @@ func TestGetTaskRunHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockRun != nil { - src := tt.mockRun - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetOperationRunResponse) - resp.OperationRun = src - }).Return(nil) + testFlowProxyReply(t, mockRun, &flowv1.GetOperationRunResponse{OperationRun: tt.mockRun}) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTaskRun", mock.Anything).Return(mockRun, nil) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_GetOperationRun_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTC q := url.Values{} @@ -281,6 +343,16 @@ func TestGetTaskRunHandler_Handle(t *testing.T) { if tt.expectedStatus != http.StatusOK { return } + + // Reads coalesce onto an in-flight identical request, so the ID is + // derived from the query and namespaced away from the bespoke + // per-method workflows that still run on the site. includeStats is + // part of it because attaching to an execution started with the + // other value would answer with the wrong stats presence. + wantStats := tt.queryParams["includeStats"] == "true" + assert.Equal(t, fmt.Sprintf("flow-grpc-task-run-get-%s-%t", runID, wantStats), started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, started.WorkflowIDConflictPolicy) + var got model.APITaskRun require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) assert.Equal(t, runID, got.ID) @@ -356,19 +428,22 @@ func TestGetAllTaskRunHandler_Handle(t *testing.T) { }, } + // Collected across subtests: two different filter sets must not derive the + // same ID, or USE_EXISTING would serve one query's result to the other. + listIDs := map[string]string{} + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockTC := &tmocks.Client{} mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockRuns != nil { - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ListOperationRunsResponse) - resp.OperationRuns = tt.mockRuns - resp.Total = int32(len(tt.mockRuns)) - }).Return(nil) + testFlowProxyReply(t, mockRun, &flowv1.ListOperationRunsResponse{ + OperationRuns: tt.mockRuns, + Total: int32(len(tt.mockRuns)), + }) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetAllTaskRuns", mock.Anything).Return(mockRun, nil) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_ListOperationRuns_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTC q := url.Values{} @@ -392,12 +467,23 @@ func TestGetAllTaskRunHandler_Handle(t *testing.T) { if tt.expectedStatus != http.StatusOK { return } + + assert.True(t, strings.HasPrefix(started.ID, "flow-grpc-task-run-get-all-"), "workflow ID = %q", started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, started.WorkflowIDConflictPolicy) + listIDs[tt.name] = started.ID + var got []*model.APITaskRun require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) require.Len(t, got, len(tt.mockRuns)) require.NotEmpty(t, rec.Header().Get("X-Pagination")) }) } + + seen := map[string]string{} + for name, id := range listIDs { + require.NotContains(t, seen, id, "%q and %q derived the same workflow ID", seen[id], name) + seen[id] = name + } } func TestGetAllTaskRunTargetHandler_Handle(t *testing.T) { @@ -479,13 +565,12 @@ func TestGetAllTaskRunTargetHandler_Handle(t *testing.T) { mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") if tt.mockTargets != nil { - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ListOperationRunTargetsResponse) - resp.Targets = tt.mockTargets - resp.Total = int32(len(tt.mockTargets)) - }).Return(nil) + testFlowProxyReply(t, mockRun, &flowv1.ListOperationRunTargetsResponse{ + Targets: tt.mockTargets, + Total: int32(len(tt.mockTargets)), + }) } - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetAllTaskRunTargets", mock.Anything).Return(mockRun, nil) + started := testFlowProxyDispatch(t, mockTC, mockRun, flowv1.Flow_ListOperationRunTargets_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTC q := url.Values{} @@ -509,6 +594,10 @@ func TestGetAllTaskRunTargetHandler_Handle(t *testing.T) { if tt.expectedStatus != http.StatusOK { return } + + assert.True(t, strings.HasPrefix(started.ID, fmt.Sprintf("flow-grpc-task-run-target-get-all-%s-", runID)), "workflow ID = %q", started.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, started.WorkflowIDConflictPolicy) + var got []*model.APITaskRunTarget require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) require.Len(t, got, len(tt.mockTargets)) @@ -558,15 +647,18 @@ func TestRunLifecycleHandlers_Handle(t *testing.T) { } } + // The method is pinned per action because all four handlers share one + // dispatch path and one ID shape: without it, pause wired to Flow's resume + // would still satisfy every other assertion here. actions := []struct { - action string - workflow string - handle func(echo.Context) error + action string + method string + handle func(echo.Context) error }{ - {"pause", "PauseTaskRun", NewPauseTaskRunHandler(dbSession, nil, scp, cfg).Handle}, - {"resume", "ResumeTaskRun", NewResumeTaskRunHandler(dbSession, nil, scp, cfg).Handle}, - {"advance", "AdvanceTaskRunPhase", NewAdvanceTaskRunPhaseHandler(dbSession, nil, scp, cfg).Handle}, - {"cancel", "CancelTaskRun", NewCancelTaskRunHandler(dbSession, nil, scp, cfg).Handle}, + {"pause", flowv1.Flow_PauseOperationRun_FullMethodName, NewPauseTaskRunHandler(dbSession, nil, scp, cfg).Handle}, + {"resume", flowv1.Flow_ResumeOperationRun_FullMethodName, NewResumeTaskRunHandler(dbSession, nil, scp, cfg).Handle}, + {"advance", flowv1.Flow_AdvanceOperationRunPhase_FullMethodName, NewAdvanceTaskRunPhaseHandler(dbSession, nil, scp, cfg).Handle}, + {"cancel", flowv1.Flow_CancelOperationRun_FullMethodName, NewCancelTaskRunHandler(dbSession, nil, scp, cfg).Handle}, } for _, act := range actions { @@ -590,11 +682,10 @@ func TestRunLifecycleHandlers_Handle(t *testing.T) { mockTC := &tmocks.Client{} mockRun := &tmocks.WorkflowRun{} mockRun.On("GetID").Return("test-workflow-id") - mockRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.OperationRun) - resp.Summary = &flowv1.OperationRunSummary{Id: &flowv1.UUID{Id: runID}} - }).Return(nil) - mockTC.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, act.workflow, mock.Anything).Return(mockRun, tt.mockExecErr) + testFlowProxyReply(t, mockRun, &flowv1.OperationRun{ + Summary: &flowv1.OperationRunSummary{Id: &flowv1.UUID{Id: runID}}, + }) + started := testFlowProxyDispatch(t, mockTC, mockRun, act.method, tt.mockExecErr) scp.IDClientMap[site.ID.String()] = mockTC bodyBytes, err := json.Marshal(tt.body) @@ -616,6 +707,9 @@ func TestRunLifecycleHandlers_Handle(t *testing.T) { if tt.expectedStatus != http.StatusAccepted { return } + + assert.Equal(t, fmt.Sprintf("flow-grpc-task-run-%s-%s", act.action, runID), started.ID) + var got model.APITaskRun require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) assert.Equal(t, runID, got.ID) From 95d4ec135113fdb11d83c83b4d38e81643a7d9c0 Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:55:45 -0700 Subject: [PATCH 4/8] refactor(rest-api): dispatch rack, tray and mutations through the proxy Replaces the remaining ten bespoke Temporal workflow types, completing the move of all 26 Flow-backed endpoints onto the generic proxy. The rack and tray handlers share ExecutePowerControlWorkflow, ExecuteBringUpRackWorkflow and ExecuteFirmwareUpdateWorkflow, so the handlers and those helpers have to move together. The mutating paths lose their timeout termination, which the proxy deliberately does not do, and their activity retries: the proxy activity runs with MaximumAttempts 1 so a mutation is never resubmitted to Flow behind the caller's back. A timeout now reports 504 rather than 500. flowmutation_test.go pins the proxied method, workflow ID, conflict policy and decoded request for each of the three helpers, which the handler tests had been matching loosely. Signed-off-by: Kun Zhao --- rest-api/api/pkg/api/handler/rack.go | 147 +++-------- rest-api/api/pkg/api/handler/rack_test.go | 176 +++++-------- rest-api/api/pkg/api/handler/tray.go | 207 +++++---------- rest-api/api/pkg/api/handler/tray_test.go | 237 ++++++++++++------ .../api/pkg/api/handler/util/common/common.go | 128 +++------- .../handler/util/common/flowmutation_test.go | 225 +++++++++++++++++ 6 files changed, 575 insertions(+), 545 deletions(-) create mode 100644 rest-api/api/pkg/api/handler/util/common/flowmutation_test.go diff --git a/rest-api/api/pkg/api/handler/rack.go b/rest-api/api/pkg/api/handler/rack.go index 2c98d4ec0c..df38b75114 100644 --- a/rest-api/api/pkg/api/handler/rack.go +++ b/rest-api/api/pkg/api/handler/rack.go @@ -4,7 +4,6 @@ package handler import ( - "context" "encoding/json" "errors" "fmt" @@ -17,7 +16,6 @@ import ( "go.opentelemetry.io/otel/attribute" temporalEnums "go.temporal.io/api/enums/v1" tClient "go.temporal.io/sdk/client" - tp "go.temporal.io/sdk/temporal" "github.com/NVIDIA/infra-controller/rest-api/api/internal/config" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/handler/util/common" @@ -29,7 +27,6 @@ import ( cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" ) // ~~~~~ Get Rack Handler ~~~~~ // @@ -161,35 +158,15 @@ func (grh GetRackHandler) Handle(c echo.Context) error { } // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: fmt.Sprintf("rack-get-%s", rackStrID), - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "GetRack", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute GetRack workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to get Rack details", nil) - } - - // Get workflow result var flowResponse flowv1.GetRackInfoResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, fmt.Sprintf("rack-get-%s", rackStrID), err, "Rack", "GetRack") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from GetRack workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to get Rack details: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetRackInfoByID_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(fmt.Sprintf("rack-get-%s", rackStrID)), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -367,35 +344,15 @@ func (garh GetAllRackHandler) Handle(c echo.Context) error { workflowID := fmt.Sprintf("rack-get-all-%s", common.QueryParamHash(apiRequest.QueryValues())) // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "GetRacks", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute GetRacks workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to get Racks", nil) - } - - // Get workflow result var flowResponse flowv1.GetListOfRacksResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Rack", "GetRacks") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from GetRacks workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to get Racks: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetListOfRacks_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -553,35 +510,15 @@ func (vrh ValidateRackHandler) Handle(c echo.Context) error { } // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: fmt.Sprintf("rack-validate-%s", rackStrID), - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "ValidateRackComponents", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute ValidateComponents workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to validate Rack", nil) - } - - // Get workflow result var flowResponse flowv1.ValidateComponentsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, fmt.Sprintf("rack-validate-%s", rackStrID), err, "Rack", "ValidateRackComponents") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from ValidateComponents workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to validate Rack: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ValidateComponents_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(fmt.Sprintf("rack-validate-%s", rackStrID)), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -718,35 +655,15 @@ func (vrsh ValidateRacksHandler) Handle(c echo.Context) error { workflowID := fmt.Sprintf("rack-validate-all-%s", common.QueryParamHash(apiRequest.QueryValues())) // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "ValidateRackComponents", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute ValidateComponents workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to validate Racks", nil) - } - - // Get workflow result var flowResponse flowv1.ValidateComponentsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Rack", "ValidateRackComponents") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from ValidateComponents workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to validate Racks: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ValidateComponents_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model diff --git a/rest-api/api/pkg/api/handler/rack_test.go b/rest-api/api/pkg/api/handler/rack_test.go index a6dafac348..5c876aa687 100644 --- a/rest-api/api/pkg/api/handler/rack_test.go +++ b/rest-api/api/pkg/api/handler/rack_test.go @@ -18,6 +18,7 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/pagination" sc "github.com/NVIDIA/infra-controller/rest-api/api/pkg/client/site" authz "github.com/NVIDIA/infra-controller/rest-api/auth/pkg/authorization" + "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" "github.com/NVIDIA/infra-controller/rest-api/common/pkg/otelecho" cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" @@ -32,6 +33,7 @@ import ( "github.com/uptrace/bun/extra/bundebug" oteltrace "go.opentelemetry.io/otel/trace" tmocks "go.temporal.io/sdk/mocks" + "google.golang.org/protobuf/proto" ) func testRackInitDB(t *testing.T) *cdb.Session { @@ -266,18 +268,8 @@ func TestGetRackHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - if tt.mockRack != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetRackInfoResponse) - resp.Rack = tt.mockRack - }).Return(nil) - } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetRackInfoResponse) - resp.Rack = nil - }).Return(nil) - } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetRack", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetRackInfoResponse{Rack: tt.mockRack}) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_GetRackInfoByID_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient q := url.Values{} @@ -548,20 +540,15 @@ func TestGetAllRackHandler_Handle(t *testing.T) { mockWorkflowRun.On("GetID").Return("test-workflow-id") // Always set up Get mock, even for error cases, as handler may still call it if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetListOfRacksResponse) - resp.Racks = tt.mockResponse.Racks - resp.Total = tt.mockResponse.Total - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetListOfRacksResponse{ + Racks: tt.mockResponse.Racks, + Total: tt.mockResponse.Total, + }) } else { - // For error cases, set up a mock that returns empty response - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetListOfRacksResponse) - resp.Racks = []*flowv1.Rack{} - resp.Total = 0 - }).Return(nil) + // For error cases, reply with an empty response + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetListOfRacksResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetRacks", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_GetListOfRacks_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient // Build query string @@ -762,23 +749,18 @@ func TestValidateRackHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = tt.mockResponse.Diffs - resp.TotalDiffs = tt.mockResponse.TotalDiffs - resp.MissingCount = tt.mockResponse.MissingCount - resp.UnexpectedCount = tt.mockResponse.UnexpectedCount - resp.MismatchCount = tt.mockResponse.MismatchCount - resp.MatchCount = tt.mockResponse.MatchCount - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{ + Diffs: tt.mockResponse.Diffs, + TotalDiffs: tt.mockResponse.TotalDiffs, + MissingCount: tt.mockResponse.MissingCount, + UnexpectedCount: tt.mockResponse.UnexpectedCount, + MismatchCount: tt.mockResponse.MismatchCount, + MatchCount: tt.mockResponse.MatchCount, + }) } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = []*flowv1.ComponentDiff{} - resp.TotalDiffs = 0 - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "ValidateRackComponents", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_ValidateComponents_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient // Build query string @@ -985,23 +967,18 @@ func TestValidateRacksHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = tt.mockResponse.Diffs - resp.TotalDiffs = tt.mockResponse.TotalDiffs - resp.MissingCount = tt.mockResponse.MissingCount - resp.UnexpectedCount = tt.mockResponse.UnexpectedCount - resp.MismatchCount = tt.mockResponse.MismatchCount - resp.MatchCount = tt.mockResponse.MatchCount - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{ + Diffs: tt.mockResponse.Diffs, + TotalDiffs: tt.mockResponse.TotalDiffs, + MissingCount: tt.mockResponse.MissingCount, + UnexpectedCount: tt.mockResponse.UnexpectedCount, + MismatchCount: tt.mockResponse.MismatchCount, + MatchCount: tt.mockResponse.MatchCount, + }) } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = []*flowv1.ComponentDiff{} - resp.TotalDiffs = 0 - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "ValidateRackComponents", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_ValidateComponents_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient q := url.Values{} @@ -1165,12 +1142,7 @@ func TestUpdateRackPowerStateHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1286,12 +1258,7 @@ func TestBatchUpdateRackPowerStateHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1408,12 +1375,7 @@ func TestUpdateRackFirmwareHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1530,12 +1492,7 @@ func TestBringUpRackHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1652,12 +1609,7 @@ func TestBatchBringUpRackHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1759,12 +1711,7 @@ func TestBatchUpdateRackFirmwareHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1827,7 +1774,8 @@ func TestRackHandlers_RuleIDPassThrough(t *testing.T) { path string body string handler echo.HandlerFunc - extractRID func(req interface{}) string + flowReq proto.Message + extractRID func(req proto.Message) string }{ { name: "power - PowerOnRackRequest carries rule_id", @@ -1836,12 +1784,9 @@ func TestRackHandlers_RuleIDPassThrough(t *testing.T) { handler: func() echo.HandlerFunc { return NewUpdateRackPowerStateHandler(dbSession, nil, scp, cfg).Handle }(), - extractRID: func(req interface{}) string { - r, ok := req.(*flowv1.PowerOnRackRequest) - if !ok { - return "" - } - return r.GetRuleId().GetId() + flowReq: &flowv1.PowerOnRackRequest{}, + extractRID: func(req proto.Message) string { + return req.(*flowv1.PowerOnRackRequest).GetRuleId().GetId() }, }, { @@ -1851,12 +1796,9 @@ func TestRackHandlers_RuleIDPassThrough(t *testing.T) { handler: func() echo.HandlerFunc { return NewUpdateRackFirmwareHandler(dbSession, nil, scp, cfg).Handle }(), - extractRID: func(req interface{}) string { - r, ok := req.(*flowv1.UpgradeFirmwareRequest) - if !ok { - return "" - } - return r.GetRuleId().GetId() + flowReq: &flowv1.UpgradeFirmwareRequest{}, + extractRID: func(req proto.Message) string { + return req.(*flowv1.UpgradeFirmwareRequest).GetRuleId().GetId() }, }, { @@ -1866,32 +1808,28 @@ func TestRackHandlers_RuleIDPassThrough(t *testing.T) { handler: func() echo.HandlerFunc { return NewBringUpRackHandler(dbSession, nil, scp, cfg).Handle }(), - extractRID: func(req interface{}) string { - r, ok := req.(*flowv1.BringUpRackRequest) - if !ok { - return "" - } - return r.GetRuleId().GetId() + flowReq: &flowv1.BringUpRackRequest{}, + extractRID: func(req proto.Message) string { + return req.(*flowv1.BringUpRackRequest).GetRuleId().GetId() }, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - var capturedReq interface{} + dispatched := false mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - resp.TaskIds = []*flowv1.UUID{{Id: uuid.NewString()}} - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{ + TaskIds: []*flowv1.UUID{{Id: uuid.NewString()}}, + }) mockTemporalClient.Mock.On("ExecuteWorkflow", - mock.Anything, mock.Anything, mock.Anything, mock.Anything, + mock.Anything, mock.Anything, grpcproxy.Flow.WorkflowName, mock.Anything, ).Run(func(args mock.Arguments) { - // (ctx, options, workflowName, flowRequest) - capturedReq = args.Get(3) + testFlowProxyRequest(t, args, tc.flowReq) + dispatched = true }).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1911,8 +1849,8 @@ func TestRackHandlers_RuleIDPassThrough(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, rec.Code, "body=%s", rec.Body.String()) - require.NotNil(t, capturedReq, "ExecuteWorkflow was not called") - assert.Equal(t, ruleID, tc.extractRID(capturedReq)) + require.True(t, dispatched, "the Flow proxy workflow was not started") + assert.Equal(t, ruleID, tc.extractRID(tc.flowReq)) }) } } diff --git a/rest-api/api/pkg/api/handler/tray.go b/rest-api/api/pkg/api/handler/tray.go index 2eb2718432..722525b592 100644 --- a/rest-api/api/pkg/api/handler/tray.go +++ b/rest-api/api/pkg/api/handler/tray.go @@ -28,15 +28,13 @@ import ( cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" temporalEnums "go.temporal.io/api/enums/v1" - tp "go.temporal.io/sdk/temporal" ) // ~~~~~ Slot resolution helpers ~~~~~ // -// resolveTrayIDsBySlot enumerates trays for the given baseSpec via Flow's -// GetTrays workflow and returns the UUIDs of components at the requested slot. +// resolveTrayIDsBySlot enumerates trays for the given baseSpec through Flow's +// GetComponents and returns the UUIDs of components at the requested slot. // // baseSpec is the OperationTargetSpec the request would otherwise have // produced (rack scope, component-pinning ids/componentIds, or "all trays @@ -46,40 +44,31 @@ import ( // // Flow has no by-slot component target shape; REST resolves slotId to // component UUIDs and drives downstream workflows with ComponentTargets. +// +// It returns the proxy's own APIError rather than a plain error so a slot +// filter cannot downgrade the status the endpoint would otherwise report: a +// timeout here means the same thing to a client as a timeout on the call the +// slot resolution precedes. func resolveTrayIDsBySlot( ctx context.Context, stc tClient.Client, baseSpec *flowv1.OperationTargetSpec, slot model.RackComponentSlotMatcher, -) ([]string, error) { +) ([]string, *cutil.APIError) { flowReq := &flowv1.GetComponentsRequest{TargetSpec: baseSpec} workflowID := fmt.Sprintf("tray-resolve-by-slot-%s", common.RequestHash(flowReq)) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - } - - wfCtx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(wfCtx, workflowOptions, "GetTrays", flowReq) - if err != nil { - return nil, fmt.Errorf("execute GetTrays workflow: %w", err) - } - var resp flowv1.GetComponentsResponse - err = we.Get(wfCtx, &resp) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || errors.Is(err, context.DeadlineExceeded) || wfCtx.Err() != nil { - return nil, fmt.Errorf("GetTrays workflow timed out: %w", err) - } - return nil, fmt.Errorf("get GetTrays result: %w", err) + apiErr := common.ExecuteFlowGRPC( + ctx, stc, + flowv1.Flow_GetComponents_FullMethodName, + flowReq, &resp, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + "", + ) + if apiErr != nil { + return nil, apiErr } ids := make([]string, 0, len(resp.GetComponents())) @@ -236,34 +225,19 @@ func (gth GetTrayHandler) Handle(c echo.Context) error { } // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: fmt.Sprintf("tray-get-%s", trayStrID), - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "GetTray", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute GetTray workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to get Tray details", nil) - } - - // Get workflow result + // + // Concurrent identical reads do not coalesce here, unlike the tray reads + // below. Whether they should is a question about this endpoint rather than + // about its transport, so the policy crosses to the proxy unchanged. var flowResponse flowv1.GetComponentInfoResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, fmt.Sprintf("tray-get-%s", trayStrID), err, "Tray", "GetTray") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from GetTray workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to get Tray details: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetComponentInfoByID_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(fmt.Sprintf("tray-get-%s", trayStrID)), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -445,34 +419,19 @@ func (gath GetAllTrayHandler) Handle(c echo.Context) error { workflowID := fmt.Sprintf("tray-get-all-%s", common.QueryParamHash(hashValues)) // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "GetTrays", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute GetTrays workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to get Trays", nil) - } - - // Get workflow result + // + // As in GetTrayHandler, concurrent identical reads do not coalesce, and + // changing that is a decision about the endpoint rather than part of moving + // it onto the proxy. var flowResponse flowv1.GetComponentsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Tray", "GetTrays") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from GetTrays workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to get Trays: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_GetComponents_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED, + ) + if proxyErr != nil { + return proxyErr } components := flowResponse.GetComponents() @@ -657,35 +616,15 @@ func (vth ValidateTrayHandler) Handle(c echo.Context) error { } // Execute workflow - workflowOptions := tClient.StartWorkflowOptions{ - ID: fmt.Sprintf("tray-validate-%s", trayStrID), - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "ValidateRackComponents", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute ValidateComponents workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to validate Tray", nil) - } - - // Get workflow result var flowResponse flowv1.ValidateComponentsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, fmt.Sprintf("tray-validate-%s", trayStrID), err, "Tray", "ValidateRackComponents") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from ValidateComponents workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to validate Tray: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ValidateComponents_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(fmt.Sprintf("tray-validate-%s", trayStrID)), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -825,8 +764,8 @@ func (vtsh ValidateTraysHandler) Handle(c echo.Context) error { ids, resolveErr := resolveTrayIDsBySlot(ctx, stc, targetSpec, model.RackComponentSlotMatcher{SlotID: apiRequest.SlotID}) if resolveErr != nil { - logger.Error().Err(resolveErr).Msg("failed to resolve trays by slot") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Trays by slot", nil) + logger.Error().Err(resolveErr.Diagnosis()).Msg("failed to resolve trays by slot") + return cutil.NewAPIErrorResponse(c, resolveErr.Code, resolveErr.Message, nil) } if len(ids) == 0 { logger.Info().Msg("no trays match slot filter; returning empty validation result") @@ -842,35 +781,15 @@ func (vtsh ValidateTraysHandler) Handle(c echo.Context) error { workflowID := fmt.Sprintf("tray-validate-all-%s", common.QueryParamHash(apiRequest.QueryValues())) - workflowOptions := tClient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "ValidateRackComponents", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute ValidateComponents workflow") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to validate Trays", nil) - } - - // Get workflow result var flowResponse flowv1.ValidateComponentsResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return common.TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, "Tray", "ValidateRackComponents") - } - code, err := common.UnwrapWorkflowError(err) - logger.Error().Err(err).Msg("failed to get result from ValidateComponents workflow") - - return cutil.NewAPIErrorResponse(c, code, fmt.Sprintf("Failed to validate Trays: %s", err), nil) + proxyErr := common.ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_ValidateComponents_FullMethodName, + flowRequest, &flowResponse, + common.FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return proxyErr } // Convert to API model @@ -1127,8 +1046,8 @@ func (pctbh BatchUpdateTrayPowerStateHandler) Handle(c echo.Context) error { ids, resolveErr := resolveTrayIDsBySlot(ctx, stc, targetSpec, model.RackComponentSlotMatcher{SlotID: request.Filter.SlotID}) if resolveErr != nil { - logger.Error().Err(resolveErr).Msg("failed to resolve trays by slot") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Trays by slot", nil) + logger.Error().Err(resolveErr.Diagnosis()).Msg("failed to resolve trays by slot") + return cutil.NewAPIErrorResponse(c, resolveErr.Code, resolveErr.Message, nil) } if len(ids) == 0 { logger.Info().Msg("no trays match slot filter; returning empty task list") @@ -1391,8 +1310,8 @@ func (futbh BatchUpdateTrayFirmwareHandler) Handle(c echo.Context) error { ids, resolveErr := resolveTrayIDsBySlot(ctx, stc, targetSpec, model.RackComponentSlotMatcher{SlotID: request.Filter.SlotID}) if resolveErr != nil { - logger.Error().Err(resolveErr).Msg("failed to resolve trays by slot") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Trays by slot", nil) + logger.Error().Err(resolveErr.Diagnosis()).Msg("failed to resolve trays by slot") + return cutil.NewAPIErrorResponse(c, resolveErr.Code, resolveErr.Message, nil) } if len(ids) == 0 { logger.Info().Msg("no trays match slot filter; returning empty task list") diff --git a/rest-api/api/pkg/api/handler/tray_test.go b/rest-api/api/pkg/api/handler/tray_test.go index df174ff5ff..753e04dbb7 100644 --- a/rest-api/api/pkg/api/handler/tray_test.go +++ b/rest-api/api/pkg/api/handler/tray_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "strings" "testing" @@ -31,7 +32,9 @@ import ( "github.com/stretchr/testify/require" "github.com/uptrace/bun/extra/bundebug" oteltrace "go.opentelemetry.io/otel/trace" + temporalEnums "go.temporal.io/api/enums/v1" tmocks "go.temporal.io/sdk/mocks" + tp "go.temporal.io/sdk/temporal" ) func testTrayInitDB(t *testing.T) *cdb.Session { @@ -283,18 +286,8 @@ func TestGetTrayHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - if tt.mockComponent != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetComponentInfoResponse) - resp.Component = tt.mockComponent - }).Return(nil) - } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetComponentInfoResponse) - resp.Component = nil - }).Return(nil) - } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTray", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetComponentInfoResponse{Component: tt.mockComponent}) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_GetComponentInfoByID_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient // Build query string @@ -629,20 +622,15 @@ func TestGetAllTrayHandler_Handle(t *testing.T) { mockWorkflowRun.On("GetID").Return("test-workflow-id") // Always set up Get mock, even for error cases, as handler may still call it if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetComponentsResponse) - resp.Components = tt.mockResponse.Components - resp.Total = tt.mockResponse.Total - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetComponentsResponse{ + Components: tt.mockResponse.Components, + Total: tt.mockResponse.Total, + }) } else { - // For error cases, set up a mock that returns empty response - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.GetComponentsResponse) - resp.Components = []*flowv1.Component{} - resp.Total = 0 - }).Return(nil) + // For error cases, reply with an empty response + testFlowProxyReply(t, mockWorkflowRun, &flowv1.GetComponentsResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "GetTrays", mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_GetComponents_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient // Build query string @@ -844,23 +832,18 @@ func TestValidateTrayHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = tt.mockResponse.Diffs - resp.TotalDiffs = tt.mockResponse.TotalDiffs - resp.MissingCount = tt.mockResponse.MissingCount - resp.UnexpectedCount = tt.mockResponse.UnexpectedCount - resp.MismatchCount = tt.mockResponse.MismatchCount - resp.MatchCount = tt.mockResponse.MatchCount - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{ + Diffs: tt.mockResponse.Diffs, + TotalDiffs: tt.mockResponse.TotalDiffs, + MissingCount: tt.mockResponse.MissingCount, + UnexpectedCount: tt.mockResponse.UnexpectedCount, + MismatchCount: tt.mockResponse.MismatchCount, + MatchCount: tt.mockResponse.MatchCount, + }) } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = []*flowv1.ComponentDiff{} - resp.TotalDiffs = 0 - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "ValidateRackComponents", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_ValidateComponents_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient q := url.Values{} @@ -1137,23 +1120,18 @@ func TestValidateTraysHandler_Handle(t *testing.T) { mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") if tt.mockResponse != nil { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = tt.mockResponse.Diffs - resp.TotalDiffs = tt.mockResponse.TotalDiffs - resp.MissingCount = tt.mockResponse.MissingCount - resp.UnexpectedCount = tt.mockResponse.UnexpectedCount - resp.MismatchCount = tt.mockResponse.MismatchCount - resp.MatchCount = tt.mockResponse.MatchCount - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{ + Diffs: tt.mockResponse.Diffs, + TotalDiffs: tt.mockResponse.TotalDiffs, + MissingCount: tt.mockResponse.MissingCount, + UnexpectedCount: tt.mockResponse.UnexpectedCount, + MismatchCount: tt.mockResponse.MismatchCount, + MatchCount: tt.mockResponse.MatchCount, + }) } else { - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.ValidateComponentsResponse) - resp.Diffs = []*flowv1.ComponentDiff{} - resp.TotalDiffs = 0 - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.ValidateComponentsResponse{}) } - mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, "ValidateRackComponents", mock.Anything).Return(mockWorkflowRun, nil) + testFlowProxyDispatch(t, mockTemporalClient, mockWorkflowRun, flowv1.Flow_ValidateComponents_FullMethodName, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient q := url.Values{} @@ -1194,6 +1172,129 @@ func TestValidateTraysHandler_Handle(t *testing.T) { } } +// TestValidateTraysHandler_SlotFilter covers the slotId branch, which resolves +// trays through a second Flow call before validating them. Flow has no by-slot +// target shape, so this resolution is the only place a slot filter is applied, +// and it also decides what the endpoint reports when the site does not answer. +func TestValidateTraysHandler_SlotFilter(t *testing.T) { + e := echo.New() + dbSession := testTrayInitDB(t) + defer dbSession.Close() + + cfg := common.GetTestConfig() + tcfg, _ := cfg.GetTemporalConfig() + scp := sc.NewClientPool(tcfg) + + org := "test-org" + _, site, _ := testTraySetupTestData(t, dbSession, org) + providerUser := testTrayBuildUser(t, dbSession, "provider-user-validate-trays-slot", org, []string{authz.ProviderAdminRole}) + + handler := NewValidateTraysHandler(dbSession, nil, scp, cfg) + tracer := oteltrace.NewNoopTracerProvider().Tracer("test") + + const wantedSlot = 3 + matchedID := uuid.NewString() + componentAt := func(slotID int32, id string) *flowv1.Component { + return &flowv1.Component{ + Position: &flowv1.RackPosition{SlotId: slotID}, + Info: &flowv1.DeviceInfo{Id: &flowv1.UUID{Id: id}}, + } + } + + tests := []struct { + name string + components []*flowv1.Component + resolveErr error + expectedStatus int + expectedIDs []string + }{ + { + name: "trays at the slot are validated by component id", + components: []*flowv1.Component{componentAt(wantedSlot, matchedID), componentAt(7, uuid.NewString())}, + expectedStatus: http.StatusOK, + expectedIDs: []string{matchedID}, + }, + { + // Flow rejects an empty component target, so the handler answers + // without a validation call at all. + name: "no tray at the slot short circuits to an empty result", + components: []*flowv1.Component{componentAt(7, uuid.NewString())}, + expectedStatus: http.StatusOK, + }, + { + // The resolution runs inside the same request, so its timeout is + // the endpoint's timeout. Reporting 500 here would contradict both + // the 504 the spec declares and what the same endpoint answers when + // no slot filter is set. + name: "a resolution timeout stays a gateway timeout", + resolveErr: tp.NewTimeoutError(temporalEnums.TIMEOUT_TYPE_START_TO_CLOSE, nil), + expectedStatus: http.StatusGatewayTimeout, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockTemporalClient := &tmocks.Client{} + + resolveRun := &tmocks.WorkflowRun{} + resolveRun.On("GetID").Return("test-workflow-id") + if tt.resolveErr != nil { + resolveRun.Mock.On("Get", mock.Anything, mock.Anything).Return(tt.resolveErr) + } else { + testFlowProxyReply(t, resolveRun, &flowv1.GetComponentsResponse{Components: tt.components}) + } + testFlowProxyMethodDispatch(t, mockTemporalClient, resolveRun, flowv1.Flow_GetComponents_FullMethodName, nil) + + validateRun := &tmocks.WorkflowRun{} + validateRun.On("GetID").Return("test-workflow-id") + testFlowProxyReply(t, validateRun, &flowv1.ValidateComponentsResponse{MatchCount: 1}) + var validated flowv1.ValidateComponentsRequest + testFlowProxyMethodDispatch(t, mockTemporalClient, validateRun, flowv1.Flow_ValidateComponents_FullMethodName, + func(args mock.Arguments) { testFlowProxyRequest(t, args, &validated) }) + + scp.IDClientMap[site.ID.String()] = mockTemporalClient + + q := url.Values{} + q.Set("siteId", site.ID.String()) + q.Set("rackId", uuid.NewString()) + q.Set("slotId", strconv.Itoa(wantedSlot)) + path := fmt.Sprintf("/v2/org/%s/nico/tray/validation?%s", org, q.Encode()) + + req := httptest.NewRequest(http.MethodGet, path, nil) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName") + ec.SetParamValues(org) + ec.Set("user", providerUser) + ec.SetRequest(ec.Request().WithContext(context.WithValue(context.Background(), otelecho.TracerKey, tracer))) + + require.NoError(t, handler.Handle(ec)) + require.Equal(t, tt.expectedStatus, rec.Code, "body=%s", rec.Body.String()) + if tt.expectedStatus != http.StatusOK { + return + } + + var apiResult model.APIRackValidationResult + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &apiResult)) + + if len(tt.expectedIDs) == 0 { + assert.Nil(t, validated.GetTargetSpec(), "validation ran despite no tray matching the slot") + assert.Zero(t, apiResult.MatchCount) + return + } + + var gotIDs []string + for _, target := range validated.GetTargetSpec().GetComponents().GetTargets() { + gotIDs = append(gotIDs, target.GetId().GetId()) + } + assert.Equal(t, tt.expectedIDs, gotIDs) + assert.Equal(t, int32(1), apiResult.MatchCount) + }) + } +} + func TestUpdateTrayPowerStateHandler_Handle(t *testing.T) { e := echo.New() dbSession := testTrayInitDB(t) @@ -1291,12 +1392,7 @@ func TestUpdateTrayPowerStateHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1407,12 +1503,7 @@ func TestBatchUpdateTrayPowerStateHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1529,12 +1620,7 @@ func TestUpdateTrayFirmwareHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient @@ -1638,12 +1724,7 @@ func TestBatchUpdateTrayFirmwareHandler_Handle(t *testing.T) { mockTemporalClient := &tmocks.Client{} mockWorkflowRun := &tmocks.WorkflowRun{} mockWorkflowRun.On("GetID").Return("test-workflow-id") - mockWorkflowRun.Mock.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { - resp := args.Get(1).(*flowv1.SubmitTaskResponse) - if tt.mockTaskIDs != nil { - resp.TaskIds = tt.mockTaskIDs - } - }).Return(nil) + testFlowProxyReply(t, mockWorkflowRun, &flowv1.SubmitTaskResponse{TaskIds: tt.mockTaskIDs}) mockTemporalClient.Mock.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(mockWorkflowRun, nil) scp.IDClientMap[site.ID.String()] = mockTemporalClient diff --git a/rest-api/api/pkg/api/handler/util/common/common.go b/rest-api/api/pkg/api/handler/util/common/common.go index 76e74d0f6c..abb1186576 100644 --- a/rest-api/api/pkg/api/handler/util/common/common.go +++ b/rest-api/api/pkg/api/handler/util/common/common.go @@ -26,6 +26,7 @@ import ( tp "go.temporal.io/sdk/temporal" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" @@ -45,7 +46,6 @@ import ( cdbp "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/paginator" flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" swe "github.com/NVIDIA/infra-controller/rest-api/site-workflow/pkg/error" - "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" ) const ( @@ -2074,8 +2074,9 @@ func QueryParamHash(params url.Values) string { return fmt.Sprintf("%x", sha256.Sum256([]byte(strings.Join(sortedParams, "&"))))[:12] } -// ExecutePowerControlWorkflow determines the appropriate power control workflow based on state, -// executes it via Temporal, and returns the raw SubmitTaskResponse. +// ExecutePowerControlWorkflow determines the appropriate Flow power control +// method based on state, proxies it to the site, and returns the raw +// SubmitTaskResponse. // // ruleID, when non-nil and non-empty, pins the operation to a specific // Operation Rule (overrides Flow's default rule resolution). Must be a valid @@ -2092,13 +2093,13 @@ func ExecutePowerControlWorkflow( workflowID string, entityName string, ) (*flowv1.SubmitTaskResponse, error) { - var workflowName string - var flowRequest interface{} + var fullMethod string + var flowRequest proto.Message ruleUUID := GetFlowUUIDPtr(ruleID) switch state { case cam.PowerControlStateOn: - workflowName = "PowerOnRack" + fullMethod = flowv1.Flow_PowerOnRack_FullMethodName flowRequest = &flowv1.PowerOnRackRequest{ TargetSpec: targetSpec, Description: fmt.Sprintf("API power on %s", entityName), @@ -2106,7 +2107,7 @@ func ExecutePowerControlWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } case cam.PowerControlStateOff: - workflowName = "PowerOffRack" + fullMethod = flowv1.Flow_PowerOffRack_FullMethodName flowRequest = &flowv1.PowerOffRackRequest{ TargetSpec: targetSpec, Description: fmt.Sprintf("API power off %s", entityName), @@ -2114,7 +2115,7 @@ func ExecutePowerControlWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } case cam.PowerControlStateCycle: - workflowName = "PowerResetRack" + fullMethod = flowv1.Flow_PowerResetRack_FullMethodName flowRequest = &flowv1.PowerResetRackRequest{ TargetSpec: targetSpec, Description: fmt.Sprintf("API power cycle %s", entityName), @@ -2122,7 +2123,7 @@ func ExecutePowerControlWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } case cam.PowerControlStateForceOff: - workflowName = "PowerOffRack" + fullMethod = flowv1.Flow_PowerOffRack_FullMethodName flowRequest = &flowv1.PowerOffRackRequest{ TargetSpec: targetSpec, Forced: true, @@ -2131,7 +2132,7 @@ func ExecutePowerControlWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } case cam.PowerControlStateForceCycle: - workflowName = "PowerResetRack" + fullMethod = flowv1.Flow_PowerResetRack_FullMethodName flowRequest = &flowv1.PowerResetRackRequest{ TargetSpec: targetSpec, Forced: true, @@ -2143,39 +2144,22 @@ func ExecutePowerControlWorkflow( return nil, cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Invalid power control state: %s", state), nil) } - workflowOptions := tclient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, workflowName, flowRequest) - if err != nil { - logger.Error().Err(err).Msg(fmt.Sprintf("failed to execute %s workflow", workflowName)) - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to power control %s", entityName), nil) - } - var flowResponse flowv1.SubmitTaskResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return nil, TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, entityName, workflowName) - } - logger.Error().Err(err).Msg(fmt.Sprintf("failed to get result from %s workflow", workflowName)) - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to power control %s", entityName), nil) + proxyErr := ProxyFlowGRPC( + ctx, c, logger, stc, + fullMethod, + flowRequest, &flowResponse, + FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return nil, proxyErr } return &flowResponse, nil } -// ExecuteBringUpRackWorkflow builds a BringUpRackRequest, executes the BringUpRack -// workflow via Temporal, and returns the raw SubmitTaskResponse. +// ExecuteBringUpRackWorkflow builds a BringUpRackRequest, proxies it to Flow's +// BringUpRack, and returns the raw SubmitTaskResponse. // // ruleID, when non-nil and non-empty, pins the bring-up to a specific // Operation Rule. @@ -2198,39 +2182,22 @@ func ExecuteBringUpRackWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } - workflowOptions := tclient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "BringUpRack", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute BringUpRack workflow") - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to bring up %s", entityName), nil) - } - var flowResponse flowv1.SubmitTaskResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return nil, TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, entityName, "BringUpRack") - } - logger.Error().Err(err).Msg("failed to get result from BringUpRack workflow") - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to bring up %s", entityName), nil) + proxyErr := ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_BringUpRack_FullMethodName, + flowRequest, &flowResponse, + FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return nil, proxyErr } return &flowResponse, nil } -// ExecuteFirmwareUpdateWorkflow builds an UpgradeFirmwareRequest, executes the UpgradeFirmware -// workflow via Temporal, and returns the raw SubmitTaskResponse. +// ExecuteFirmwareUpdateWorkflow builds an UpgradeFirmwareRequest, proxies it to +// Flow's UpgradeFirmware, and returns the raw SubmitTaskResponse. // // targets, when non-empty, restricts the upgrade to the listed firmware // sub-parts within each targeted tray (e.g. ["bmc", "nvos"] for switch @@ -2263,32 +2230,15 @@ func ExecuteFirmwareUpdateWorkflow( OverrideReadinessCheck: overrideReadinessCheck, } - workflowOptions := tclient.StartWorkflowOptions{ - ID: workflowID, - WorkflowIDReusePolicy: temporalEnums.WORKFLOW_ID_REUSE_POLICY_ALLOW_DUPLICATE, - WorkflowIDConflictPolicy: temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, - WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, - TaskQueue: queue.SiteTaskQueue, - } - - ctx, cancel := context.WithTimeout(ctx, cutil.WorkflowContextTimeout) - defer cancel() - - we, err := stc.ExecuteWorkflow(ctx, workflowOptions, "UpgradeFirmware", flowRequest) - if err != nil { - logger.Error().Err(err).Msg("failed to execute UpgradeFirmware workflow") - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to upgrade firmware for %s", entityName), nil) - } - var flowResponse flowv1.SubmitTaskResponse - err = we.Get(ctx, &flowResponse) - if err != nil { - var timeoutErr *tp.TimeoutError - if errors.As(err, &timeoutErr) || err == context.DeadlineExceeded || ctx.Err() != nil { - return nil, TerminateWorkflowOnTimeOut(c, logger, stc, workflowID, err, entityName, "UpgradeFirmware") - } - logger.Error().Err(err).Msg("failed to get result from UpgradeFirmware workflow") - return nil, cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, fmt.Sprintf("Failed to upgrade firmware for %s", entityName), nil) + proxyErr := ProxyFlowGRPC( + ctx, c, logger, stc, + flowv1.Flow_UpgradeFirmware_FullMethodName, + flowRequest, &flowResponse, + FlowWorkflowID(workflowID), temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, + ) + if proxyErr != nil { + return nil, proxyErr } return &flowResponse, nil diff --git a/rest-api/api/pkg/api/handler/util/common/flowmutation_test.go b/rest-api/api/pkg/api/handler/util/common/flowmutation_test.go new file mode 100644 index 0000000000..a80a32f559 --- /dev/null +++ b/rest-api/api/pkg/api/handler/util/common/flowmutation_test.go @@ -0,0 +1,225 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package common + +import ( + "context" + "net/http" + "testing" + + "github.com/labstack/echo/v4" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + temporalEnums "go.temporal.io/api/enums/v1" + tclient "go.temporal.io/sdk/client" + tmocks "go.temporal.io/sdk/mocks" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + + cam "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/model" + "github.com/NVIDIA/infra-controller/rest-api/common/pkg/grpcproxy" + flowv1 "github.com/NVIDIA/infra-controller/rest-api/proto/flow/gen/v1" +) + +// proxiedCall is what a mutation helper handed to the proxy workflow. +type proxiedCall struct { + options tclient.StartWorkflowOptions + workflowName string + request grpcproxy.Request +} + +// newMutationProxyClient returns a Temporal client that captures the proxy call +// and answers it with reply, so a helper runs its success path. +func newMutationProxyClient(t *testing.T, reply proto.Message) (*tmocks.Client, *proxiedCall) { + t.Helper() + + replyJSON, err := protojson.Marshal(reply) + require.NoError(t, err) + + workflowRun := &tmocks.WorkflowRun{} + workflowRun.On("Get", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + out, ok := args.Get(1).(*grpcproxy.Response) + require.True(t, ok, "Get target is %T", args.Get(1)) + out.ResponseJSON = replyJSON + }).Return(nil) + + call := &proxiedCall{} + temporalClient := &tmocks.Client{} + temporalClient.On("ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + call.options = args.Get(1).(tclient.StartWorkflowOptions) + call.workflowName = args.Get(2).(string) + call.request = args.Get(3).(grpcproxy.Request) + }).Return(workflowRun, nil) + + return temporalClient, call +} + +func mutationTargetSpec(rackID string) *flowv1.OperationTargetSpec { + return &flowv1.OperationTargetSpec{ + Targets: &flowv1.OperationTargetSpec_Racks{ + Racks: &flowv1.RackTargets{ + Targets: []*flowv1.RackTarget{ + {Identifier: &flowv1.RackTarget_Id{Id: &flowv1.UUID{Id: rackID}}}, + }, + }, + }, + } +} + +// TestFlowMutationHelpersProxyRequests pins what the power, bring-up and +// firmware helpers send. Each covers a Flow method reached only through these +// helpers, so an incorrect method, workflow ID, conflict policy or request +// payload would otherwise surface as a misrouted mutation on a live site. +func TestFlowMutationHelpersProxyRequests(t *testing.T) { + const ( + rackID = "6f1b7c4e-9c2a-4d1e-8f3b-2a5c7d9e1b04" + ruleID = "b3d2c1a0-5e4f-4a3b-9c8d-7e6f5a4b3c2d" + workflowID = "rack-power-1" + entityName = "rack r1" + ) + ruleIDArg := ruleID + version := "1.2.3" + + cases := []struct { + name string + execute func(context.Context, echo.Context, tclient.Client) (*flowv1.SubmitTaskResponse, error) + wantFullMethod string + wantRequest proto.Message + }{ + { + name: "power on", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecutePowerControlWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), cam.PowerControlStateOn, &ruleIDArg, false, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_PowerOnRack_FullMethodName, + wantRequest: &flowv1.PowerOnRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Description: "API power on rack r1", + RuleId: &flowv1.UUID{Id: ruleID}, + }, + }, + { + name: "power off", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecutePowerControlWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), cam.PowerControlStateOff, nil, false, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_PowerOffRack_FullMethodName, + wantRequest: &flowv1.PowerOffRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Description: "API power off rack r1", + }, + }, + { + name: "power cycle", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecutePowerControlWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), cam.PowerControlStateCycle, nil, false, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_PowerResetRack_FullMethodName, + wantRequest: &flowv1.PowerResetRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Description: "API power cycle rack r1", + }, + }, + { + // Forced shares PowerOffRack with the unforced state, so the flag is + // the only thing separating them on the wire. + name: "force power off", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecutePowerControlWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), cam.PowerControlStateForceOff, nil, true, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_PowerOffRack_FullMethodName, + wantRequest: &flowv1.PowerOffRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Forced: true, + Description: "API force power off rack r1", + OverrideReadinessCheck: true, + }, + }, + { + name: "force power cycle", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecutePowerControlWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), cam.PowerControlStateForceCycle, nil, false, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_PowerResetRack_FullMethodName, + wantRequest: &flowv1.PowerResetRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Forced: true, + Description: "API force power cycle rack r1", + }, + }, + { + name: "bring up", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecuteBringUpRackWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), "API bring up rack r1", &ruleIDArg, true, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_BringUpRack_FullMethodName, + wantRequest: &flowv1.BringUpRackRequest{ + TargetSpec: mutationTargetSpec(rackID), + Description: "API bring up rack r1", + RuleId: &flowv1.UUID{Id: ruleID}, + OverrideReadinessCheck: true, + }, + }, + { + name: "firmware update", + execute: func(ctx context.Context, c echo.Context, stc tclient.Client) (*flowv1.SubmitTaskResponse, error) { + return ExecuteFirmwareUpdateWorkflow(ctx, c, zerolog.Nop(), stc, mutationTargetSpec(rackID), &version, []string{"bmc", "nvos"}, nil, false, workflowID, entityName) + }, + wantFullMethod: flowv1.Flow_UpgradeFirmware_FullMethodName, + wantRequest: &flowv1.UpgradeFirmwareRequest{ + TargetSpec: mutationTargetSpec(rackID), + TargetVersion: &version, + SubTargets: []string{"bmc", "nvos"}, + Description: "API firmware update rack r1", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + reply := &flowv1.SubmitTaskResponse{} + temporalClient, call := newMutationProxyClient(t, reply) + echoCtx, _ := newProxyEchoContext() + + got, err := tc.execute(context.Background(), echoCtx, temporalClient) + + require.NoError(t, err) + require.NotNil(t, got) + + assert.Equal(t, grpcproxy.Flow.WorkflowName, call.workflowName) + assert.Equal(t, tc.wantFullMethod, call.request.FullMethod) + + t.Run("coalesces retries onto the mutation already in flight", func(t *testing.T) { + assert.Equal(t, FlowWorkflowID(workflowID), call.options.ID) + assert.Equal(t, temporalEnums.WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING, call.options.WorkflowIDConflictPolicy) + }) + + t.Run("sends the expected request", func(t *testing.T) { + decoded := tc.wantRequest.ProtoReflect().New().Interface() + require.NoError(t, protojson.Unmarshal(call.request.RequestJSON, decoded)) + assert.Empty(t, call.request.EncryptedSecrets) + assert.True(t, proto.Equal(tc.wantRequest, decoded), "want %v, got %v", tc.wantRequest, decoded) + }) + }) + } +} + +// TestExecutePowerControlWorkflowRejectsUnknownState keeps an unroutable state +// from reaching Flow: the helper picks the method from it, so there is nothing +// to proxy. It answers 400 on the response itself rather than through the +// returned error, which stays nil once the write succeeds. +func TestExecutePowerControlWorkflowRejectsUnknownState(t *testing.T) { + temporalClient := &tmocks.Client{} + echoCtx, recorder := newProxyEchoContext() + + got, err := ExecutePowerControlWorkflow(context.Background(), echoCtx, zerolog.Nop(), temporalClient, mutationTargetSpec("6f1b7c4e-9c2a-4d1e-8f3b-2a5c7d9e1b04"), "hibernate", nil, false, "rack-power-1", "rack r1") + + assert.Nil(t, got) + assert.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, recorder.Code) + assert.Contains(t, recorder.Body.String(), "Invalid power control state: hibernate") + temporalClient.AssertNotCalled(t, "ExecuteWorkflow", mock.Anything, mock.Anything, mock.Anything, mock.Anything) +} From 38c248988a693d1b4621134e3d7b45d8b46df2e1 Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:56:06 -0700 Subject: [PATCH 5/8] docs(rest-api): record the proxy as the dispatch path for Flow endpoints Points contributors at ProxyFlowGRPC instead of a bespoke workflow per method, and states in the skill why a lost result leaves the execution running rather than terminating it. Signed-off-by: Kun Zhao --- rest-api/AGENTS.md | 8 ++- rest-api/skills/rest-flow-grpc-proxy/SKILL.md | 55 +++++++++++++++---- 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/rest-api/AGENTS.md b/rest-api/AGENTS.md index 2c3e658824..cd872015ea 100644 --- a/rest-api/AGENTS.md +++ b/rest-api/AGENTS.md @@ -274,9 +274,11 @@ main patterns: - Flow-backed inventory and task APIs use Flow request/response protobufs in the API model layer and keep target-shape helpers next to the model or handler that owns the REST shape. Use Rack, Tray, Task, and Task Rule as references. - Thin unary Flow pass-throughs should use `handler/util/common.ExecuteFlowGRPC` - rather than a bespoke Temporal workflow per method. Switching an existing - endpoint over spans releases; see the skill for the required order. + Every Flow-backed endpoint dispatches through the generic proxy, so use + `handler/util/common.ProxyFlowGRPC` (or `ExecuteFlowGRPC` where the caller + must return a plain `error`) rather than adding a bespoke Temporal workflow + per method. Retiring the bespoke workflows spans releases; see the skill for + the required order. - Curated REST endpoints that call NICo Core `forge.Forge` unary methods should use `handler/util/common.ExecuteCoreGRPC` with a typed protobuf request. Do not create a bespoke Temporal workflow for a simple unary Core call. BMC diff --git a/rest-api/skills/rest-flow-grpc-proxy/SKILL.md b/rest-api/skills/rest-flow-grpc-proxy/SKILL.md index abee9a7e86..a708443899 100644 --- a/rest-api/skills/rest-flow-grpc-proxy/SKILL.md +++ b/rest-api/skills/rest-flow-grpc-proxy/SKILL.md @@ -36,8 +36,18 @@ endpoints that need to call on-site Flow through the generic Flow gRPC proxy. `ExecuteFlowGRPC` requires the caller to supply: 1. `workflowID` — deterministic for read/list dedup, fresh UUID for creates. -2. `conflictPolicy` — typically `WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING` - for Flow handlers that coalesce identical in-flight requests. +2. `conflictPolicy` — the two travel together. A deterministic ID takes + `WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING` so identical in-flight requests + coalesce onto one Flow call; a fresh ID takes + `WORKFLOW_ID_CONFLICT_POLICY_UNSPECIFIED`, matching `ExecuteCoreGRPC`, whose + IDs are always fresh. + +Two helpers in `common` carry this for every migrated handler: +`common.FlowWorkflowID` applies the transport namespace, and +`common.ProxyFlowGRPC` dispatches and renders the failure as an Echo response. +Use `ExecuteFlowGRPC` directly only in helpers that hand the error back to a +caller instead of rendering a response, as `resolveTrayIDsBySlot` does; return +the `*cutil.APIError` unwrapped so the status the proxy chose survives. Kinds of ID derivation that must stay in the handler, using the TaskRun endpoints as the worked example: @@ -69,6 +79,14 @@ answers later than that cannot deliver its answer. Keep `api/internal/server` guards the outer bound. Raising the on-site budget means raising the server and load-balancer timeouts first. +Because the workflow timeout sits inside the caller's context timeout, a timeout +that Temporal itself reports comes from an execution that has closed. The ladder +does not cover the other way a caller loses its result: `wfCtx` derives from the +request context, so a client disconnect or a shorter upstream deadline can end +the wait while the execution runs on. `wfCtx.Err()` is the only evidence that +this happened, which is why a `context.DeadlineExceeded` in the workflow result +alone does not classify as it — it can come from inside the execution. + ## Before Coding Confirm these details before editing: @@ -81,9 +99,17 @@ Confirm these details before editing: - Workflow ID derivation and conflict policy for this call. - Secret fields that must not appear in Temporal history (top-level protojson field names). -- Whether timeout should terminate the workflow (`TerminateWorkflowOnTimeOut`) - when `ExecuteFlowGRPC` returns `504`, which is what the bespoke TaskRun - workflows do today. +- Whether the call fits in the proxy's budget. The activity is cut off at + `grpcproxy.ActivityStartToCloseTimeout`, currently 40s. The bespoke workflows + declared 2 minutes, and bring-up and firmware 5 minutes, but every Flow caller + bounded itself with the 50s `cutil.WorkflowContextTimeout`, so no declared + budget was reachable. Migrating still narrows the window: a Flow call that took + 41-49s used to succeed and now fails. +- Whether the call tolerates losing an activity retry. `InvokeFlowGRPC` runs the + activity with `MaximumAttempts: 1`, so a transient Flow error surfaces to the + client instead of being retried. Bespoke workflows that allowed a second + attempt lose it on migration, and the retry policy lives in the site workflow, + so making it configurable takes another agent release to take effect. ## Implementation Workflow @@ -94,8 +120,17 @@ Confirm these details before editing: respProtoOrNil, workflowID, conflictPolicy, siteIDSecretKey, secretFields...)`. Passing `secretFields` requires a non-empty `siteIDSecretKey`; the helper rejects the combination rather than send the fields unredacted. -4. On `StatusGatewayTimeout`, call `TerminateWorkflowOnTimeOut` with the same - `workflowID` when matching existing TaskRun UX. +4. Return `StatusGatewayTimeout` as it comes. Do not call + `TerminateWorkflowOnTimeOut`. When Temporal reports the timeout the execution + has already closed, and terminating a closed execution fails and reports a + data desync that did not happen. When the caller stopped waiting instead, the + execution may still be running, and terminating it still does not help: the + activity does not heartbeat, so Temporal cannot deliver cancellation while + its Flow RPC is in progress, and terminating only the workflow discards the + result without stopping that RPC. For a deterministic ID with `USE_EXISTING` + it also frees the ID, so a retried request starts a duplicate mutation + instead of attaching to the call already in flight. An abandoned execution + stays bounded by the 45s and 40s timeouts. 5. Return a curated REST response. Do not expose Flow protobufs or secret fields directly unless the API contract already does. 6. For a new public REST endpoint, register the route and update OpenAPI. For a @@ -120,6 +155,6 @@ wait for every site to run that agent, and switch handlers in a later release. Retire the workflow a migration replaces in a third release, once no supported cloud release still submits it. -No handler dispatches through `ExecuteFlowGRPC` yet; -`rest-api/api/pkg/api/handler/taskrun.go` is the first migration and still uses -its per-method workflows. +When adding a Flow endpoint, reach for the proxy: there is no longer a bespoke +Flow workflow to copy, and adding one would reintroduce the per-method +registration this replaced. From fd679a6063d8e191890d7b6940786aaa367c8e84 Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:58:16 -0700 Subject: [PATCH 6/8] docs(rest-api): declare 504 on the Flow-backed operations The proxy reports a lost result as 504 where the bespoke workflows reported 500, so the 35 affected operations now document it. GatewayTimeoutError tells a client the two cases apart: a read is safe to retry, while a mutation may still be running at the site and should be polled first, because retrying into a live call attaches to it rather than starting a second one. Signed-off-by: Kun Zhao --- rest-api/openapi/spec.yaml | 88 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/rest-api/openapi/spec.yaml b/rest-api/openapi/spec.yaml index fbb9ebcec5..4632a5b752 100644 --- a/rest-api/openapi/spec.yaml +++ b/rest-api/openapi/spec.yaml @@ -10694,6 +10694,8 @@ paths: description: Pagination result in JSON format '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/{id}': @@ -10772,6 +10774,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/validation': @@ -10867,6 +10871,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/{id}/validation': @@ -10955,6 +10961,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/power': @@ -11010,6 +11018,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/{id}/power': @@ -11071,6 +11081,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/firmware': @@ -11122,6 +11134,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/{id}/firmware': @@ -11176,6 +11190,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/bringup': @@ -11227,6 +11243,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/rack/{id}/bringup': @@ -11281,6 +11299,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/task/{id}': @@ -11337,6 +11357,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task '/v2/org/{org}/nico/task/{id}/cancel': @@ -11414,6 +11436,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task '/v2/org/{org}/nico/task/rule': @@ -11454,6 +11478,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rule get: @@ -11510,6 +11536,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rule '/v2/org/{org}/nico/task/rule/{id}': @@ -11557,6 +11585,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rule patch: @@ -11587,6 +11617,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rule delete: @@ -11617,6 +11649,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rule '/v2/org/{org}/nico/task/run': @@ -11660,6 +11694,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run get: @@ -11736,6 +11772,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}': @@ -11791,6 +11829,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}/target': @@ -11892,6 +11932,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}/pause': @@ -11941,6 +11983,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}/resume': @@ -11990,6 +12034,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}/advance': @@ -12040,6 +12086,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/task/run/{id}/cancel': @@ -12089,6 +12137,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Task Run '/v2/org/{org}/nico/rack/{id}/task': @@ -12174,6 +12224,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Rack '/v2/org/{org}/nico/tray': @@ -12309,6 +12361,8 @@ paths: description: Pagination result in JSON format '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/{id}': @@ -12370,6 +12424,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/validation': @@ -12469,6 +12525,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/{id}/validation': @@ -12523,6 +12581,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/power': @@ -12580,6 +12640,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/{id}/power': @@ -12641,6 +12703,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/firmware': @@ -12695,6 +12759,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/{id}/firmware': @@ -12749,6 +12815,8 @@ paths: $ref: '#/components/responses/ValidationError' '403': $ref: '#/components/responses/ForbiddenError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/tray/{id}/task': @@ -12834,6 +12902,8 @@ paths: $ref: '#/components/responses/ForbiddenError' '404': $ref: '#/components/responses/NotFoundError' + '504': + $ref: '#/components/responses/GatewayTimeoutError' tags: - Tray '/v2/org/{org}/nico/ipxe-template': @@ -28267,6 +28337,24 @@ components: source: nico message: Error handling API request data: null + GatewayTimeoutError: + description: |- + The Site did not return a result within the request budget allocated to + Site communication within the request cycle. This can occur if the Site + Controller is down or under heavy request load. + + Operations are in general safe to retry. In rare cases, operations that + create or update resources may succeed even if timeout is returned. + content: + application/json: + schema: + $ref: '#/components/schemas/NICoAPIError' + examples: + example-1: + value: + source: nico + message: Flow proxy request timed out + data: null NotFoundError: description: Error response when requested object is not found content: From c6574c88f5f24101d36a222fc5aac143e1a7a600 Mon Sep 17 00:00:00 2001 From: Kun Zhao Date: Tue, 11 Aug 2026 10:59:02 -0700 Subject: [PATCH 7/8] chore(rest-api): regenerate openapi docs + sdk Covers the 504 declarations added to the Flow-backed operations. Signed-off-by: Kun Zhao --- rest-api/docs/index.html | 562 ++++++++++++++++++++++++-- rest-api/sdk/standard/api_rack.go | 121 ++++++ rest-api/sdk/standard/api_rule.go | 55 +++ rest-api/sdk/standard/api_task.go | 22 + rest-api/sdk/standard/api_task_run.go | 88 ++++ rest-api/sdk/standard/api_tray.go | 99 +++++ 6 files changed, 911 insertions(+), 36 deletions(-) diff --git a/rest-api/docs/index.html b/rest-api/docs/index.html index 2dc4559841..74843b4d76 100644 --- a/rest-api/docs/index.html +++ b/rest-api/docs/index.html @@ -12326,9 +12326,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

Whether the component is considered leaking coolant

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Rack

Get a Rack by ID.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12404,9 +12418,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "name": "Rack-01",
  • "manufacturer": "Dell",
  • "model": "PowerEdge R750",
  • "serialNumber": "SN-RACK-001",
  • "description": "Primary compute rack",
  • "location": {
    },
  • "components": [
    ]
}

Validate Racks

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "name": "Rack-01",
  • "manufacturer": "Dell",
  • "model": "PowerEdge R750",
  • "serialNumber": "SN-RACK-001",
  • "description": "Primary compute rack",
  • "location": {
    },
  • "components": [
    ]
}

Validate Racks

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
Example
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 10
}

Validate a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/validation

Response samples

Content type
application/json
Example
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 10
}

Validate a Rack

Validate a Rack's components by comparing expected vs actual state.

@@ -12496,9 +12538,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
Example
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 5
}

Power control Racks

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}/validation

Response samples

Content type
application/json
Example
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 5
}

Power control Racks

Power control Racks with optional filters. If no filter is specified, targets all racks in the Site.

@@ -12536,9 +12592,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "off"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Power control a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/power

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "off"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Power control a Rack

Power control a Rack identified by Rack UUID.

@@ -12586,9 +12656,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update Racks

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}/power

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update Racks

Update firmware on Racks with optional name filter. If no filter is specified, targets all racks in the Site.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12624,9 +12708,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/firmware

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update a Rack

Update firmware on a Rack identified by Rack UUID.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12710,9 +12808,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "version": "24.11.0"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Bring up Racks

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}/firmware

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "version": "24.11.0"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Bring up Racks

Bring up Racks with optional name filter. If no filter is specified, targets all racks in the Site.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12746,9 +12858,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Bring up a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/bringup

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Bring up a Rack

Bring up a Rack identified by Rack UUID.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12778,9 +12904,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Retrieve all Tasks for a Rack

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}/bringup

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Retrieve all Tasks for a Rack

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
[
  • {
    }
]

Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/rack/{id}/task

Response samples

Content type
application/json
[
  • {
    }
]

Tray

Tray represents a component within a Rack.

Retrieve all Trays

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

ID of the rack this tray belongs to

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Tray

Get a Tray by ID.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -12996,9 +13164,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "id": "660e8400-e29b-41d4-a716-446655440001",
  • "componentId": "fm100ht4v4mce2qstjnl8970nnj3ie6ecek4mtjn27pea4kre5gsa49jg0g",
  • "type": "compute",
  • "name": "compute-tray-1",
  • "manufacturer": "NVIDIA",
  • "model": "GB200",
  • "serialNumber": "TSN001",
  • "description": "Compute tray in slot 1",
  • "firmwareVersion": "2.1.0",
  • "powerState": "on",
  • "position": {
    },
  • "rackId": "550e8400-e29b-41d4-a716-446655440000"
}

Validate Trays

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/{id}

Response samples

Content type
application/json
{
  • "id": "660e8400-e29b-41d4-a716-446655440001",
  • "componentId": "fm100ht4v4mce2qstjnl8970nnj3ie6ecek4mtjn27pea4kre5gsa49jg0g",
  • "type": "compute",
  • "name": "compute-tray-1",
  • "manufacturer": "NVIDIA",
  • "model": "GB200",
  • "serialNumber": "TSN001",
  • "description": "Compute tray in slot 1",
  • "firmwareVersion": "2.1.0",
  • "powerState": "on",
  • "position": {
    },
  • "rackId": "550e8400-e29b-41d4-a716-446655440000"
}

Validate Trays

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 10
}

Validate a Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/validation

Response samples

Content type
application/json
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 10
}

Validate a Tray

Validate a Tray by comparing expected vs actual state.

@@ -13098,9 +13294,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 5
}

Power control Trays

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/{id}/validation

Response samples

Content type
application/json
{
  • "diffs": [ ],
  • "totalDiffs": 0,
  • "missingCount": 0,
  • "unexpectedCount": 0,
  • "mismatchCount": 0,
  • "matchCount": 5
}

Power control Trays

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Power control a Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/power

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Power control a Tray

Power control a Tray identified by Tray UUID.

@@ -13210,9 +13434,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update Trays

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/{id}/power

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "state": "on"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update Trays

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update a Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/firmware

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Firmware update a Tray

Update firmware on a Tray identified by Tray UUID.

Org must have an Infrastructure Provider entity. User must have authorization role with PROVIDER_ADMIN suffix.

@@ -13408,9 +13660,23 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "version": "24.11.0"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Retrieve all Tasks for a Tray

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/{id}/firmware

Request samples

Content type
application/json
Example
{
  • "siteId": "550e8400-e29b-41d4-a716-446655440000",
  • "version": "24.11.0"
}

Response samples

Content type
application/json
{
  • "taskIds": [
    ]
}

Retrieve all Tasks for a Tray

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
[
  • {
    }
]

Task

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/tray/{id}/task

Response samples

Content type
application/json
[
  • {
    }
]

Task

Task represents an asynchronous, site-scoped operation (for example firmware update, power state change, or rack bring-up). Tasks are created when operations run against Racks, Trays, or other components. Endpoints in this tag retrieve or cancel a Task by ID; list Tasks for a Rack or Tray under the Rack and Tray tags.

Retrieve a Task

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "status": "Running",
  • "description": "Power on rack components",
  • "message": "Processing 3 of 5 components"
}

Cancel a Task

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/{id}

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "status": "Running",
  • "description": "Power on rack components",
  • "message": "Processing 3 of 5 components"
}

Cancel a Task

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "660e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "status": "Terminated",
  • "description": "Power on rack components",
  • "message": "Cancelled by user"
}

Rule

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/{id}/cancel

Request samples

Content type
application/json
{
  • "siteId": "660e8400-e29b-41d4-a716-446655440000"
}

Response samples

Content type
application/json
{
  • "id": "550e8400-e29b-41d4-a716-446655440000",
  • "status": "Terminated",
  • "description": "Power on rack components",
  • "message": "Cancelled by user"
}

Rule

Operation Rule defines, per Site, how a particular operation (for example PowerControl / power_on or FirmwareControl / upgrade) should be executed against a set of components: ordered execution stages, per-component-type concurrency, pre / main / post actions, timeouts, and retry policy. Rules are reusable templates owned by Flow; this tag exposes CRUD (POST, GET, PATCH, DELETE) over them.

Create an Operation Rule

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    }
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    },
  • "isDefault": true,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z"
}

List Operation Rules

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/rule

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    }
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    },
  • "isDefault": true,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z"
}

List Operation Rules

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve an Operation Rule

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/rule

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve an Operation Rule

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    },
  • "isDefault": true,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z"
}

Update an Operation Rule

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/rule/{id}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "ruleDefinition": {
    },
  • "isDefault": true,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z"
}

Update an Operation Rule

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "ruleDefinition": {
    }
}

Response samples

Content type
application/json
{
  • "source": "nico",
  • "message": "Error validating request data",
  • "data": {
    }
}

Delete an Operation Rule

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/rule/{id}

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "ruleDefinition": {
    }
}

Response samples

Content type
application/json
{
  • "source": "nico",
  • "message": "Error validating request data",
  • "data": {
    }
}

Delete an Operation Rule

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "source": "nico",
  • "message": "Error validating request data",
  • "data": {
    }
}

Task Run

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/rule/{id}

Response samples

Content type
application/json
{
  • "source": "nico",
  • "message": "Error validating request data",
  • "data": {
    }
}

Task Run

A Task Run is a phased, policy-gated execution of one operation (currently firmware) across many Racks. A Task Run narrows a candidate set of Racks with an optional selector, divides the selected Racks into phases, and drives one execution target per Rack; each target in turn drives at most one Task. Safety gates pause the Task Run when failures exceed a threshold, and phase gates hold each phase until an operator advances it. This tag exposes creation, retrieval, target listing, and the pause / resume / advance / cancel lifecycle actions; drill into per-Rack execution detail via the Task tag using each target's taskId.

Create a Task Run

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "selector": {
    },
  • "options": {
    },
  • "operation": {
    }
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Retrieve all Task Runs

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "name": "string",
  • "description": "string",
  • "selector": {
    },
  • "options": {
    },
  • "operation": {
    }
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Retrieve all Task Runs

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when request data cannot be validated

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Task Run

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run

Response samples

Content type
application/json
[
  • {
    }
]

Retrieve a Task Run

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Retrieve all Task Run Targets

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Retrieve all Task Run Targets

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Response samples

Content type
application/json
[
  • {
    }
]

Pause a Task Run

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}/target

Response samples

Content type
application/json
[
  • {
    }
]

Pause a Task Run

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Resume a Task Run

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}/pause

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Resume a Task Run

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Advance a Task Run to its next phase

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}/resume

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Advance a Task Run to its next phase

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "expectedPhaseIndex": 0
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Cancel a Task Run

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}/advance

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "expectedPhaseIndex": 0
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Cancel a Task Run

Typical API Call Flow for Tenant " class="sc-iJSMbW sc-cBEgGa sc-ciCrSJ fiNpIH dNfUH dDDioG">

Error response when user is not authorized to call an endpoint or retrieve/modify objects

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "reason": "string"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Network Security Group

https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/task/run/{id}/cancel

Request samples

Content type
application/json
{
  • "siteId": "60189e9c-7d12-438c-b9ca-6998d9c364b1",
  • "reason": "string"
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "string",
  • "description": "string",
  • "operationType": "PowerControl",
  • "operationCode": "string",
  • "status": "Unknown",
  • "statusReason": "Unknown",
  • "statusMessage": "string",
  • "totalPhases": 0,
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z",
  • "started": "2019-08-24T14:15:22Z",
  • "finished": "2019-08-24T14:15:22Z",
  • "stats": {
    }
}

Network Security Group

Typical API Call Flow for Tenant