From b4c507e6e618f94eb158ecd15307703c7cd24bca Mon Sep 17 00:00:00 2001 From: Chet Nichols III Date: Fri, 15 May 2026 10:21:12 -0700 Subject: [PATCH] chore: Apply the new FromProto/ToProto modeling to InfiniBandPartition Brings `InfiniBandPartition` in line with the proto-modeling changes. The handler keeps its REST surface; Create and Update route through `ToProto` on the API request types and the entity gains the full round-trip plus a delete-request builder. The entity also picks up the typed `Labels` field per the `InstanceType` reference in #540 -- it reaches `Labels.ToProto()` / `Labels.FromProto(...)` directly. Primary callouts are: - API request side: `APIInfiniBandPartitionCreateRequest.ToProto(ibp)` and `APIInfiniBandPartitionUpdateRequest.ToProto(ibp)`; both source corresponding fields via `ibp.ToProto()`. - Entity side: `InfiniBandPartition.ToProto`, `FromProto`, and `ToDeletionRequestProto`. There's also a new `InfiniBandPartition.Validate()` so site-driven `FromProto` callers can do `FromProto` + `Validate` -- mirroring the MachineCapability pattern (which went in at #569). - `InfiniBandPartitionStatus` is now a typed string with its own `.FromProto(state)` and `.Message()` methods, replacing the `InfiniBandPartitionStatusFromProto(state)` helper that split a single proto state into `(status, message)`. The workflow activity caller is just a two-liner now. - Handler simplified to one-liners, and the inline `cwssaws` / `model/util` imports drop. As part of the new typed `InfiniBandPartitionStatus` (and the already-typed `Labels`), the typing propagates through the API and DAO layers, but nothing changes on the wire -- JSON serialization of `type X string` is identical to `string`, the OpenAPI spec still just says "string", and the SDK is unaffected. Tests added! Signed-off-by: Chet Nichols III --- .../api/handler/infinibandinterface_test.go | 4 +- api/pkg/api/handler/infinibandpartition.go | 51 +--- .../api/handler/infinibandpartition_test.go | 10 +- api/pkg/api/handler/instance_test.go | 18 +- api/pkg/api/handler/instancebatch_test.go | 2 +- api/pkg/api/handler/nvlinkinterface_test.go | 2 +- api/pkg/api/model/infinibandpartition.go | 57 +++- api/pkg/api/model/infinibandpartition_test.go | 66 +++++ db/pkg/db/model/helper_test.go | 2 +- db/pkg/db/model/infinibandinterface_test.go | 22 +- db/pkg/db/model/infinibandpartition.go | 264 +++++++++++++++-- db/pkg/db/model/infinibandpartition_test.go | 273 +++++++++++++++++- .../infinibandpartition.go | 33 +-- workflow/pkg/util/testing.go | 2 +- 14 files changed, 659 insertions(+), 147 deletions(-) diff --git a/api/pkg/api/handler/infinibandinterface_test.go b/api/pkg/api/handler/infinibandinterface_test.go index e3a233dcf..bd478893a 100644 --- a/api/pkg/api/handler/infinibandinterface_test.go +++ b/api/pkg/api/handler/infinibandinterface_test.go @@ -106,7 +106,7 @@ func TestGetAllInfiniBandInterface_Handle(t *testing.T) { ibPartitions := []*cdbm.InfiniBandPartition{} for i := 0; i < 3; i++ { - ibPartition := testBuildIBPartition(t, dbSession, fmt.Sprintf("test-InfiniBandPartition-%d", i), tn1.Org, st1, tn1, nil, cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibPartition := testBuildIBPartition(t, dbSession, fmt.Sprintf("test-InfiniBandPartition-%d", i), tn1.Org, st1, tn1, nil, cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibPartition) ibPartitions = append(ibPartitions, ibPartition) } @@ -743,7 +743,7 @@ func TestGetAllInstanceInfiniBandInterfaceHandler_Handle(t *testing.T) { ibps := []*cdbm.InfiniBandPartition{} for i := 0; i < 25; i++ { - ibp1 := testBuildIBPartition(t, dbSession, "test-infiniband-partition-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-infiniband-partition-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ibps = append(ibps, ibp1) } diff --git a/api/pkg/api/handler/infinibandpartition.go b/api/pkg/api/handler/infinibandpartition.go index aea478c5e..5ae1a30de 100644 --- a/api/pkg/api/handler/infinibandpartition.go +++ b/api/pkg/api/handler/infinibandpartition.go @@ -23,7 +23,6 @@ import ( "github.com/NVIDIA/infra-controller-rest/api/internal/config" "github.com/NVIDIA/infra-controller-rest/api/pkg/api/handler/util/common" "github.com/NVIDIA/infra-controller-rest/api/pkg/api/model" - "github.com/NVIDIA/infra-controller-rest/api/pkg/api/model/util" "github.com/NVIDIA/infra-controller-rest/api/pkg/api/pagination" sc "github.com/NVIDIA/infra-controller-rest/api/pkg/client/site" auth "github.com/NVIDIA/infra-controller-rest/auth/pkg/authorization" @@ -33,7 +32,6 @@ import ( "github.com/NVIDIA/infra-controller-rest/db/pkg/db/paginator" swe "github.com/NVIDIA/infra-controller-rest/site-workflow/pkg/error" - cwssaws "github.com/NVIDIA/infra-controller-rest/workflow-schema/schema/site-agent/workflows/v1" "github.com/NVIDIA/infra-controller-rest/workflow/pkg/queue" ) @@ -228,7 +226,7 @@ func (cibph CreateInfiniBandPartitionHandler) Handle(c echo.Context) error { } // create the status detail record - newSSD, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), *cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending), + newSSD, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), string(cdbm.InfiniBandPartitionStatusPending), cdb.GetStrPtr("received InfiniBand Partition creation request, pending")) if derr != nil { logger.Error().Err(derr).Msg("error creating Status Detail DB entry") @@ -247,13 +245,7 @@ func (cibph CreateInfiniBandPartitionHandler) Handle(c echo.Context) error { return cutil.NewAPIError(http.StatusInternalServerError, "Failed to retrieve client for Site", nil) } - createIBPRequest := &cwssaws.IBPartitionCreationRequest{ - Id: &cwssaws.IBPartitionId{Value: ibp.ID.String()}, - Config: &cwssaws.IBPartitionConfig{ - Name: ibp.Name, - TenantOrganizationId: orgTenant.Org, - }, - } + createIBPRequest := apiRequest.ToProto(ibp) workflowOptions := temporalClient.StartWorkflowOptions{ ID: "infiniband-partition-create-" + ibp.ID.String(), @@ -261,20 +253,6 @@ func (cibph CreateInfiniBandPartitionHandler) Handle(c echo.Context) error { WorkflowExecutionTimeout: cutil.WorkflowExecutionTimeout, } - // InfiniBand Partition metadata info - metadata := &cwssaws.Metadata{ - Name: ibp.Name, - Description: "", - } - - if ibp.Description != nil { - metadata.Description = *ibp.Description - } - - metadata.Labels = util.ProtobufLabelsFromAPILabels(ibp.Labels) - - createIBPRequest.Metadata = metadata - logger.Info().Msg("triggering InfiniBand Partition create workflow") // Add context deadlines @@ -464,7 +442,7 @@ func (gaibph GetAllInfiniBandPartitionHandler) Handle(c echo.Context) error { statusQuery := c.QueryParam("status") if statusQuery != "" { gaibph.tracerSpan.SetAttribute(handlerSpan, attribute.String("status", statusQuery), logger) - _, ok := cdbm.InfiniBandPartitionStatusMap[statusQuery] + _, ok := cdbm.InfiniBandPartitionStatusMap[cdbm.InfiniBandPartitionStatus(statusQuery)] if !ok { logger.Warn().Msg(fmt.Sprintf("invalid value in status query: %v", statusQuery)) return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Invalid Status value in query", nil) @@ -847,19 +825,7 @@ func (uibph UpdateInfiniBandPartitionHandler) Handle(c echo.Context) error { return cutil.NewAPIError(http.StatusInternalServerError, "Failed to retrieve client for Site", nil) } - updateIBPRequest := &cwssaws.IBPartitionUpdateRequest{ - Id: &cwssaws.IBPartitionId{Value: ibp.ID.String()}, - Config: &cwssaws.IBPartitionConfig{ - Name: uipb.Name, - TenantOrganizationId: orgTenant.Org, - }, - } - metadata := &cwssaws.Metadata{Name: uipb.Name} - if uipb.Description != nil { - metadata.Description = *uipb.Description - } - metadata.Labels = util.ProtobufLabelsFromAPILabels(uipb.Labels) - updateIBPRequest.Metadata = metadata + updateIBPRequest := apiRequest.ToProto(uipb) workflowOptions := temporalClient.StartWorkflowOptions{ ID: "infiniband-partition-update-" + ibp.ID.String(), @@ -1065,12 +1031,13 @@ func (dibph DeleteInfiniBandPartitionHandler) Handle(c echo.Context) error { err = cdb.WithTx(ctx, dibph.dbSession, func(tx *cdb.Tx) error { // Update InfiniBand Partition and set status to Deleting + deletingStatus := cdbm.InfiniBandPartitionStatusDeleting if _, derr := ibpDAO.Update( ctx, tx, cdbm.InfiniBandPartitionUpdateInput{ InfiniBandPartitionID: ibp.ID, - Status: cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusDeleting), + Status: &deletingStatus, }, ); derr != nil { logger.Error().Err(derr).Msg("error updating InfiniBand Partition in DB") @@ -1078,7 +1045,7 @@ func (dibph DeleteInfiniBandPartitionHandler) Handle(c echo.Context) error { } // Create status detail - if _, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), *cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusDeleting), + if _, derr := sdDAO.CreateFromParams(ctx, tx, ibp.ID.String(), string(deletingStatus), cdb.GetStrPtr("Received request for deletion, pending processing")); derr != nil { logger.Error().Err(derr).Msg("error creating Status Detail DB entry") return cutil.NewAPIError(http.StatusInternalServerError, "Failed to create Status Detail for InfiniBand Partition deletion", nil) @@ -1091,9 +1058,7 @@ func (dibph DeleteInfiniBandPartitionHandler) Handle(c echo.Context) error { return cutil.NewAPIError(http.StatusInternalServerError, "Failed to retrieve client for Site", nil) } - deleteIBPRequest := &cwssaws.IBPartitionDeletionRequest{ - Id: &cwssaws.IBPartitionId{Value: ibp.ID.String()}, - } + deleteIBPRequest := ibp.ToDeletionRequestProto() workflowOptions := temporalClient.StartWorkflowOptions{ ID: "infiniband-partition-delete-" + ibp.ID.String(), diff --git a/api/pkg/api/handler/infinibandpartition_test.go b/api/pkg/api/handler/infinibandpartition_test.go index 71ad40ef5..423236e40 100644 --- a/api/pkg/api/handler/infinibandpartition_test.go +++ b/api/pkg/api/handler/infinibandpartition_test.go @@ -58,9 +58,9 @@ func testBuildAllocation(t *testing.T, dbSession *cdb.Session, st *cdbm.Site, tn return al } -func testBuildIBPartition(t *testing.T, dbSession *cdb.Session, name string, org string, site *cdbm.Site, tenant *cdbm.Tenant, controllerIBPartionID *uuid.UUID, status *string, isMissingOnSite bool) *cdbm.InfiniBandPartition { +func testBuildIBPartition(t *testing.T, dbSession *cdb.Session, name string, org string, site *cdbm.Site, tenant *cdbm.Tenant, controllerIBPartionID *uuid.UUID, status *cdbm.InfiniBandPartitionStatus, isMissingOnSite bool) *cdbm.InfiniBandPartition { if status == nil { - status = cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady) + status = cdb.Ptr(cdbm.InfiniBandPartitionStatusReady) } ibp := &cdbm.InfiniBandPartition{ ID: uuid.New(), @@ -510,8 +510,8 @@ func TestInfiniBandPartitionHandler_GetAll(t *testing.T) { }, ) assert.Nil(t, err) - common.TestBuildStatusDetail(t, dbSession, ipb.ID.String(), cdbm.InfiniBandPartitionStatusPending, cdb.GetStrPtr("request received, pending processing")) - common.TestBuildStatusDetail(t, dbSession, ipb.ID.String(), cdbm.InfiniBandPartitionStatusReady, cdb.GetStrPtr("InfiniBand Partition is now ready for use")) + common.TestBuildStatusDetail(t, dbSession, ipb.ID.String(), string(cdbm.InfiniBandPartitionStatusPending), cdb.GetStrPtr("request received, pending processing")) + common.TestBuildStatusDetail(t, dbSession, ipb.ID.String(), string(cdbm.InfiniBandPartitionStatusReady), cdb.GetStrPtr("InfiniBand Partition is now ready for use")) ibps = append(ibps, *ipb) } @@ -706,7 +706,7 @@ func TestInfiniBandPartitionHandler_GetAll(t *testing.T) { reqOrgName: tnOrg1, user: tnu1, querySiteID: cdb.GetStrPtr(site1.ID.String()), - queryStatus: cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusPending), + queryStatus: cdb.GetTypedStrPtr(cdbm.InfiniBandPartitionStatusPending), expectedErr: false, expectedStatus: http.StatusOK, expectedCnt: paginator.DefaultLimit, diff --git a/api/pkg/api/handler/instance_test.go b/api/pkg/api/handler/instance_test.go index ac0e48110..7e7e74845 100644 --- a/api/pkg/api/handler/instance_test.go +++ b/api/pkg/api/handler/instance_test.go @@ -686,7 +686,7 @@ func TestCreateInstanceHandler_Handle(t *testing.T) { assert.NotNil(t, al1) // InfiniBand Interface Support - ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ist1 := testInstanceBuildInstanceType(t, dbSession, ip, "test-instance-type-1", st1, cdbm.InstanceStatusReady) @@ -4079,7 +4079,7 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { assert.NotNil(t, nsgTenant2Site1) // InfiniBand Interface Support - ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ibi1 := testInstanceBuildIBInterface(t, dbSession, inst1, st1, ibp1, 0, true, nil, cdb.GetStrPtr(cdbm.InfiniBandInterfaceStatusReady), false) @@ -4090,20 +4090,20 @@ func TestUpdateInstanceHandler_Handle(t *testing.T) { // Add InfiniBand capability to Instance Type common.TestBuildMachineCapability(t, dbSession, nil, &ist1.ID, cdbm.MachineCapabilityTypeInfiniBand, "MT28908 Family [ConnectX-6]", nil, nil, cdb.GetStrPtr("Mellanox Technologies"), cdb.GetIntPtr(5), cdb.Ptr(cdbm.MachineCapabilityDeviceType("")), nil) - ibp2 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp2 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp2) - ibp3 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp3 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp3) - ibp4 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp4 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp4) - ibp6 := testBuildIBPartition(t, dbSession, "test-ibp-ibdup-4slot", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp6 := testBuildIBPartition(t, dbSession, "test-ibp-ibdup-4slot", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp6) // Extra InfiniBand Partitions for updating instance with InfiniBand Interfaces - ibp5 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st2, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp5 := testBuildIBPartition(t, dbSession, "test-ibp-2", tnOrg1, st2, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp5) // Instance with four READY InfiniBand interfaces — distinct (partition ID, device, device instance) per row; @@ -6974,7 +6974,7 @@ func TestGetInstanceHandler_Handle(t *testing.T) { testUpdateInstance(t, dbSession, inst1) // InfiniBand Interface Support - ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ibi1 := testInstanceBuildIBInterface(t, dbSession, inst1, st1, ibp1, 0, false, cdb.GetIntPtr(1), cdb.GetStrPtr(cdbm.InfiniBandInterfaceStatusReady), false) @@ -7672,7 +7672,7 @@ func TestGetAllInstanceHandler_Handle(t *testing.T) { testUpdateInstance(t, dbSession, inst1) // InfiniBand Interface Support - ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ibi1 := testInstanceBuildIBInterface(t, dbSession, inst1, st1, ibp1, 0, false, cdb.GetIntPtr(1), cdb.GetStrPtr(cdbm.InfiniBandInterfaceStatusReady), false) diff --git a/api/pkg/api/handler/instancebatch_test.go b/api/pkg/api/handler/instancebatch_test.go index 4bbbdc527..89b792f33 100644 --- a/api/pkg/api/handler/instancebatch_test.go +++ b/api/pkg/api/handler/instancebatch_test.go @@ -136,7 +136,7 @@ func TestBatchCreateInstanceHandler_Handle(t *testing.T) { assert.NotNil(t, vpcPrefixSite2) // InfiniBand Partition for testing InfiniBand Interfaces - ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-ibp-1", tnOrg, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) // NVLink Logical Partition for testing NVLink Interfaces diff --git a/api/pkg/api/handler/nvlinkinterface_test.go b/api/pkg/api/handler/nvlinkinterface_test.go index 2164c6431..139451d94 100644 --- a/api/pkg/api/handler/nvlinkinterface_test.go +++ b/api/pkg/api/handler/nvlinkinterface_test.go @@ -786,7 +786,7 @@ func TestGetAllInstanceNVLinkInterfaceHandler_Handle(t *testing.T) { ibps := []*cdbm.InfiniBandPartition{} for i := 0; i < 25; i++ { - ibp1 := testBuildIBPartition(t, dbSession, "test-infiniband-partition-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), false) + ibp1 := testBuildIBPartition(t, dbSession, "test-infiniband-partition-1", tnOrg1, st1, tn1, cdb.GetUUIDPtr(uuid.New()), cdb.Ptr(cdbm.InfiniBandPartitionStatusReady), false) assert.NotNil(t, ibp1) ibps = append(ibps, ibp1) } diff --git a/api/pkg/api/model/infinibandpartition.go b/api/pkg/api/model/infinibandpartition.go index 573b58f29..cdac3af85 100644 --- a/api/pkg/api/model/infinibandpartition.go +++ b/api/pkg/api/model/infinibandpartition.go @@ -9,6 +9,7 @@ import ( "github.com/NVIDIA/infra-controller-rest/api/pkg/api/model/util" cdbm "github.com/NVIDIA/infra-controller-rest/db/pkg/db/model" + cwssaws "github.com/NVIDIA/infra-controller-rest/workflow-schema/schema/site-agent/workflows/v1" validation "github.com/go-ozzo/ozzo-validation/v4" validationis "github.com/go-ozzo/ozzo-validation/v4/is" ) @@ -31,12 +32,16 @@ type APIInfiniBandPartitionCreateRequest struct { } // Validate ensure the values passed in request are acceptable -func (ibpcr APIInfiniBandPartitionCreateRequest) Validate() error { - err := validation.ValidateStruct(&ibpcr, +func (ibpcr *APIInfiniBandPartitionCreateRequest) Validate() error { + err := validation.ValidateStruct(ibpcr, validation.Field(&ibpcr.Name, validation.Required.Error(validationErrorStringLength), validation.By(util.ValidateNameCharacters), validation.Length(2, 256).Error(validationErrorStringLength)), + validation.Field(&ibpcr.Description, + validation.When(ibpcr.Description != nil, + validation.Length(0, 1024).Error(validationErrorDescriptionStringLength)), + ), validation.Field(&ibpcr.SiteID, validation.Required.Error(validationErrorValueRequired), validationis.UUID.Error(validationErrorInvalidUUID)), @@ -49,6 +54,25 @@ func (ibpcr APIInfiniBandPartitionCreateRequest) Validate() error { return util.ValidateLabels(ibpcr.Labels) } +// ToProto builds the workflow request that asks a Site to create this +// InfiniBand Partition for this API request. `ibp` is the +// just-persisted DB record; its `ToProto()` is the source of the +// canonical wire fields (Id / Config.Name / Config.TenantOrganizationId +// / Metadata) which the create request reuses verbatim. +// +// The method trusts that the request has already been Validated and +// that the handler has performed any cross-context checks Validate +// cannot see (org/tenant association, Site readiness, name +// uniqueness). It returns no error. +func (ibpcr *APIInfiniBandPartitionCreateRequest) ToProto(ibp *cdbm.InfiniBandPartition) *cwssaws.IBPartitionCreationRequest { + ibpProto := ibp.ToProto() + return &cwssaws.IBPartitionCreationRequest{ + Id: ibpProto.Id, + Config: ibpProto.Config, + Metadata: ibpProto.Metadata, + } +} + // APIInfiniBandPartitionUpdateRequest is the data structure to capture user request to update a InfiniBandPartition type APIInfiniBandPartitionUpdateRequest struct { // Name is the name of the InfiniBand Partition @@ -60,12 +84,15 @@ type APIInfiniBandPartitionUpdateRequest struct { } // Validate ensure the values passed in request are acceptable -func (ibpur APIInfiniBandPartitionUpdateRequest) Validate() error { - err := validation.ValidateStruct(&ibpur, +func (ibpur *APIInfiniBandPartitionUpdateRequest) Validate() error { + err := validation.ValidateStruct(ibpur, validation.Field(&ibpur.Name, validation.When(ibpur.Name != nil, validation.Required.Error(validationErrorStringLength)), validation.When(ibpur.Name != nil, validation.By(util.ValidateNameCharacters)), validation.When(ibpur.Name != nil, validation.Length(2, 256).Error(validationErrorStringLength))), + validation.Field(&ibpur.Description, + validation.When(ibpur.Description != nil, validation.Length(0, 1024).Error(validationErrorDescriptionStringLength)), + ), ) if err != nil { @@ -75,6 +102,24 @@ func (ibpur APIInfiniBandPartitionUpdateRequest) Validate() error { return util.ValidateLabels(ibpur.Labels) } +// ToProto builds the workflow request that pushes this Update's +// merged-into-DB state to a Site. The persisted `ibp` is the source +// of the wire fields because the handler has already merged the +// request's (sparse) update fields into the entity by the time this +// is called; sending the post-merge state matches the pre-existing +// handler behaviour and keeps unchanged fields populated. +// +// The method trusts that the request has already been Validated. It +// returns no error. +func (ibpur *APIInfiniBandPartitionUpdateRequest) ToProto(ibp *cdbm.InfiniBandPartition) *cwssaws.IBPartitionUpdateRequest { + ibpProto := ibp.ToProto() + return &cwssaws.IBPartitionUpdateRequest{ + Id: ibpProto.Id, + Config: ibpProto.Config, + Metadata: ibpProto.Metadata, + } +} + // APIInfiniBandPartition is the data structure to capture API representation of a InfiniBand Partition type APIInfiniBandPartition struct { // ID is the unique UUID v4 identifier for the InfiniBand Partition @@ -108,7 +153,7 @@ type APIInfiniBandPartition struct { // Labels is the labels of the InfiniBand Partition Labels map[string]string `json:"labels"` // Status is the status o the InfiniBand Partition - Status string `json:"status"` + Status cdbm.InfiniBandPartitionStatus `json:"status"` // StatusHistory is the status detail records for the InfiniBand Partition over time StatusHistory []APIStatusDetail `json:"statusHistory"` // Created indicates the ISO datetime string for when the InfiniBand Partition was created @@ -167,7 +212,7 @@ type APIInfiniBandPartitionSummary struct { // Controller IB Partition is the ID of the Site Controller Partition corresponding to the InfiniBand Partition ControllerIBPartitionID *string `json:"controllerIBPartitionId"` // Status is the status of the InfiniBand Partition - Status string `json:"status"` + Status cdbm.InfiniBandPartitionStatus `json:"status"` } // NewAPIInfiniBandPartitionSummary accepts a DB layer InfiniBandPartition object returns an API layer object diff --git a/api/pkg/api/model/infinibandpartition_test.go b/api/pkg/api/model/infinibandpartition_test.go index f6d8f14af..7703a5c8a 100644 --- a/api/pkg/api/model/infinibandpartition_test.go +++ b/api/pkg/api/model/infinibandpartition_test.go @@ -5,6 +5,7 @@ package model import ( "fmt" + "strings" "testing" "time" @@ -12,6 +13,7 @@ import ( cdbm "github.com/NVIDIA/infra-controller-rest/db/pkg/db/model" "github.com/google/uuid" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAPIInfiniBandPartitionCreateRequest_Validate(t *testing.T) { @@ -35,6 +37,11 @@ func TestAPIInfiniBandPartitionCreateRequest_Validate(t *testing.T) { obj: APIInfiniBandPartitionCreateRequest{Name: "test"}, expectErr: true, }, + { + desc: "error when description is too long", + obj: APIInfiniBandPartitionCreateRequest{Name: "test", Description: cdb.GetStrPtr(strings.Repeat("x", 1025)), SiteID: uuid.New().String()}, + expectErr: true, + }, } for _, tc := range tests { t.Run(tc.desc, func(t *testing.T) { @@ -47,6 +54,33 @@ func TestAPIInfiniBandPartitionCreateRequest_Validate(t *testing.T) { } } +func TestAPIInfiniBandPartitionCreateRequest_ToProto(t *testing.T) { + id := uuid.New() + desc := "primary" + ibp := &cdbm.InfiniBandPartition{ + ID: id, + Org: "org-1", + Name: "ibp-a", + Description: &desc, + Labels: map[string]string{"env": "prod"}, + } + + t.Run("builds creation request from the entity proto", func(t *testing.T) { + req := APIInfiniBandPartitionCreateRequest{Name: "ibp-a", SiteID: uuid.NewString()} + got := req.ToProto(ibp) + require.NotNil(t, got) + require.NotNil(t, got.Id) + assert.Equal(t, id.String(), got.Id.Value) + require.NotNil(t, got.Config) + assert.Equal(t, "ibp-a", got.Config.Name) + assert.Equal(t, "org-1", got.Config.TenantOrganizationId) + require.NotNil(t, got.Metadata) + assert.Equal(t, "ibp-a", got.Metadata.Name) + assert.Equal(t, "primary", got.Metadata.Description) + require.Len(t, got.Metadata.Labels, 1) + }) +} + func TestAPIInfiniBandPartitionUpdateRequest_Validate(t *testing.T) { tests := []struct { desc string @@ -63,6 +97,11 @@ func TestAPIInfiniBandPartitionUpdateRequest_Validate(t *testing.T) { obj: APIInfiniBandPartitionUpdateRequest{Name: cdb.GetStrPtr("updatedname"), Description: cdb.GetStrPtr("updated")}, expectErr: false, }, + { + desc: "error when description is too long", + obj: APIInfiniBandPartitionUpdateRequest{Description: cdb.GetStrPtr(strings.Repeat("x", 1025))}, + expectErr: true, + }, } for _, tc := range tests { t.Run(tc.desc, func(t *testing.T) { @@ -75,6 +114,33 @@ func TestAPIInfiniBandPartitionUpdateRequest_Validate(t *testing.T) { } } +func TestAPIInfiniBandPartitionUpdateRequest_ToProto(t *testing.T) { + id := uuid.New() + desc := "primary" + ibp := &cdbm.InfiniBandPartition{ + ID: id, + Org: "org-1", + Name: "ibp-a", + Description: &desc, + Labels: map[string]string{"env": "prod"}, + } + + t.Run("builds update request from the entity proto", func(t *testing.T) { + req := APIInfiniBandPartitionUpdateRequest{Name: cdb.GetStrPtr("ibp-a")} + got := req.ToProto(ibp) + require.NotNil(t, got) + require.NotNil(t, got.Id) + assert.Equal(t, id.String(), got.Id.Value) + require.NotNil(t, got.Config) + assert.Equal(t, "ibp-a", got.Config.Name) + assert.Equal(t, "org-1", got.Config.TenantOrganizationId) + require.NotNil(t, got.Metadata) + assert.Equal(t, "ibp-a", got.Metadata.Name) + assert.Equal(t, "primary", got.Metadata.Description) + require.Len(t, got.Metadata.Labels, 1) + }) +} + func TestAPIInfiniBandPartitionNew(t *testing.T) { dbIBP := &cdbm.InfiniBandPartition{ ID: uuid.New(), diff --git a/db/pkg/db/model/helper_test.go b/db/pkg/db/model/helper_test.go index 7ba08be14..55c723484 100644 --- a/db/pkg/db/model/helper_test.go +++ b/db/pkg/db/model/helper_test.go @@ -270,7 +270,7 @@ func testBuildFabric(t *testing.T, dbSession *db.Session, id *string, org string return fb } -func testBuildInfiniBandPartition(t *testing.T, dbSession *db.Session, id *uuid.UUID, name string, description *string, org string, tenantID uuid.UUID, siteID uuid.UUID, controllerIBInfiniBandPartitionID *uuid.UUID, partitionKey *string, partitionName *string, serviceLevel *int, rateLimit *float32, mtu *int, enableSharp *bool, labels map[string]string, status *string, createdBy uuid.UUID) *InfiniBandPartition { +func testBuildInfiniBandPartition(t *testing.T, dbSession *db.Session, id *uuid.UUID, name string, description *string, org string, tenantID uuid.UUID, siteID uuid.UUID, controllerIBInfiniBandPartitionID *uuid.UUID, partitionKey *string, partitionName *string, serviceLevel *int, rateLimit *float32, mtu *int, enableSharp *bool, labels map[string]string, status *InfiniBandPartitionStatus, createdBy uuid.UUID) *InfiniBandPartition { pid := uuid.New() if id != nil { pid = *id diff --git a/db/pkg/db/model/infinibandinterface_test.go b/db/pkg/db/model/infinibandinterface_test.go index 6715273f4..0d38b888f 100644 --- a/db/pkg/db/model/infinibandinterface_test.go +++ b/db/pkg/db/model/infinibandinterface_test.go @@ -91,7 +91,7 @@ func TestInfiniBandInterfaceSQLDAO_GetByID(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i1) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibif := testBuildInfiniBandInterface(t, dbSession, nil, st.ID, i1.ID, ibpr.ID, 1, false, nil, nil, false, db.GetStrPtr(InfiniBandInterfaceStatusReady), tnu.ID) // OTEL Spanner configuration @@ -259,8 +259,8 @@ func TestInfiniBandInterface_GetAll(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i2) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn1.Org, tn1.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu1.ID) - ibpr2 := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition2", nil, tn2.Org, tn2.ID, st2.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu2.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn1.Org, tn1.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu1.ID) + ibpr2 := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition2", nil, tn2.Org, tn2.ID, st2.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu2.ID) totalCount := 30 infiniBandInterfaces := []InfiniBandInterface{} @@ -571,7 +571,7 @@ func TestInfiniBandInterfaceSQLDAO_Create(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i1) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibif := &InfiniBandInterface{ SiteID: st.ID, @@ -717,12 +717,12 @@ func TestInfiniBandInterfaceSQLDAO_Update(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i1) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibif := testBuildInfiniBandInterface(t, dbSession, nil, st.ID, i1.ID, ibpr.ID, 1, false, nil, nil, false, db.GetStrPtr(InfiniBandInterfaceStatusReady), tnu.ID) uInfiniBandInterface := ibif uInfiniBandInterface.VirtualFunctionID = db.GetIntPtr(2) - uInfiniBandInterface.Status = InfiniBandPartitionStatusPending + uInfiniBandInterface.Status = InfiniBandInterfaceStatusPending uInfiniBandInterface.IsMissingOnSite = true // OTEL Spanner configuration @@ -848,7 +848,7 @@ func TestInfiniBandInterfaceSQLDAO_Clear(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i1) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibif := testBuildInfiniBandInterface(t, dbSession, nil, st.ID, i1.ID, ibpr.ID, 1, false, nil, nil, false, db.GetStrPtr(InfiniBandInterfaceStatusReady), tnu.ID) // OTEL Spanner configuration @@ -978,7 +978,7 @@ func TestInfiniBandInterfaceSQLDAO_Delete(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, i1) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-infinibandpartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibif := testBuildInfiniBandInterface(t, dbSession, nil, st.ID, i1.ID, ibpr.ID, 1, false, nil, nil, false, db.GetStrPtr(InfiniBandInterfaceStatusReady), tnu.ID) // OTEL Spanner configuration @@ -1068,7 +1068,7 @@ func TestInfiniBandInterfaceSQLDAO_CreateMultiple(t *testing.T) { ) assert.Nil(t, err) - ibp := testBuildInfiniBandPartition(t, dbSession, nil, "test-ibpartition", db.GetStrPtr("Test IB Partition"), tn.Org, tn.ID, st.ID, nil, nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibp := testBuildInfiniBandPartition(t, dbSession, nil, "test-ibpartition", db.GetStrPtr("Test IB Partition"), tn.Org, tn.ID, st.ID, nil, nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) ibisd := NewInfiniBandInterfaceDAO(dbSession) @@ -1232,8 +1232,8 @@ func TestInfiniBandInterfaceSQLDAO_DeleteAllBySiteID(t *testing.T) { inst1 := buildInstanceForSite(st1, "host1.com", "mcType1") inst2 := buildInstanceForSite(st2, "host2.com", "mcType2") - ibp1 := testBuildInfiniBandPartition(t, dbSession, nil, "ibp-site-1", nil, tn.Org, tn.ID, st1.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) - ibp2 := testBuildInfiniBandPartition(t, dbSession, nil, "ibp-site-2", nil, tn.Org, tn.ID, st2.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibp1 := testBuildInfiniBandPartition(t, dbSession, nil, "ibp-site-1", nil, tn.Org, tn.ID, st1.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) + ibp2 := testBuildInfiniBandPartition(t, dbSession, nil, "ibp-site-2", nil, tn.Org, tn.ID, st2.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) // Two interfaces in the target site, one in another site that should remain. ibi1a := testBuildInfiniBandInterface(t, dbSession, nil, st1.ID, inst1.ID, ibp1.ID, 1, true, nil, nil, false, db.GetStrPtr(InfiniBandInterfaceStatusReady), tnu.ID) diff --git a/db/pkg/db/model/infinibandpartition.go b/db/pkg/db/model/infinibandpartition.go index d1b418f37..331ac12cd 100644 --- a/db/pkg/db/model/infinibandpartition.go +++ b/db/pkg/db/model/infinibandpartition.go @@ -6,29 +6,48 @@ package model import ( "context" "database/sql" + "errors" + "fmt" + "strings" "time" "github.com/NVIDIA/infra-controller-rest/db/pkg/db" "github.com/NVIDIA/infra-controller-rest/db/pkg/db/paginator" stracer "github.com/NVIDIA/infra-controller-rest/db/pkg/tracer" + cwssaws "github.com/NVIDIA/infra-controller-rest/workflow-schema/schema/site-agent/workflows/v1" + validation "github.com/go-ozzo/ozzo-validation/v4" "github.com/google/uuid" + "github.com/rs/zerolog/log" "github.com/uptrace/bun" ) +// InfiniBandPartitionStatus is the domain enum for the lifecycle state +// of an `InfiniBandPartition`. Defining it as a named string lets us +// hang the workflow-proto conversion on it as methods +// (`(*s).FromProto`, `(s).Message`) per the "Named types own their +// proto behavior" rule, and keeps the DB column comparable as a plain +// string at the storage layer. +type InfiniBandPartitionStatus string + +// InfiniBandPartitionStatus values. Stored as plain strings in the DB +// column `infiniband_partition.status`. const ( // InfiniBandPartitionStatusPending indicates that the InfiniBandPartition request was received but not yet processed - InfiniBandPartitionStatusPending = "Pending" + InfiniBandPartitionStatusPending InfiniBandPartitionStatus = "Pending" // InfiniBandPartitionStatusProvisioning indicates that the InfiniBandPartition is being provisioned - InfiniBandPartitionStatusProvisioning = "Provisioning" + InfiniBandPartitionStatusProvisioning InfiniBandPartitionStatus = "Provisioning" // InfiniBandPartitionStatusReady indicates that the InfiniBandPartition has been successfully provisioned on the Site - InfiniBandPartitionStatusReady = "Ready" + InfiniBandPartitionStatusReady InfiniBandPartitionStatus = "Ready" // InfiniBandPartitionStatusConfiguring indicates that the InfiniBandPartition is being configuring - InfiniBandPartitionStatusConfiguring = "Configuring" + InfiniBandPartitionStatusConfiguring InfiniBandPartitionStatus = "Configuring" // InfiniBandPartitionStatusError is the status of a InfiniBandPartition that is in error mode - InfiniBandPartitionStatusError = "Error" + InfiniBandPartitionStatusError InfiniBandPartitionStatus = "Error" // InfiniBandPartitionStatusDeleting indicates that the InfiniBandPartition is being deleted - InfiniBandPartitionStatusDeleting = "Deleting" + InfiniBandPartitionStatusDeleting InfiniBandPartitionStatus = "Deleting" +) + +const ( // InfiniBandPartitionRelationName is the relation name for the InfiniBandPartition model InfiniBandPartitionRelationName = "InfiniBandPartition" @@ -45,7 +64,7 @@ var ( TenantRelationName: true, } // InfiniBandPartitionStatusMap is a list of valid status for the InfiniBandPartition model - InfiniBandPartitionStatusMap = map[string]bool{ + InfiniBandPartitionStatusMap = map[InfiniBandPartitionStatus]bool{ InfiniBandPartitionStatusPending: true, InfiniBandPartitionStatusProvisioning: true, InfiniBandPartitionStatusReady: true, @@ -55,32 +74,202 @@ var ( } ) +// FromProto maps a workflow `TenantState` proto enum to the +// corresponding DB-side `InfiniBandPartitionStatus`, mirroring the +// "leaf-named-type owns its proto behavior" rule. An unknown proto +// state leaves the receiver as the empty string so the caller can +// detect "no DB-side equivalent" (the pre-typed helper returned +// `(nil, nil)` for the same case). +func (s *InfiniBandPartitionStatus) FromProto(state cwssaws.TenantState) { + switch state { + case cwssaws.TenantState_PROVISIONING: + *s = InfiniBandPartitionStatusProvisioning + case cwssaws.TenantState_CONFIGURING: + *s = InfiniBandPartitionStatusConfiguring + case cwssaws.TenantState_READY: + *s = InfiniBandPartitionStatusReady + case cwssaws.TenantState_FAILED: + *s = InfiniBandPartitionStatusError + default: + log.Warn().Str("TenantState", state.String()).Msg("unsupported InfiniBandPartitionStatus requested") + *s = "" + } +} + +// Message returns the canonical human-readable message that pairs +// with this status. Returns the empty string for an unrecognized +// status (typically the zero value). +func (s InfiniBandPartitionStatus) Message() string { + switch s { + case InfiniBandPartitionStatusProvisioning: + return "InfiniBand Partition is being provisioned on Site" + case InfiniBandPartitionStatusConfiguring: + return "InfiniBand Partition is being configured on Site" + case InfiniBandPartitionStatusReady: + return "InfiniBand Partition is ready for use" + case InfiniBandPartitionStatusError: + return "InfiniBand Partition is in error state" + } + return "" +} + // InfiniBandPartition represents entries in the InfiniBandPartition table type InfiniBandPartition struct { bun.BaseModel `bun:"table:infiniband_partition,alias:ibp"` - ID uuid.UUID `bun:"type:uuid,pk"` - Name string `bun:"name,notnull"` - Description *string `bun:"description"` - Org string `bun:"org,notnull"` - SiteID uuid.UUID `bun:"site_id,type:uuid,notnull"` - Site *Site `bun:"rel:belongs-to,join:site_id=id"` - TenantID uuid.UUID `bun:"tenant_id,type:uuid,notnull"` - Tenant *Tenant `bun:"rel:belongs-to,join:tenant_id=id"` - ControllerIBPartitionID *uuid.UUID `bun:"controller_ib_partition_id,type:uuid"` - PartitionKey *string `bun:"partition_key"` - PartitionName *string `bun:"partition_name"` - ServiceLevel *int `bun:"service_level"` - RateLimit *float32 `bun:"rate_limit"` - Mtu *int `bun:"mtu"` - EnableSharp *bool `bun:"enable_sharp"` - Labels map[string]string `bun:"labels,type:jsonb"` - Status string `bun:"status,notnull"` - IsMissingOnSite bool `bun:"is_missing_on_site,notnull"` - Created time.Time `bun:"created,nullzero,notnull,default:current_timestamp"` - Updated time.Time `bun:"updated,nullzero,notnull,default:current_timestamp"` - Deleted *time.Time `bun:"deleted,soft_delete"` - CreatedBy uuid.UUID `bun:"type:uuid,notnull"` + ID uuid.UUID `bun:"type:uuid,pk"` + Name string `bun:"name,notnull"` + Description *string `bun:"description"` + Org string `bun:"org,notnull"` + SiteID uuid.UUID `bun:"site_id,type:uuid,notnull"` + Site *Site `bun:"rel:belongs-to,join:site_id=id"` + TenantID uuid.UUID `bun:"tenant_id,type:uuid,notnull"` + Tenant *Tenant `bun:"rel:belongs-to,join:tenant_id=id"` + ControllerIBPartitionID *uuid.UUID `bun:"controller_ib_partition_id,type:uuid"` + PartitionKey *string `bun:"partition_key"` + PartitionName *string `bun:"partition_name"` + ServiceLevel *int `bun:"service_level"` + RateLimit *float32 `bun:"rate_limit"` + Mtu *int `bun:"mtu"` + EnableSharp *bool `bun:"enable_sharp"` + Labels Labels `bun:"labels,type:jsonb"` + Status InfiniBandPartitionStatus `bun:"status,notnull"` + IsMissingOnSite bool `bun:"is_missing_on_site,notnull"` + Created time.Time `bun:"created,nullzero,notnull,default:current_timestamp"` + Updated time.Time `bun:"updated,nullzero,notnull,default:current_timestamp"` + Deleted *time.Time `bun:"deleted,soft_delete"` + CreatedBy uuid.UUID `bun:"type:uuid,notnull"` +} + +// Validate checks that the populated InfiniBandPartition is wire-safe. +// Mirrors the API-side rules so callers that build an +// `InfiniBandPartition` from site-supplied or request data can gate +// it through one consistent contract. +func (ibp *InfiniBandPartition) Validate() error { + statuses := make([]any, 0, len(InfiniBandPartitionStatusMap)) + for s := range InfiniBandPartitionStatusMap { + statuses = append(statuses, s) + } + return validation.ValidateStruct(ibp, + validation.Field(&ibp.Name, + validation.Required.Error("InfiniBandPartition Name must be specified"), + validation.Length(2, 256).Error("InfiniBandPartition Name must be at least 2 characters and maximum 256 characters"), + validation.By(validateInfiniBandPartitionNameWhitespace)), + validation.Field(&ibp.Status, + validation.Required.Error("InfiniBandPartition Status must be specified"), + validation.In(statuses...).Error(fmt.Sprintf("invalid InfiniBandPartition Status: %q", ibp.Status))), + ) +} + +// validateInfiniBandPartitionNameWhitespace rejects Names with leading +// or trailing whitespace, mirroring the API-side +// `util.ValidateNameCharacters` rule so the wire-bound DB-model gate +// matches the API one. Shared by `(*InfiniBandPartition).Validate()` +// and the partial-field DAO Update path. +func validateInfiniBandPartitionNameWhitespace(value interface{}) error { + s, ok := value.(string) + if !ok { + return errors.New("InfiniBandPartition Name must be a string") + } + if strings.TrimSpace(s) != s { + return errors.New("InfiniBandPartition Name must not contain leading or trailing whitespace") + } + return nil +} + +// toMetadataProto builds a workflow Metadata proto from the +// InfiniBandPartition's Name, Description, and Labels. Description +// defaults to the empty string when ibp.Description is nil. Labels +// are produced via `(Labels).ToProto()` so the conversion stays on +// the named type per the proto-conversion convention. +func (ibp *InfiniBandPartition) toMetadataProto() *cwssaws.Metadata { + md := &cwssaws.Metadata{ + Name: ibp.Name, + Description: "", + Labels: ibp.Labels.ToProto(), + } + if ibp.Description != nil { + md.Description = *ibp.Description + } + return md +} + +// ToProto converts this InfiniBandPartition into its workflow proto +// representation. Used as the canonical entity-to-proto conversion; +// request-shape protos (create / update) are produced by `ToProto` +// methods on the corresponding API request types in +// api/pkg/api/model/infinibandpartition.go. +// +// `Config.TenantOrganizationId` is sourced from `ibp.Org`, which is +// the persisted tenant org id (populated from the path param at +// create time and carried with the entity thereafter). +func (ibp *InfiniBandPartition) ToProto() *cwssaws.IBPartition { + return &cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: ibp.ID.String()}, + Config: &cwssaws.IBPartitionConfig{ + Name: ibp.Name, + TenantOrganizationId: ibp.Org, + }, + Metadata: ibp.toMetadataProto(), + } +} + +// FromProto populates this InfiniBandPartition from its workflow proto +// representation. A nil proto is a no-op. This is the inverse of +// `ToProto` and exists for convention symmetry — currently no code +// path on the cloud side reconstructs a full InfiniBandPartition +// entity from a `cwssaws.IBPartition` (the site is the destination, +// not the source for the create/delete request shapes used today), +// but the method is provided so future reconciliation flows have a +// single canonical entry point. +// +// Field-level contract: +// - `ibp.ID` is preserved on a missing or unparseable `proto.Id`, +// because callers pre-validate the UUID before calling. +// - `Name` is sourced from `proto.Metadata.Name` when set, falling +// back to the (deprecated) top-level `proto.Config.Name` so the +// method keeps working through the deprecation window. +// - Optional pointer fields (Description, Labels) are cleared when +// the proto omits them so `FromProto` is a clean reset rather +// than a partial merge. +func (ibp *InfiniBandPartition) FromProto(proto *cwssaws.IBPartition) { + if proto == nil { + return + } + if proto.Id != nil { + if id, err := uuid.Parse(proto.Id.Value); err == nil { + ibp.ID = id + } + } + // Reset Config / Metadata-derived fields up front so the `clean + // reset rather than a partial merge` contract holds when those + // proto sub-messages are nil or omit a field. + ibp.Name = "" + ibp.Org = "" + ibp.Description = nil + ibp.Labels = nil + if proto.Config != nil { + ibp.Name = proto.Config.Name + ibp.Org = proto.Config.TenantOrganizationId + } + if proto.Metadata != nil { + if proto.Metadata.Name != "" { + ibp.Name = proto.Metadata.Name + } + if proto.Metadata.Description != "" { + desc := proto.Metadata.Description + ibp.Description = &desc + } + ibp.Labels.FromProto(proto.Metadata.GetLabels()) + } +} + +// ToDeletionRequestProto builds the workflow request that asks a Site to +// delete this InfiniBand Partition. +func (ibp *InfiniBandPartition) ToDeletionRequestProto() *cwssaws.IBPartitionDeletionRequest { + return &cwssaws.IBPartitionDeletionRequest{ + Id: &cwssaws.IBPartitionId{Value: ibp.ID.String()}, + } } // InfiniBandPartitionCreateInput input parameters for Create method @@ -99,7 +288,7 @@ type InfiniBandPartitionCreateInput struct { Mtu *int EnableSharp *bool Labels map[string]string - Status string + Status InfiniBandPartitionStatus CreatedBy uuid.UUID } @@ -116,7 +305,7 @@ type InfiniBandPartitionUpdateInput struct { Mtu *int EnableSharp *bool Labels map[string]string - Status *string + Status *InfiniBandPartitionStatus IsMissingOnSite *bool } @@ -347,6 +536,10 @@ func (ibpsd InfiniBandPartitionSQLDAO) Create(ctx context.Context, tx *db.Tx, in CreatedBy: input.CreatedBy, } + if err := ibp.Validate(); err != nil { + return nil, err + } + _, err := db.GetIDB(tx, ibpsd.dbSession).NewInsert().Model(ibp).Exec(ctx) if err != nil { return nil, err @@ -377,6 +570,12 @@ func (ibpsd InfiniBandPartitionSQLDAO) Update(ctx context.Context, tx *db.Tx, in updatedFields := []string{} if input.Name != nil { + if err := validation.Validate(*input.Name, + validation.Required.Error("InfiniBandPartition Name must be specified"), + validation.Length(2, 256).Error("InfiniBandPartition Name must be at least 2 characters and maximum 256 characters"), + validation.By(validateInfiniBandPartitionNameWhitespace)); err != nil { + return nil, err + } ibp.Name = *input.Name updatedFields = append(updatedFields, "name") ibpsd.tracerSpan.SetAttribute(InfiniBandPartitionDAOSpan, "name", *input.Name) @@ -427,6 +626,9 @@ func (ibpsd InfiniBandPartitionSQLDAO) Update(ctx context.Context, tx *db.Tx, in ibpsd.tracerSpan.SetAttribute(InfiniBandPartitionDAOSpan, "labels", input.Labels) } if input.Status != nil { + if !InfiniBandPartitionStatusMap[*input.Status] { + return nil, fmt.Errorf("invalid InfiniBandPartition Status: %q", *input.Status) + } ibp.Status = *input.Status updatedFields = append(updatedFields, "status") ibpsd.tracerSpan.SetAttribute(InfiniBandPartitionDAOSpan, "status", *input.Status) diff --git a/db/pkg/db/model/infinibandpartition_test.go b/db/pkg/db/model/infinibandpartition_test.go index a343020a8..25974d3c0 100644 --- a/db/pkg/db/model/infinibandpartition_test.go +++ b/db/pkg/db/model/infinibandpartition_test.go @@ -11,12 +11,217 @@ import ( "github.com/NVIDIA/infra-controller-rest/db/pkg/db" "github.com/NVIDIA/infra-controller-rest/db/pkg/db/paginator" stracer "github.com/NVIDIA/infra-controller-rest/db/pkg/tracer" + cwssaws "github.com/NVIDIA/infra-controller-rest/workflow-schema/schema/site-agent/workflows/v1" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" otrace "go.opentelemetry.io/otel/trace" ) +func TestInfiniBandPartition_ToProto(t *testing.T) { + id := uuid.New() + desc := "primary" + + t.Run("populates id, config, and metadata", func(t *testing.T) { + ibp := &InfiniBandPartition{ + ID: id, + Name: "ibp-a", + Org: "org-1", + Description: &desc, + Labels: map[string]string{"env": "prod"}, + } + got := ibp.ToProto() + require.NotNil(t, got) + require.NotNil(t, got.Id) + assert.Equal(t, id.String(), got.Id.Value) + require.NotNil(t, got.Config) + assert.Equal(t, "ibp-a", got.Config.Name) + assert.Equal(t, "org-1", got.Config.TenantOrganizationId) + require.NotNil(t, got.Metadata) + assert.Equal(t, "ibp-a", got.Metadata.Name) + assert.Equal(t, "primary", got.Metadata.Description) + require.Len(t, got.Metadata.Labels, 1) + assert.Equal(t, "env", got.Metadata.Labels[0].Key) + require.NotNil(t, got.Metadata.Labels[0].Value) + assert.Equal(t, "prod", *got.Metadata.Labels[0].Value) + }) + + t.Run("nil description and labels yield zero-value metadata", func(t *testing.T) { + ibp := &InfiniBandPartition{ID: id, Org: "org-1", Name: "ibp-a"} + got := ibp.ToProto() + require.NotNil(t, got.Metadata) + assert.Equal(t, "", got.Metadata.Description) + assert.Nil(t, got.Metadata.Labels) + }) +} + +func TestInfiniBandPartition_FromProto(t *testing.T) { + id := uuid.New() + + t.Run("nil proto leaves receiver unchanged", func(t *testing.T) { + ibp := &InfiniBandPartition{ID: id, Name: "preserved", Org: "org-1"} + ibp.FromProto(nil) + assert.Equal(t, id, ibp.ID) + assert.Equal(t, "preserved", ibp.Name) + assert.Equal(t, "org-1", ibp.Org) + }) + + t.Run("invalid id leaves ibp.ID unchanged", func(t *testing.T) { + ibp := &InfiniBandPartition{ID: id} + ibp.FromProto(&cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: "not-a-uuid"}, + Config: &cwssaws.IBPartitionConfig{Name: "ibp-a", TenantOrganizationId: "org-1"}, + }) + assert.Equal(t, id, ibp.ID) + assert.Equal(t, "ibp-a", ibp.Name) + assert.Equal(t, "org-1", ibp.Org) + }) + + t.Run("populates fields from proto", func(t *testing.T) { + ibp := &InfiniBandPartition{} + ibp.FromProto(&cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: id.String()}, + Config: &cwssaws.IBPartitionConfig{Name: "ibp-a", TenantOrganizationId: "org-1"}, + Metadata: &cwssaws.Metadata{ + Name: "ibp-a", + Description: "primary", + Labels: []*cwssaws.Label{ + {Key: "env", Value: db.GetStrPtr("prod")}, + }, + }, + }) + assert.Equal(t, id, ibp.ID) + assert.Equal(t, "ibp-a", ibp.Name) + assert.Equal(t, "org-1", ibp.Org) + require.NotNil(t, ibp.Description) + assert.Equal(t, "primary", *ibp.Description) + assert.Equal(t, Labels{"env": "prod"}, ibp.Labels) + }) + + t.Run("missing optional fields are explicitly cleared", func(t *testing.T) { + stale := "stale" + ibp := &InfiniBandPartition{ + ID: id, + Description: &stale, + Labels: map[string]string{"old": "val"}, + } + ibp.FromProto(&cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: id.String()}, + Config: &cwssaws.IBPartitionConfig{Name: "ibp-a", TenantOrganizationId: "org-1"}, + }) + assert.Nil(t, ibp.Description) + assert.Nil(t, ibp.Labels) + }) + + t.Run("prefers Metadata.Name over the Config.Name field", func(t *testing.T) { + ibp := &InfiniBandPartition{} + ibp.FromProto(&cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: id.String()}, + Config: &cwssaws.IBPartitionConfig{Name: "config-name", TenantOrganizationId: "org-1"}, + Metadata: &cwssaws.Metadata{Name: "metadata-name"}, + }) + assert.Equal(t, "metadata-name", ibp.Name) + }) + + t.Run("falls back to Config.Name when Metadata.Name is empty", func(t *testing.T) { + ibp := &InfiniBandPartition{} + ibp.FromProto(&cwssaws.IBPartition{ + Id: &cwssaws.IBPartitionId{Value: id.String()}, + Config: &cwssaws.IBPartitionConfig{Name: "config-fallback", TenantOrganizationId: "org-1"}, + Metadata: &cwssaws.Metadata{Name: ""}, + }) + assert.Equal(t, "config-fallback", ibp.Name) + }) +} + +func TestInfiniBandPartition_ToDeletionRequestProto(t *testing.T) { + id := uuid.New() + ibp := &InfiniBandPartition{ID: id} + req := ibp.ToDeletionRequestProto() + require.NotNil(t, req) + require.NotNil(t, req.Id) + assert.Equal(t, id.String(), req.Id.Value) +} + +func TestInfiniBandPartitionStatus_FromProto(t *testing.T) { + tests := []struct { + name string + state cwssaws.TenantState + wantStatus InfiniBandPartitionStatus + }{ + {name: "PROVISIONING maps to Provisioning", state: cwssaws.TenantState_PROVISIONING, wantStatus: InfiniBandPartitionStatusProvisioning}, + {name: "CONFIGURING maps to Configuring", state: cwssaws.TenantState_CONFIGURING, wantStatus: InfiniBandPartitionStatusConfiguring}, + {name: "READY maps to Ready", state: cwssaws.TenantState_READY, wantStatus: InfiniBandPartitionStatusReady}, + {name: "FAILED maps to Error", state: cwssaws.TenantState_FAILED, wantStatus: InfiniBandPartitionStatusError}, + {name: "unknown state yields empty", state: cwssaws.TenantState(9999), wantStatus: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var got InfiniBandPartitionStatus + got.FromProto(tc.state) + assert.Equal(t, tc.wantStatus, got) + }) + } +} + +func TestInfiniBandPartitionStatus_Message(t *testing.T) { + t.Run("known statuses return non-empty messages", func(t *testing.T) { + for _, s := range []InfiniBandPartitionStatus{ + InfiniBandPartitionStatusProvisioning, + InfiniBandPartitionStatusConfiguring, + InfiniBandPartitionStatusReady, + InfiniBandPartitionStatusError, + } { + assert.NotEmpty(t, s.Message(), "expected non-empty message for %q", s) + } + }) + t.Run("unknown status returns empty", func(t *testing.T) { + assert.Empty(t, InfiniBandPartitionStatus("").Message()) + assert.Empty(t, InfiniBandPartitionStatus("Mystery").Message()) + }) +} + +func TestInfiniBandPartition_Validate(t *testing.T) { + valid := &InfiniBandPartition{ + Name: "test-ibp", + Status: InfiniBandPartitionStatusReady, + } + + t.Run("populated partition is valid", func(t *testing.T) { + assert.NoError(t, valid.Validate()) + }) + t.Run("empty Status errors", func(t *testing.T) { + ibp := *valid + ibp.Status = "" + assert.Error(t, ibp.Validate()) + }) + t.Run("invalid Status errors", func(t *testing.T) { + ibp := *valid + ibp.Status = "Bogus" + assert.Error(t, ibp.Validate()) + }) + t.Run("empty Name errors", func(t *testing.T) { + ibp := *valid + ibp.Name = "" + assert.Error(t, ibp.Validate()) + }) + t.Run("Name with leading whitespace errors", func(t *testing.T) { + ibp := *valid + ibp.Name = " test-ibp" + assert.Error(t, ibp.Validate()) + }) + t.Run("Name with trailing whitespace errors", func(t *testing.T) { + ibp := *valid + ibp.Name = "test-ibp " + assert.Error(t, ibp.Validate()) + }) + t.Run("single-character Name errors (too short)", func(t *testing.T) { + ibp := *valid + ibp.Name = "x" + assert.Error(t, ibp.Validate()) + }) +} + func testInfiniBandPartitionSetupSchema(t *testing.T, dbSession *db.Session) { // Create tables err := dbSession.DB.ResetModel(context.Background(), (*Tenant)(nil)) @@ -57,7 +262,7 @@ func TestInfiniBandPartitionSQLDAO_GetByID(t *testing.T) { st := testBuildSite(t, dbSession, nil, ip.ID, "test-site", "Test Site", ip.Org, ipu.ID) - ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + ibpr := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) // OTEL Spanner configuration _, _, ctx := testCommonTraceProviderSetup(t, context.Background()) @@ -184,9 +389,9 @@ func TestInfiniBandPartition_GetAll(t *testing.T) { } if i%2 == 0 { - pt = testBuildInfiniBandPartition(t, dbSession, nil, fmt.Sprintf("test-InfiniBandPartition-batch-v1-%v", i), db.GetStrPtr(fmt.Sprintf("test-InfiniBandPartition-desc-batch-1-%v", i)), tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, map[string]string{fmt.Sprintf("test-InfiniBandPartition-batch-key1-%v", i): fmt.Sprintf("test-InfiniBandPartition-batch-value1-%v", i)}, db.GetStrPtr(InfiniBandPartitionStatusReady), tn.CreatedBy) + pt = testBuildInfiniBandPartition(t, dbSession, nil, fmt.Sprintf("test-InfiniBandPartition-batch-v1-%v", i), db.GetStrPtr(fmt.Sprintf("test-InfiniBandPartition-desc-batch-1-%v", i)), tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, map[string]string{fmt.Sprintf("test-InfiniBandPartition-batch-key1-%v", i): fmt.Sprintf("test-InfiniBandPartition-batch-value1-%v", i)}, db.Ptr(InfiniBandPartitionStatusReady), tn.CreatedBy) } else { - pt = testBuildInfiniBandPartition(t, dbSession, nil, fmt.Sprintf("test-InfiniBandPartition-batch-v2-%v", i), db.GetStrPtr(fmt.Sprintf("test-InfiniBandPartition-desc-batch-2-%v", i)), tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, map[string]string{fmt.Sprintf("test-InfiniBandPartition-batch-key2-%v", i): fmt.Sprintf("test-InfiniBandPartition-batch-value2-%v", i)}, db.GetStrPtr(InfiniBandPartitionStatusDeleting), tn.CreatedBy) + pt = testBuildInfiniBandPartition(t, dbSession, nil, fmt.Sprintf("test-InfiniBandPartition-batch-v2-%v", i), db.GetStrPtr(fmt.Sprintf("test-InfiniBandPartition-desc-batch-2-%v", i)), tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, map[string]string{fmt.Sprintf("test-InfiniBandPartition-batch-key2-%v", i): fmt.Sprintf("test-InfiniBandPartition-batch-value2-%v", i)}, db.Ptr(InfiniBandPartitionStatusDeleting), tn.CreatedBy) } InfiniBandPartitions = append(InfiniBandPartitions, *pt) @@ -418,7 +623,7 @@ func TestInfiniBandPartition_GetAll(t *testing.T) { tenantIDs: nil, siteIDs: nil, orgs: nil, - searchQuery: db.GetStrPtr(InfiniBandPartitionStatusReady), + searchQuery: db.GetTypedStrPtr(InfiniBandPartitionStatusReady), }, wantCount: totalCount / 2, wantTotalCount: totalCount / 2, @@ -434,7 +639,7 @@ func TestInfiniBandPartition_GetAll(t *testing.T) { tenantIDs: nil, siteIDs: nil, orgs: nil, - searchQuery: db.GetStrPtr(InfiniBandPartitionStatusDeleting), + searchQuery: db.GetTypedStrPtr(InfiniBandPartitionStatusDeleting), }, wantCount: totalCount / 2, wantTotalCount: totalCount / 2, @@ -530,7 +735,7 @@ func TestInfiniBandPartition_GetAll(t *testing.T) { tenantIDs: nil, siteIDs: nil, orgs: nil, - statuses: []string{InfiniBandPartitionStatusDeleting}, + statuses: []string{string(InfiniBandPartitionStatusDeleting)}, }, wantCount: totalCount / 2, wantTotalCount: totalCount / 2, @@ -601,7 +806,7 @@ func TestInfiniBandPartitionSQLDAO_Create(t *testing.T) { tenantID uuid.UUID ControllerIBInfiniBandPartitionID *uuid.UUID Labels map[string]string - status string + status InfiniBandPartitionStatus createdBy User } @@ -668,6 +873,25 @@ func TestInfiniBandPartitionSQLDAO_Create(t *testing.T) { wantErr: false, verifyChildSpanner: true, }, + { + name: "create with invalid status returns error", + fields: fields{ + dbSession: dbSession, + }, + args: args{ + ctx: ctx, + name: "invalid-status-ibp", + description: ibpr.Description, + org: ibpr.Org, + tenantID: ibpr.TenantID, + siteID: ibpr.SiteID, + ControllerIBInfiniBandPartitionID: ibpr.ControllerIBPartitionID, + Labels: ibpr.Labels, + status: InfiniBandPartitionStatus("Bogus"), + createdBy: User{ID: ibpr.CreatedBy}, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -690,6 +914,9 @@ func TestInfiniBandPartitionSQLDAO_Create(t *testing.T) { }, ) require.Equal(t, tt.wantErr, err != nil) + if tt.wantErr { + return + } assert.Equal(t, tt.want.Name, got.Name) assert.Equal(t, *tt.want.Description, *got.Description) @@ -728,7 +955,7 @@ func TestInfiniBandPartitionSQLDAO_Update(t *testing.T) { st := testBuildSite(t, dbSession, nil, ip.ID, "test-site", "Test Site", ip.Org, ipu.ID) - pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) uInfiniBandPartition := &InfiniBandPartition{ Name: "test-updated", @@ -755,7 +982,7 @@ func TestInfiniBandPartitionSQLDAO_Update(t *testing.T) { description *string ControllerIBPartitionID *uuid.UUID Labels map[string]string - Status string + Status InfiniBandPartitionStatus IsMissingOnSite bool } tests := []struct { @@ -785,6 +1012,23 @@ func TestInfiniBandPartitionSQLDAO_Update(t *testing.T) { wantErr: false, verifyChildSpanner: true, }, + { + name: "update with invalid status returns error", + fields: fields{ + dbSession: dbSession, + }, + args: args{ + ctx: ctx, + id: pt.ID, + name: &uInfiniBandPartition.Name, + description: uInfiniBandPartition.Description, + ControllerIBPartitionID: uInfiniBandPartition.ControllerIBPartitionID, + Labels: uInfiniBandPartition.Labels, + Status: InfiniBandPartitionStatus("Bogus"), + IsMissingOnSite: uInfiniBandPartition.IsMissingOnSite, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -805,9 +1049,12 @@ func TestInfiniBandPartitionSQLDAO_Update(t *testing.T) { }, ) - fmt.Printf("\ngot ID: %v, Created: %v, Updated: %v", got.ID.String(), got.Created, got.Updated) - require.Equal(t, tt.wantErr, err != nil) + if tt.wantErr { + return + } + + fmt.Printf("\ngot ID: %v, Created: %v, Updated: %v", got.ID.String(), got.Created, got.Updated) assert.Equal(t, tt.want.Name, got.Name) assert.Equal(t, *tt.want.Description, *got.Description) @@ -852,7 +1099,7 @@ func TestInfiniBandPartitionSQLDAO_Delete(t *testing.T) { st := testBuildSite(t, dbSession, nil, ip.ID, "test-site", "Test Site", ip.Org, ipu.ID) - pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) // OTEL Spanner configuration _, _, ctx := testCommonTraceProviderSetup(t, context.Background()) @@ -918,7 +1165,7 @@ func TestInfiniBandPartitionSQLDAO_Clear(t *testing.T) { st := testBuildSite(t, dbSession, nil, ip.ID, "test-site", "Test Site", ip.Org, ipu.ID) - pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.GetStrPtr(InfiniBandPartitionStatusReady), tnu.ID) + pt := testBuildInfiniBandPartition(t, dbSession, nil, "test-InfiniBandPartition", nil, tn.Org, tn.ID, st.ID, db.GetUUIDPtr(uuid.New()), nil, nil, nil, nil, nil, nil, nil, db.Ptr(InfiniBandPartitionStatusReady), tnu.ID) // OTEL Spanner configuration _, _, ctx := testCommonTraceProviderSetup(t, context.Background()) diff --git a/workflow/pkg/activity/infinibandpartition/infinibandpartition.go b/workflow/pkg/activity/infinibandpartition/infinibandpartition.go index cf64d8068..9ef017d89 100644 --- a/workflow/pkg/activity/infinibandpartition/infinibandpartition.go +++ b/workflow/pkg/activity/infinibandpartition/infinibandpartition.go @@ -201,10 +201,12 @@ func (mibp ManageInfiniBandPartition) UpdateInfiniBandPartitionsInDB(ctx context continue } - status, statusMessage := getInfiniBandPartitionStatus(controllerIbp.Status.State) + var status cdbm.InfiniBandPartitionStatus + status.FromProto(controllerIbp.Status.State) - if status != nil && *status != ibp.Status { - err = mibp.updateIBPStatusInDB(ctx, nil, ibp.ID, status, statusMessage) + if status != "" && status != ibp.Status { + message := status.Message() + err = mibp.updateIBPStatusInDB(ctx, nil, ibp.ID, &status, &message) if err != nil { slogger.Error().Err(err).Msg("failed to update InfiniBand Partition status detail in DB") } @@ -264,9 +266,10 @@ func (mibp ManageInfiniBandPartition) UpdateInfiniBandPartitionsInDB(ctx context continue } - serr = mibp.updateIBPStatusInDB(ctx, nil, ibp.ID, cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusError), cdb.GetStrPtr("InfiniBand Partition is missing on Site")) + errStatus := cdbm.InfiniBandPartitionStatusError + serr = mibp.updateIBPStatusInDB(ctx, nil, ibp.ID, &errStatus, cdb.GetStrPtr("InfiniBand Partition is missing on Site")) if serr != nil { - slogger.Error().Err(err).Msg("failed to update InfiniBand Partition status detail in DB") + slogger.Error().Err(serr).Msg("failed to update InfiniBand Partition status detail in DB") } } } @@ -275,7 +278,7 @@ func (mibp ManageInfiniBandPartition) UpdateInfiniBandPartitionsInDB(ctx context } // updateIBPStatusInDB is helper function to write InfiniBandPartition updates to DB -func (mibp ManageInfiniBandPartition) updateIBPStatusInDB(ctx context.Context, tx *cdb.Tx, ibpID uuid.UUID, status *string, statusMessage *string) error { +func (mibp ManageInfiniBandPartition) updateIBPStatusInDB(ctx context.Context, tx *cdb.Tx, ibpID uuid.UUID, status *cdbm.InfiniBandPartitionStatus, statusMessage *string) error { if status != nil { ibpDAO := cdbm.NewInfiniBandPartitionDAO(mibp.dbSession) @@ -292,7 +295,7 @@ func (mibp ManageInfiniBandPartition) updateIBPStatusInDB(ctx context.Context, t } statusDetailDAO := cdbm.NewStatusDetailDAO(mibp.dbSession) - _, err = statusDetailDAO.CreateFromParams(ctx, tx, ibpID.String(), *status, statusMessage) + _, err = statusDetailDAO.CreateFromParams(ctx, tx, ibpID.String(), string(*status), statusMessage) if err != nil { return err } @@ -300,22 +303,6 @@ func (mibp ManageInfiniBandPartition) updateIBPStatusInDB(ctx context.Context, t return nil } -// Utility function to get InfiniBand Partition status from Controller IBPartition state -func getInfiniBandPartitionStatus(controllerIBPartitionTenantState cwssaws.TenantState) (*string, *string) { - switch controllerIBPartitionTenantState { - case cwssaws.TenantState_PROVISIONING: - return cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusProvisioning), cdb.GetStrPtr("InfiniBand Partition is being provisioned on Site") - case cwssaws.TenantState_CONFIGURING: - return cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusConfiguring), cdb.GetStrPtr("InfiniBand Partition is being configured on Site") - case cwssaws.TenantState_READY: - return cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusReady), cdb.GetStrPtr("InfiniBand Partition is ready for use") - case cwssaws.TenantState_FAILED: - return cdb.GetStrPtr(cdbm.InfiniBandPartitionStatusError), cdb.GetStrPtr("InfiniBand Partition is in error state") - default: - return nil, nil - } -} - // NewManageInfiniBandPartition returns a new ManageInfiniBandPartition activity func NewManageInfiniBandPartition(dbSession *cdb.Session, siteClientPool *sc.ClientPool) ManageInfiniBandPartition { return ManageInfiniBandPartition{ diff --git a/workflow/pkg/util/testing.go b/workflow/pkg/util/testing.go index 570244b96..4869ee252 100644 --- a/workflow/pkg/util/testing.go +++ b/workflow/pkg/util/testing.go @@ -272,7 +272,7 @@ func TestBuildSubnet(t *testing.T, dbSession *cdb.Session, tenant *cdbm.Tenant, } // TestBuildInfiniBandPartition builds and returns an InfiniBandPartition -func TestBuildInfiniBandPartition(t *testing.T, dbSession *cdb.Session, name string, site *cdbm.Site, tenant *cdbm.Tenant, controllerIBPartitionID *uuid.UUID, status string, isMissingOnSite bool) *cdbm.InfiniBandPartition { +func TestBuildInfiniBandPartition(t *testing.T, dbSession *cdb.Session, name string, site *cdbm.Site, tenant *cdbm.Tenant, controllerIBPartitionID *uuid.UUID, status cdbm.InfiniBandPartitionStatus, isMissingOnSite bool) *cdbm.InfiniBandPartition { ibp := &cdbm.InfiniBandPartition{ ID: uuid.New(), Name: name,