From 279c3952668afd9ef91564072e98dda45885bc78 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Thu, 6 Aug 2026 13:44:33 -0400 Subject: [PATCH 1/2] Fix status consumer race by persisting instance before dispatch InstanceService.CreateInstance dispatched to the provider before persisting the instance's DB row. Any status event for that instance (e.g. from StatusConsumer) arriving while the create request was still in flight found no record, hit ErrInstanceNotFound, and was unconditionally Ack'd - silently and permanently discarding it. Persist a PENDING placeholder row first, then dispatch, then apply the provider's response status via a new conditional UpdateStatusIfPending so a newer status already applied mid-dispatch is never regressed by the provider's own (possibly stale) synchronous response. Roll the placeholder back via HardDelete if the provider call fails, preserving the existing "no artifact on failed create" contract. This brings the implementation in line with the already-approved sp-resource-manager and sp-resource-status-reader design docs, which both assume the DB row exists before any status event can arrive. Fixes: [FLPATH-4769](https://redhat.atlassian.net/browse/FLPATH-4769) Closes #44 --- Assisted-By: Claude (Anthropic) Signed-off-by: Jordi Gil Co-authored-by: Cursor --- internal/sp/healthcheck/monitor_test.go | 4 + .../resource_manager/service_type_instance.go | 51 +++++-- .../service_type_instance_test.go | 130 +++++++++++++++--- .../resource_manager/service_instance.go | 28 ++++ 4 files changed, 184 insertions(+), 29 deletions(-) diff --git a/internal/sp/healthcheck/monitor_test.go b/internal/sp/healthcheck/monitor_test.go index 45a7141..12eeb12 100644 --- a/internal/sp/healthcheck/monitor_test.go +++ b/internal/sp/healthcheck/monitor_test.go @@ -151,6 +151,10 @@ func (m *mockInstanceStore) UpdateStatus(_ context.Context, _ string, _ string, return nil } +func (m *mockInstanceStore) UpdateStatusIfPending(_ context.Context, _ string, _ string, _ string, _ string) (bool, error) { + return false, nil +} + func (m *mockInstanceStore) MarkForDeletion(_ context.Context, _ string) error { return nil } func (m *mockInstanceStore) ListPendingDeletions(_ context.Context) ([]model.ServiceTypeInstance, error) { diff --git a/internal/sp/service/resource_manager/service_type_instance.go b/internal/sp/service/resource_manager/service_type_instance.go index 27b007c..de49a3d 100644 --- a/internal/sp/service/resource_manager/service_type_instance.go +++ b/internal/sp/service/resource_manager/service_type_instance.go @@ -82,32 +82,61 @@ func (s *InstanceService) CreateInstance(ctx context.Context, request *resource_ return nil, service.NewValidationError("spec.service_type must not be empty") } - // Send request to provider endpoint with the resolved ID - providerResponse, err := s.createInstanceWithProvider(ctx, provider.Endpoint, request, instanceID) - if err != nil { - log.Error("Provider provisioning failed", "instance_id", *instanceID, "provider_name", providerName, "error", err) - return nil, service.NewProviderError(fmt.Sprintf("Error from Provider (%s): %v", providerName, err)) - } - - // Create instance in database + // Persist a placeholder row *before* dispatching to the provider. This closes + // the race where a status event for this instance (e.g. from StatusConsumer) + // arrives while the create request is still in flight: the row already + // exists, so the update applies instead of being silently and permanently + // discarded as ErrInstanceNotFound (see enhancements/sp-resource-manager and + // sp-resource-status-reader design docs). instance := model.ServiceTypeInstance{ ID: *instanceID, ProviderName: providerName, ServiceType: serviceType, - Status: providerResponse.Status, + Status: rmstore.StatusPending, Spec: request.Spec, } created, err := s.store.ServiceTypeInstance().Create(ctx, instance) if err != nil { log.Error("Failed to create instance in store", "instance_id", *instanceID, "error", err) - return nil, service.NewInternalError(fmt.Sprintf("failed to create database record for instance %s: %v", providerResponse.ID, err)) + return nil, service.NewInternalError(fmt.Sprintf("failed to create database record for instance %s: %v", *instanceID, err)) + } + + // Send request to provider endpoint with the resolved ID + providerResponse, err := s.createInstanceWithProvider(ctx, provider.Endpoint, request, instanceID) + if err != nil { + log.Error("Provider provisioning failed", "instance_id", *instanceID, "provider_name", providerName, "error", err) + if delErr := s.store.ServiceTypeInstance().HardDelete(ctx, *instanceID); delErr != nil { + log.Error("Failed to roll back placeholder instance after provider failure", "instance_id", *instanceID, "error", delErr) + } + return nil, service.NewProviderError(fmt.Sprintf("Error from Provider (%s): %v", providerName, err)) + } + + // Apply the provider's response status, but only if a real status event + // hasn't already moved this instance past the PENDING placeholder - a + // newer, real status must win over this synchronous response, which may + // already be stale by the time it arrives (this is the same class of + // hazard this fix closes, one step later in the flow). + applied, err := s.store.ServiceTypeInstance().UpdateStatusIfPending(ctx, *instanceID, rmstore.StatusPending, providerResponse.Status, "") + switch { + case err != nil: + // The row already exists, so a later real status event self-heals it - + // this isn't a full "lost create" like the bug this replaces. + log.Warn("Failed to apply post-dispatch status update; a later status event will self-heal", "instance_id", *instanceID, "error", err) + case applied: + created.Status = providerResponse.Status + default: + if refreshed, getErr := s.store.ServiceTypeInstance().Get(ctx, *instanceID, false); getErr == nil { + created = refreshed + } else { + log.Warn("Failed to re-fetch instance after a newer status event pre-empted the provider response", "instance_id", *instanceID, "error", getErr) + } } log.Info("Instance created successfully", "instance_id", created.ID, "provider_name", providerName, - "status", providerResponse.Status, + "status", created.Status, ) return ModelToAPI(created), nil } diff --git a/internal/sp/service/resource_manager/service_type_instance_test.go b/internal/sp/service/resource_manager/service_type_instance_test.go index a2f4340..a5f1992 100644 --- a/internal/sp/service/resource_manager/service_type_instance_test.go +++ b/internal/sp/service/resource_manager/service_type_instance_test.go @@ -13,6 +13,7 @@ import ( rmsvc "github.com/dcm-project/control-plane/internal/sp/service/resource_manager" "github.com/dcm-project/control-plane/internal/sp/store" "github.com/dcm-project/control-plane/internal/sp/store/model" + "github.com/dcm-project/control-plane/internal/sp/testutil" "github.com/go-resty/resty/v2" "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" @@ -69,7 +70,7 @@ var _ = Describe("InstanceService", func() { } Expect(db.Create(&provider).Error).NotTo(HaveOccurred()) - dataStore = store.NewStore(db) + dataStore = store.NewStore(db, store.WithServiceTypeInstanceRetry(testutil.FastServiceTypeInstanceRetry()...)) instanceService = rmsvc.NewInstanceService(dataStore, resty.New(). SetTimeout(5*time.Second). SetRetryCount(0)) @@ -342,18 +343,56 @@ var _ = Describe("InstanceService", func() { Expect(providerCalled).To(BeFalse()) }) - It("returns internal error with instance ID when DB insert fails", func() { - var instanceID string - var providerCallCount int - mockProviderWithID := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - providerCallCount++ - instanceID = uuid.New().String() + It("applies a status update that arrives while the create request is still in flight to the provider (BAC-1)", func() { + // The mock provider's handler is the exact dispatch point: while the + // synchronous create call to the provider is still outstanding, simulate + // a status event for this same instance arriving via the real production + // code path (StatusConsumer.handleMessage calls this same store method). + // The handler runs on httptest's own goroutine, so the result must cross + // back to the test over a channel rather than a shared variable. + statusUpdateErrCh := make(chan error, 1) + mockProviderMidDispatch := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + instanceID := r.URL.Query().Get("id") + statusUpdateErrCh <- dataStore.ServiceTypeInstance().UpdateStatus(ctx, instanceID, "RUNNING", "provisioning started") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{ + "id": instanceID, + "status": "PROVISIONING", + }) + })) + defer mockProviderMidDispatch.Close() - if providerCallCount == 1 { - sqlDB, _ := db.DB() - _ = sqlDB.Close() - } + providerMidDispatch := model.Provider{ + ID: uuid.New().String(), + Name: "provider-mid-dispatch", + ServiceType: "vm", + Endpoint: mockProviderMidDispatch.URL, + HealthStatus: model.HealthStatusReady, + } + Expect(db.Create(&providerMidDispatch).Error).NotTo(HaveOccurred()) + req := &resource_manager.ServiceTypeInstance{ + ProviderName: "provider-mid-dispatch", + Spec: map[string]interface{}{"cpu": 1, "service_type": "vm"}, + } + + _, err := instanceService.CreateInstance(ctx, req, nil) + Expect(err).NotTo(HaveOccurred()) + + statusUpdateErr := <-statusUpdateErrCh + Expect(statusUpdateErr).NotTo(HaveOccurred()) + }) + + It("does not let the provider's own synchronous response regress a newer status already applied mid-dispatch (BAC-5)", func() { + // Same mid-dispatch event as BAC-1, but the provider's own HTTP response + // reports a stale status ("PROVISIONING") relative to the async event + // that already landed ("RUNNING"). The final record must keep the newer, + // real status, not whatever the synchronous response says. + statusUpdateErrCh := make(chan error, 1) + mockProviderStaleResponse := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + instanceID := r.URL.Query().Get("id") + statusUpdateErrCh <- dataStore.ServiceTypeInstance().UpdateStatus(ctx, instanceID, "RUNNING", "provisioning started") w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) _ = json.NewEncoder(w).Encode(map[string]string{ @@ -361,19 +400,73 @@ var _ = Describe("InstanceService", func() { "status": "PROVISIONING", }) })) - defer mockProviderWithID.Close() + defer mockProviderStaleResponse.Close() - providerWithID := model.Provider{ + providerStaleResponse := model.Provider{ ID: uuid.New().String(), - Name: "provider-db-fail", + Name: "provider-stale-response", ServiceType: "vm", - Endpoint: mockProviderWithID.URL, + Endpoint: mockProviderStaleResponse.URL, HealthStatus: model.HealthStatusReady, } - Expect(db.Create(&providerWithID).Error).NotTo(HaveOccurred()) + Expect(db.Create(&providerStaleResponse).Error).NotTo(HaveOccurred()) req := &resource_manager.ServiceTypeInstance{ - ProviderName: "provider-db-fail", + ProviderName: "provider-stale-response", + Spec: map[string]interface{}{"cpu": 1, "service_type": "vm"}, + } + + result, err := instanceService.CreateInstance(ctx, req, nil) + Expect(err).NotTo(HaveOccurred()) + Expect(<-statusUpdateErrCh).NotTo(HaveOccurred()) + + fetched, err := instanceService.GetInstance(ctx, *result.Id, false) + Expect(err).NotTo(HaveOccurred()) + Expect(fetched.Status).NotTo(BeNil()) + Expect(*fetched.Status).To(Equal("RUNNING")) + }) + + It("does not leave an orphaned instance visible after a provider failure (BAC-2)", func() { + mockProviderFailing := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error": "boom"}`)) + })) + defer mockProviderFailing.Close() + + providerFailing := model.Provider{ + ID: uuid.New().String(), + Name: "provider-failing", + ServiceType: "vm", + Endpoint: mockProviderFailing.URL, + HealthStatus: model.HealthStatusReady, + } + Expect(db.Create(&providerFailing).Error).NotTo(HaveOccurred()) + + specifiedID := uuid.New().String() + req := &resource_manager.ServiceTypeInstance{ + ProviderName: "provider-failing", + Spec: map[string]interface{}{"cpu": 1, "service_type": "vm"}, + } + + _, err := instanceService.CreateInstance(ctx, req, &specifiedID) + Expect(err).To(HaveOccurred()) + + _, err = instanceService.GetInstance(ctx, specifiedID, false) + Expect(err).To(HaveOccurred()) + var svcErr *service.ServiceError + Expect(err).To(BeAssignableToTypeOf(svcErr)) + errors.As(err, &svcErr) + Expect(svcErr.Code).To(Equal(service.ErrCodeNotFound)) + }) + + It("returns internal error naming the instance id and never contacts the provider when the DB persist fails (BAC-4)", func() { + // Drop only the service_type_instances table so the provider lookup + // (a separate table) still succeeds, and the failure is isolated to the + // initial persist step - before the provider is ever dispatched to. + Expect(db.Migrator().DropTable(&model.ServiceTypeInstance{})).To(Succeed()) + + req := &resource_manager.ServiceTypeInstance{ + ProviderName: "test-provider", Spec: map[string]interface{}{"cpu": 2, "service_type": "vm"}, } @@ -385,7 +478,8 @@ var _ = Describe("InstanceService", func() { errors.As(err, &svcErr) Expect(svcErr.Code).To(Equal(service.ErrCodeInternal)) Expect(svcErr.Message).To(ContainSubstring("failed to create database record")) - Expect(svcErr.Message).To(ContainSubstring(instanceID)) + Expect(svcErr.Message).To(MatchRegexp(`[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`)) + Expect(providerCalled).To(BeFalse()) }) }) diff --git a/internal/sp/store/resource_manager/service_instance.go b/internal/sp/store/resource_manager/service_instance.go index d54b4e7..e058363 100644 --- a/internal/sp/store/resource_manager/service_instance.go +++ b/internal/sp/store/resource_manager/service_instance.go @@ -38,6 +38,7 @@ type ServiceTypeInstance interface { //nolint:interfacebloat Get(ctx context.Context, id string, showDeleted bool) (*model.ServiceTypeInstance, error) ExistsByID(ctx context.Context, id string) (bool, error) UpdateStatus(ctx context.Context, instanceID string, status string, statusMessage string) error + UpdateStatusIfPending(ctx context.Context, instanceID string, pendingStatus string, status string, statusMessage string) (bool, error) MarkForDeletion(ctx context.Context, id string) error ListPendingDeletions(ctx context.Context) ([]model.ServiceTypeInstance, error) IncrementDeletionRetry(ctx context.Context, id string) error @@ -174,6 +175,25 @@ func (s *ServiceTypeInstanceStore) UpdateStatus(ctx context.Context, instanceID return nil } +// UpdateStatusIfPending applies status/statusMessage only if the instance's +// current status still equals pendingStatus. It reports applied=false (with +// no error) if the row has already moved past that placeholder - e.g. a real +// status event landed first - so callers can defer to that newer status +// instead of overwriting it. +func (s *ServiceTypeInstanceStore) UpdateStatusIfPending(ctx context.Context, instanceID string, pendingStatus string, status string, statusMessage string) (bool, error) { + result := s.db.WithContext(ctx). + Model(&model.ServiceTypeInstance{}). + Where("id = ? AND status = ?", instanceID, pendingStatus). + Updates(model.ServiceTypeInstance{ + Status: status, + StatusMessage: statusMessage, + }) + if result.Error != nil { + return false, result.Error + } + return result.RowsAffected > 0, nil +} + func (s *ServiceTypeInstanceStore) ExistsByID(ctx context.Context, id string) (bool, error) { var instance model.ServiceTypeInstance err := s.db.WithContext(ctx).Select("id").Where("id = ?", id).Take(&instance).Error @@ -192,6 +212,14 @@ const ( DeletionStatusPendingProvider = "PENDING_PROVIDER" ) +// StatusPending is the placeholder status an instance record is created with +// before its create request is dispatched to the provider. Persisting this +// row first (rather than after a successful dispatch) closes the window +// where a status event for the instance could arrive before any record of +// it exists - see enhancements/sp-resource-manager and +// sp-resource-status-reader design docs. +const StatusPending = "PENDING" + func (s *ServiceTypeInstanceStore) MarkForDeletion(ctx context.Context, id string) error { now := time.Now() result := s.db.WithContext(ctx). From 2bee5403cfb8b14e2010f8e4a58aa4be749eed36 Mon Sep 17 00:00:00 2001 From: Jordi Gil Date: Thu, 6 Aug 2026 19:58:10 -0400 Subject: [PATCH 2/2] fix(sp): drop synchronous provider status write, superseded by StatusConsumer CreateInstance persisted the provider's synchronous HTTP response status (via UpdateStatusIfPending) after a successful dispatch. Per-service-type status vocabularies are well-defined and providers follow them correctly, but the write itself added complexity for no real benefit: it's designed to lose to a real status event, and StatusConsumer's async update supersedes it within ~100ms-2s in practice. Nothing downstream reads the value off the create response either. Drop the post-dispatch UpdateStatusIfPending call and its applied/re-fetch branching - the row stays PENDING after a successful dispatch, and StatusConsumer owns every subsequent transition. Remove BAC-5 and its test, since it only existed to guard this write, and delete UpdateStatusIfPending itself (store method + interface + mock stub) now that CreateInstance was its only caller. Addresses review feedback on #46. --- Assisted-By: Claude (Anthropic) Signed-off-by: Jordi Gil Co-authored-by: Cursor Signed-off-by: Jordi Gil --- internal/sp/healthcheck/monitor_test.go | 4 -- .../resource_manager/service_type_instance.go | 28 +++---------- .../service_type_instance_test.go | 42 ------------------- .../resource_manager/service_instance.go | 20 --------- 4 files changed, 5 insertions(+), 89 deletions(-) diff --git a/internal/sp/healthcheck/monitor_test.go b/internal/sp/healthcheck/monitor_test.go index 12eeb12..45a7141 100644 --- a/internal/sp/healthcheck/monitor_test.go +++ b/internal/sp/healthcheck/monitor_test.go @@ -151,10 +151,6 @@ func (m *mockInstanceStore) UpdateStatus(_ context.Context, _ string, _ string, return nil } -func (m *mockInstanceStore) UpdateStatusIfPending(_ context.Context, _ string, _ string, _ string, _ string) (bool, error) { - return false, nil -} - func (m *mockInstanceStore) MarkForDeletion(_ context.Context, _ string) error { return nil } func (m *mockInstanceStore) ListPendingDeletions(_ context.Context) ([]model.ServiceTypeInstance, error) { diff --git a/internal/sp/service/resource_manager/service_type_instance.go b/internal/sp/service/resource_manager/service_type_instance.go index de49a3d..bbc875e 100644 --- a/internal/sp/service/resource_manager/service_type_instance.go +++ b/internal/sp/service/resource_manager/service_type_instance.go @@ -103,8 +103,7 @@ func (s *InstanceService) CreateInstance(ctx context.Context, request *resource_ } // Send request to provider endpoint with the resolved ID - providerResponse, err := s.createInstanceWithProvider(ctx, provider.Endpoint, request, instanceID) - if err != nil { + if _, err := s.createInstanceWithProvider(ctx, provider.Endpoint, request, instanceID); err != nil { log.Error("Provider provisioning failed", "instance_id", *instanceID, "provider_name", providerName, "error", err) if delErr := s.store.ServiceTypeInstance().HardDelete(ctx, *instanceID); delErr != nil { log.Error("Failed to roll back placeholder instance after provider failure", "instance_id", *instanceID, "error", delErr) @@ -112,27 +111,10 @@ func (s *InstanceService) CreateInstance(ctx context.Context, request *resource_ return nil, service.NewProviderError(fmt.Sprintf("Error from Provider (%s): %v", providerName, err)) } - // Apply the provider's response status, but only if a real status event - // hasn't already moved this instance past the PENDING placeholder - a - // newer, real status must win over this synchronous response, which may - // already be stale by the time it arrives (this is the same class of - // hazard this fix closes, one step later in the flow). - applied, err := s.store.ServiceTypeInstance().UpdateStatusIfPending(ctx, *instanceID, rmstore.StatusPending, providerResponse.Status, "") - switch { - case err != nil: - // The row already exists, so a later real status event self-heals it - - // this isn't a full "lost create" like the bug this replaces. - log.Warn("Failed to apply post-dispatch status update; a later status event will self-heal", "instance_id", *instanceID, "error", err) - case applied: - created.Status = providerResponse.Status - default: - if refreshed, getErr := s.store.ServiceTypeInstance().Get(ctx, *instanceID, false); getErr == nil { - created = refreshed - } else { - log.Warn("Failed to re-fetch instance after a newer status event pre-empted the provider response", "instance_id", *instanceID, "error", getErr) - } - } - + // The row stays PENDING here - StatusConsumer owns every status transition + // from this point on. The provider's synchronous response isn't persisted: + // it's immediately superseded by StatusConsumer's async update in practice, + // and nothing downstream reads it off the create response. log.Info("Instance created successfully", "instance_id", created.ID, "provider_name", providerName, diff --git a/internal/sp/service/resource_manager/service_type_instance_test.go b/internal/sp/service/resource_manager/service_type_instance_test.go index a5f1992..59ecc52 100644 --- a/internal/sp/service/resource_manager/service_type_instance_test.go +++ b/internal/sp/service/resource_manager/service_type_instance_test.go @@ -384,48 +384,6 @@ var _ = Describe("InstanceService", func() { Expect(statusUpdateErr).NotTo(HaveOccurred()) }) - It("does not let the provider's own synchronous response regress a newer status already applied mid-dispatch (BAC-5)", func() { - // Same mid-dispatch event as BAC-1, but the provider's own HTTP response - // reports a stale status ("PROVISIONING") relative to the async event - // that already landed ("RUNNING"). The final record must keep the newer, - // real status, not whatever the synchronous response says. - statusUpdateErrCh := make(chan error, 1) - mockProviderStaleResponse := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - instanceID := r.URL.Query().Get("id") - statusUpdateErrCh <- dataStore.ServiceTypeInstance().UpdateStatus(ctx, instanceID, "RUNNING", "provisioning started") - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(map[string]string{ - "id": instanceID, - "status": "PROVISIONING", - }) - })) - defer mockProviderStaleResponse.Close() - - providerStaleResponse := model.Provider{ - ID: uuid.New().String(), - Name: "provider-stale-response", - ServiceType: "vm", - Endpoint: mockProviderStaleResponse.URL, - HealthStatus: model.HealthStatusReady, - } - Expect(db.Create(&providerStaleResponse).Error).NotTo(HaveOccurred()) - - req := &resource_manager.ServiceTypeInstance{ - ProviderName: "provider-stale-response", - Spec: map[string]interface{}{"cpu": 1, "service_type": "vm"}, - } - - result, err := instanceService.CreateInstance(ctx, req, nil) - Expect(err).NotTo(HaveOccurred()) - Expect(<-statusUpdateErrCh).NotTo(HaveOccurred()) - - fetched, err := instanceService.GetInstance(ctx, *result.Id, false) - Expect(err).NotTo(HaveOccurred()) - Expect(fetched.Status).NotTo(BeNil()) - Expect(*fetched.Status).To(Equal("RUNNING")) - }) - It("does not leave an orphaned instance visible after a provider failure (BAC-2)", func() { mockProviderFailing := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusInternalServerError) diff --git a/internal/sp/store/resource_manager/service_instance.go b/internal/sp/store/resource_manager/service_instance.go index e058363..029d181 100644 --- a/internal/sp/store/resource_manager/service_instance.go +++ b/internal/sp/store/resource_manager/service_instance.go @@ -38,7 +38,6 @@ type ServiceTypeInstance interface { //nolint:interfacebloat Get(ctx context.Context, id string, showDeleted bool) (*model.ServiceTypeInstance, error) ExistsByID(ctx context.Context, id string) (bool, error) UpdateStatus(ctx context.Context, instanceID string, status string, statusMessage string) error - UpdateStatusIfPending(ctx context.Context, instanceID string, pendingStatus string, status string, statusMessage string) (bool, error) MarkForDeletion(ctx context.Context, id string) error ListPendingDeletions(ctx context.Context) ([]model.ServiceTypeInstance, error) IncrementDeletionRetry(ctx context.Context, id string) error @@ -175,25 +174,6 @@ func (s *ServiceTypeInstanceStore) UpdateStatus(ctx context.Context, instanceID return nil } -// UpdateStatusIfPending applies status/statusMessage only if the instance's -// current status still equals pendingStatus. It reports applied=false (with -// no error) if the row has already moved past that placeholder - e.g. a real -// status event landed first - so callers can defer to that newer status -// instead of overwriting it. -func (s *ServiceTypeInstanceStore) UpdateStatusIfPending(ctx context.Context, instanceID string, pendingStatus string, status string, statusMessage string) (bool, error) { - result := s.db.WithContext(ctx). - Model(&model.ServiceTypeInstance{}). - Where("id = ? AND status = ?", instanceID, pendingStatus). - Updates(model.ServiceTypeInstance{ - Status: status, - StatusMessage: statusMessage, - }) - if result.Error != nil { - return false, result.Error - } - return result.RowsAffected > 0, nil -} - func (s *ServiceTypeInstanceStore) ExistsByID(ctx context.Context, id string) (bool, error) { var instance model.ServiceTypeInstance err := s.db.WithContext(ctx).Select("id").Where("id = ?", id).Take(&instance).Error