From 876dd32491fa1d536926094d977630287e928e56 Mon Sep 17 00:00:00 2001 From: Matthew Barnes Date: Fri, 5 Jun 2026 08:06:51 -0400 Subject: [PATCH] feat(frontend): Remove the subscription lock Now that Clusters Service calls have been moved to the RP backend, the frontend pods need only write to Cosmos DB before responding to client requests. This can be done atomically using Cosmos DB's Transactional Batch Operations and therefore the cloud-distributed locking pattern is no longer needed. The locking pattern was a bottleneck in high-traffic subscriptions and was the source of much latency in the Azure control plane. --- frontend/cmd/cmd.go | 7 +- frontend/pkg/frontend/frontend.go | 3 - frontend/pkg/frontend/frontend_test.go | 5 - .../frontend/middleware_locksubscription.go | 103 ------- .../middleware_locksubscription_test.go | 100 ------- frontend/pkg/frontend/routes.go | 4 +- frontend/pkg/frontend/testhelpers.go | 2 - internal/database/lock.go | 272 ------------------ internal/database/locks_db_client.go | 53 ---- .../databasetesting/mock_dbclient_test.go | 35 --- .../databasetesting/mock_locks_db_client.go | 119 -------- .../utils/integrationutils/cosmos_testinfo.go | 10 - .../integrationutils/frontend_testinfo.go | 1 - .../integrationutils/mock_cosmos_testinfo.go | 7 - .../utils/integrationutils/utils.go | 2 +- 15 files changed, 3 insertions(+), 720 deletions(-) delete mode 100644 frontend/pkg/frontend/middleware_locksubscription.go delete mode 100644 frontend/pkg/frontend/middleware_locksubscription_test.go delete mode 100644 internal/database/lock.go delete mode 100644 internal/database/locks_db_client.go delete mode 100644 internal/databasetesting/mock_locks_db_client.go diff --git a/frontend/cmd/cmd.go b/frontend/cmd/cmd.go index 1ee2c6a964c..cce84d8f776 100644 --- a/frontend/cmd/cmd.go +++ b/frontend/cmd/cmd.go @@ -230,11 +230,6 @@ func (opts *FrontendOpts) Run() error { return fmt.Errorf("failed to create the resources database client: %w", err) } - locksDBClient, err := database.NewLocksDBClient(ctx, cosmosDatabaseClient) - if err != nil { - return fmt.Errorf("failed to create the locks database client: %w", err) - } - listener, err := net.Listen("tcp4", fmt.Sprintf(":%d", opts.port)) if err != nil { return err @@ -269,7 +264,7 @@ func (opts *FrontendOpts) Run() error { f := frontend.NewFrontend( logger, listener, metricsListener, legacyregistry.Registerer(), legacyregistry.DefaultGatherer, - resourcesDBClient, locksDBClient, csClient, auditClient, opts.location, opts.clusterServiceProvisionShard, + resourcesDBClient, csClient, auditClient, opts.location, opts.clusterServiceProvisionShard, opts.clusterServiceNoopProvision, opts.clusterServiceNoopDeprovision, opts.exitOnPanic, ) diff --git a/frontend/pkg/frontend/frontend.go b/frontend/pkg/frontend/frontend.go index 5a47e3e8ad8..e10606c713b 100644 --- a/frontend/pkg/frontend/frontend.go +++ b/frontend/pkg/frontend/frontend.go @@ -59,7 +59,6 @@ type Frontend struct { server http.Server metricsServer http.Server resourcesDBClient database.ResourcesDBClient - locksDBClient database.LocksDBClient auditClient audit.Client collector *metrics.SubscriptionCollector healthGauge prometheus.Gauge @@ -88,7 +87,6 @@ func NewFrontend( registerer prometheus.Registerer, gatherer prometheus.Gatherer, resourcesDBClient database.ResourcesDBClient, - locksDBClient database.LocksDBClient, csClient ocm.ClusterServiceClientSpec, auditClient audit.Client, azureLocation string, @@ -120,7 +118,6 @@ func NewFrontend( }, auditClient: auditClient, resourcesDBClient: resourcesDBClient, - locksDBClient: locksDBClient, collector: metrics.NewSubscriptionCollector(registerer, resourcesDBClient, azureLocation), clusterServiceProvisionShard: clusterServiceProvisionShard, clusterServiceNoopProvision: clusterServiceNoopProvision, diff --git a/frontend/pkg/frontend/frontend_test.go b/frontend/pkg/frontend/frontend_test.go index e8fa7179a6d..fe598c2a690 100644 --- a/frontend/pkg/frontend/frontend_test.go +++ b/frontend/pkg/frontend/frontend_test.go @@ -100,7 +100,6 @@ func TestSubscriptionsGET(t *testing.T) { reg, reg, mockResourcesDBClient, - databasetesting.NewMockLocksDBClient(), nil, newNoopAuditClient(t), api.TestLocation, @@ -250,7 +249,6 @@ func TestSubscriptionsPUT(t *testing.T) { reg, reg, mockResourcesDBClient, - databasetesting.NewMockLocksDBClient(), nil, newNoopAuditClient(t), api.TestLocation, @@ -466,7 +464,6 @@ func TestDeploymentPreflight(t *testing.T) { reg, reg, mockResourcesDBClient, - databasetesting.NewMockLocksDBClient(), nil, newNoopAuditClient(t), api.TestLocation, @@ -592,7 +589,6 @@ func TestRequestAdminCredential(t *testing.T) { reg, reg, mockResourcesDBClient, - databasetesting.NewMockLocksDBClient(), nil, newNoopAuditClient(t), api.TestLocation, @@ -705,7 +701,6 @@ func TestRevokeCredentials(t *testing.T) { reg, reg, mockResourcesDBClient, - databasetesting.NewMockLocksDBClient(), nil, newNoopAuditClient(t), api.TestLocation, diff --git a/frontend/pkg/frontend/middleware_locksubscription.go b/frontend/pkg/frontend/middleware_locksubscription.go deleted file mode 100644 index 303477406f6..00000000000 --- a/frontend/pkg/frontend/middleware_locksubscription.go +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright 2025 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package frontend - -import ( - "context" - "errors" - "net/http" - - "github.com/Azure/ARO-HCP/internal/api/arm" - "github.com/Azure/ARO-HCP/internal/database" - "github.com/Azure/ARO-HCP/internal/utils" -) - -type middlewareLockSubscription struct { - locksDBClient database.LocksDBClient -} - -func newMiddlewareLockSubscription(locksDBClient database.LocksDBClient) *middlewareLockSubscription { - return &middlewareLockSubscription{ - locksDBClient: locksDBClient, - } -} - -// handleRequest this is best effort, not guaranteed correct. This must not be relied upon for guaranteeing correctness. -func (h *middlewareLockSubscription) handleRequest(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { - ctx := r.Context() - logger := utils.LoggerFromContext(ctx) - - subscriptionID := r.PathValue(PathSegmentSubscriptionID) - - // This may be nil when running "go test". - lockClient := h.locksDBClient.LockClient() - - if lockClient == nil { - next(w, r) - } else { - // Wait for the default TTL to acquire lock. - timeout := lockClient.GetDefaultTimeToLive() - lock, err := lockClient.AcquireLock(ctx, subscriptionID, &timeout) - if err != nil { - message := "Failed to acquire lock: " - if errors.Is(err, context.DeadlineExceeded) { - message += "timed out" - lockClient.SetRetryAfterHeader(w.Header()) - arm.WriteError( - w, http.StatusServiceUnavailable, - arm.CloudErrorCodeLockContention, - "/subscriptions/"+subscriptionID, "%s", message) - } else { - message += err.Error() - arm.WriteInternalServerError(w) - } - logger.Error(err, message) - return - } - logger.Info("Acquired lock") - - // Hold the lock until the remaining handlers complete. - // If we lose the lock the context will be cancelled. - // TODO this implementation is racy. If the internal c.RenewLock fails, but does not return quickly then a second lock can be acquired. - lockedCtx, stop := lockClient.HoldLock(ctx, lock) - defer func() { - lock = stop() - if lock != nil { - // Release should work even if the work of the request is cancelled. Prefer the standard context if it isn't - // cancelled. If it is cancelled, create a new context and attach a logger to it. - releaseContext := ctx - if ctx.Err() != nil { - var releaseCancel context.CancelFunc - releaseContext, releaseCancel = context.WithTimeout(context.Background(), lockClient.GetDefaultTimeToLive()) - defer releaseCancel() - releaseContext = utils.ContextWithLogger(releaseContext, logger) - } - - err = lockClient.ReleaseLock(releaseContext, lock) - if err == nil { - logger.Info("Released lock") - } else { - // Failure here is non-fatal but still log the error. - // The lock's TTL ensures it will be released eventually. - logger.Error(err, "Failed to release lock") - } - } - }() - - r = r.WithContext(lockedCtx) - - next(w, r) - } -} diff --git a/frontend/pkg/frontend/middleware_locksubscription_test.go b/frontend/pkg/frontend/middleware_locksubscription_test.go deleted file mode 100644 index e1963f03ea2..00000000000 --- a/frontend/pkg/frontend/middleware_locksubscription_test.go +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright 2025 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package frontend - -import ( - "context" - "fmt" - "net/http" - "net/http/httptest" - "testing" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos" - - "github.com/Azure/ARO-HCP/internal/database" - "github.com/Azure/ARO-HCP/internal/databasetesting" -) - -// trackingLockClient is a test double that tracks whether stop was called. -type trackingLockClient struct { - stopWasCalled *bool - defaultTTL time.Duration -} - -func (c *trackingLockClient) GetDefaultTimeToLive() time.Duration { - return c.defaultTTL -} - -func (c *trackingLockClient) SetRetryAfterHeader(header http.Header) { - header.Set("Retry-After", fmt.Sprintf("%d", int(c.defaultTTL.Seconds()))) -} - -func (c *trackingLockClient) AcquireLock(ctx context.Context, id string, timeout *time.Duration) (*azcosmos.ItemResponse, error) { - return &azcosmos.ItemResponse{}, nil -} - -func (c *trackingLockClient) TryAcquireLock(ctx context.Context, id string) (*azcosmos.ItemResponse, error) { - return &azcosmos.ItemResponse{}, nil -} - -func (c *trackingLockClient) HoldLock(ctx context.Context, item *azcosmos.ItemResponse) (context.Context, database.StopHoldLock) { - return ctx, func() *azcosmos.ItemResponse { - *c.stopWasCalled = true - return nil - } -} - -func (c *trackingLockClient) RenewLock(ctx context.Context, item *azcosmos.ItemResponse) (*azcosmos.ItemResponse, error) { - return item, nil -} - -func (c *trackingLockClient) ReleaseLock(ctx context.Context, item *azcosmos.ItemResponse) error { - return nil -} - -var _ database.LockClientInterface = &trackingLockClient{} - -func TestMiddlewareLockSubscription(t *testing.T) { - panicingHandler := func(writer http.ResponseWriter, request *http.Request) { - panic("force failure") - } - - stopWasCalled := false - ctx := context.Background() - mockLocksDBClient := databasetesting.NewMockLocksDBClient() - mockLocksDBClient.SetLockClient(&trackingLockClient{ - stopWasCalled: &stopWasCalled, - defaultTTL: 10 * time.Second, - }) - - request := httptest.NewRequestWithContext(ctx, "PUT", "http://example.com", nil) - request.SetPathValue(PathSegmentSubscriptionID, "TheSubscriptionID") - response := httptest.NewRecorder() - - func() { - defer func() { - if r := recover(); r != nil { - fmt.Println("Recovery as expected", r) - } - }() - - newMiddlewareLockSubscription(mockLocksDBClient).handleRequest(response, request, panicingHandler) - }() - - if !stopWasCalled { - t.Error("stop was not called") - } -} diff --git a/frontend/pkg/frontend/routes.go b/frontend/pkg/frontend/routes.go index 7cd9ae4cc85..a0b9294a783 100644 --- a/frontend/pkg/frontend/routes.go +++ b/frontend/pkg/frontend/routes.go @@ -129,7 +129,6 @@ func (f *Frontend) routes(r prometheus.Registerer) http.Handler { MiddlewareResourceID, MiddlewareLoggingPostMux, newMiddlewareValidatedAPIVersion(f.apiRegistry).handleRequest, - newMiddlewareLockSubscription(f.locksDBClient).handleRequest, newMiddlewareValidateSubscriptionState(f.resourcesDBClient).handleRequest) middlewareMux.Handle( MuxPattern(http.MethodPut, PatternSubscriptions, PatternResourceGroups, PatternProviders, PatternClusters), @@ -188,8 +187,7 @@ func (f *Frontend) routes(r prometheus.Registerer) http.Handler { postMuxMiddleware.HandlerFunc(errorutils.ReportError(f.ArmSubscriptionGet))) postMuxMiddleware = NewMiddleware( MiddlewareResourceID, - MiddlewareLoggingPostMux, - newMiddlewareLockSubscription(f.locksDBClient).handleRequest) + MiddlewareLoggingPostMux) middlewareMux.Handle( MuxPattern(http.MethodPut, PatternSubscriptions), postMuxMiddleware.HandlerFunc(errorutils.ReportError(f.ArmSubscriptionPut))) diff --git a/frontend/pkg/frontend/testhelpers.go b/frontend/pkg/frontend/testhelpers.go index 166781c0432..df39ee7c7f8 100644 --- a/frontend/pkg/frontend/testhelpers.go +++ b/frontend/pkg/frontend/testhelpers.go @@ -36,7 +36,6 @@ func newNoopAuditClient(t *testing.T) *audit.AuditClient { func NewTestFrontend(t *testing.T) *Frontend { mockResourcesDBClient := databasetesting.NewMockResourcesDBClient() - mockLocksDBClient := databasetesting.NewMockLocksDBClient() reg := prometheus.NewRegistry() f := NewFrontend( @@ -46,7 +45,6 @@ func NewTestFrontend(t *testing.T) *Frontend { reg, reg, mockResourcesDBClient, - mockLocksDBClient, nil, newNoopAuditClient(t), api.TestLocation, diff --git a/internal/database/lock.go b/internal/database/lock.go deleted file mode 100644 index 24141d8a9b3..00000000000 --- a/internal/database/lock.go +++ /dev/null @@ -1,272 +0,0 @@ -// Copyright 2025 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package database - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - "os" - "strconv" - "time" - - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - - "github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos" -) - -// Copied from azcore/internal/shared/shared.go -func Delay(ctx context.Context, delay time.Duration) error { - select { - case <-time.After(delay): - return nil - case <-ctx.Done(): - return ctx.Err() - } -} - -type LockClientInterface interface { - GetDefaultTimeToLive() time.Duration - SetRetryAfterHeader(header http.Header) - AcquireLock(ctx context.Context, id string, timeout *time.Duration) (*azcosmos.ItemResponse, error) - TryAcquireLock(ctx context.Context, id string) (*azcosmos.ItemResponse, error) - HoldLock(ctx context.Context, item *azcosmos.ItemResponse) (context.Context, StopHoldLock) - RenewLock(ctx context.Context, item *azcosmos.ItemResponse) (*azcosmos.ItemResponse, error) - ReleaseLock(ctx context.Context, item *azcosmos.ItemResponse) error -} - -type LockClient struct { - name string - containerClient *azcosmos.ContainerClient - defaultTimeToLive int32 -} - -// lockDocument implements a global distributed lock. -// Its contents should be opaque outside of LockClient. -type lockDocument struct { - BaseDocument - Owner string `json:"owner,omitempty"` - TTL int32 `json:"ttl,omitempty"` -} - -// NewLockClient creates a LockClient around a ContainerClient. It attempts to -// read container properties to extract a default TTL. If this fails or if the -// container does not define a default TTL, the function returns an error. -func NewLockClient(ctx context.Context, containerClient *azcosmos.ContainerClient) (*LockClient, error) { - hostname, err := os.Hostname() - if err != nil { - return nil, err - } - - c := &LockClient{ - name: hostname, - containerClient: containerClient, - } - - response, err := containerClient.Read(ctx, nil) - if err != nil { - return nil, err - } - - if response.ContainerProperties != nil && response.ContainerProperties.DefaultTimeToLive != nil { - c.defaultTimeToLive = *response.ContainerProperties.DefaultTimeToLive - } else { - return nil, fmt.Errorf("container '%s' does not have a default TTL", containerClient.ID()) - } - - return c, nil -} - -// SetName overrides how a lock item identifies the owner. This is for -// informational purposes only. LockClient uses the hostname by default. -func (c *LockClient) SetName(name string) { - c.name = name -} - -// GetDefaultTimeToLive returns the default time-to-live value of the -// container as a time.Duration. -func (c *LockClient) GetDefaultTimeToLive() time.Duration { - return time.Duration(c.defaultTimeToLive) * time.Second -} - -// SetRetryAfterHeader sets a "Retry-After" header to the default TTL value. -func (c *LockClient) SetRetryAfterHeader(header http.Header) { - header.Set("Retry-After", strconv.Itoa(int(c.defaultTimeToLive))) -} - -// AcquireLock persistently tries to acquire a lock for the given ID. If a -// timeout is provided, the function will cease after the timeout duration -// and return a context.DeadlineExceeded error. -func (c *LockClient) AcquireLock(ctx context.Context, id string, timeout *time.Duration) (*azcosmos.ItemResponse, error) { - var lock *azcosmos.ItemResponse - - if timeout != nil { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, *timeout) - defer cancel() - } - - for lock == nil { - var err error - - lock, err = c.TryAcquireLock(ctx, id) - if err != nil { - return nil, err - } - if lock == nil { - // TTL values are in whole seconds, - // so wait one second before retrying. - err = Delay(ctx, time.Second) - if err != nil { - return nil, err - } - } - } - - return lock, nil -} - -// TryAcquireLock tries once to acquire a lock for the given ID. If the lock -// is already taken, it returns a nil azcosmos.ItemResponse and no error. -func (c *LockClient) TryAcquireLock(ctx context.Context, id string) (*azcosmos.ItemResponse, error) { - doc := &lockDocument{ - BaseDocument: BaseDocument{ID: id}, - Owner: c.name, - TTL: c.defaultTimeToLive, - } - - data, err := json.Marshal(doc) - if err != nil { - return nil, err - } - - pk := azcosmos.NewPartitionKeyString(doc.ID) - options := &azcosmos.ItemOptions{ - EnableContentResponseOnWrite: true, - } - response, err := c.containerClient.CreateItem(ctx, pk, data, options) - if IsConflictError(err) { - return nil, nil // lock already acquired by someone else - } else if err != nil { - return nil, err - } - - return &response, nil -} - -type StopHoldLock func() *azcosmos.ItemResponse - -// HoldLock tries to hold an acquired lock by renewing it periodically from a -// goroutine until the returned stop function is called. The function also returns -// a new context which is cancelled if the lock is lost or some other error occurs. -// The stop function terminates the goroutine and returns the current lock, or nil -// if the lock was lost. -func (c *LockClient) HoldLock(ctx context.Context, item *azcosmos.ItemResponse) (cancelCtx context.Context, stop StopHoldLock) { - cancelCtx, cancelCause := context.WithCancelCause(ctx) - done := make(chan struct{}) - - stop = func() *azcosmos.ItemResponse { - cancelCause(nil) - <-done // wait for goroutine to finish - return item - } - - go func() { - defer utilruntime.HandleCrash() - defer close(done) - for { - var doc *lockDocument - - err := json.Unmarshal(item.Value, &doc) - if err != nil { - cancelCause(fmt.Errorf("failed to unmarshal lock: %w", err)) - return - } - - // Aim to renew one second before TTL expires. - timeToRenew := time.Unix(int64(doc.CosmosTimestamp), 0) - if doc.TTL > 0 { - timeToRenew = timeToRenew.Add(time.Duration(doc.TTL-1) * time.Second) - } - - select { - case <-time.After(time.Until(timeToRenew)): - item, err = c.RenewLock(cancelCtx, item) - if err != nil { - cancelCause(fmt.Errorf("failed to renew lock: %w", err)) - return - } - if item == nil { - // We lost the lock, cancel the context. - cancelCause(nil) - return - } - case <-cancelCtx.Done(): - return - } - } - }() - - return -} - -// RenewLock attempts to renew an acquired lock. If successful it returns a new lock. -// If the lock was somehow lost, it returns a nil azcosmos.ItemResponse and no error. -func (c *LockClient) RenewLock(ctx context.Context, item *azcosmos.ItemResponse) (*azcosmos.ItemResponse, error) { - var doc *lockDocument - - err := json.Unmarshal(item.Value, &doc) - if err != nil { - return nil, err - } - - pk := azcosmos.NewPartitionKeyString(doc.ID) - options := &azcosmos.ItemOptions{ - EnableContentResponseOnWrite: true, - IfMatchEtag: &item.ETag, - } - response, err := c.containerClient.UpsertItem(ctx, pk, item.Value, options) - if IsPreconditionFailedError(err) { - return nil, nil // lock already acquired by someone else - } else if err != nil { - return nil, err - } - - return &response, nil -} - -// ReleaseLock attempts to release an acquired lock. Errors should be logged but not -// treated as fatal, since the container item's TTL value guarantees that it will be -// released eventually. -func (c *LockClient) ReleaseLock(ctx context.Context, item *azcosmos.ItemResponse) error { - var doc *lockDocument - - err := json.Unmarshal(item.Value, &doc) - if err != nil { - return err - } - - pk := azcosmos.NewPartitionKeyString(doc.ID) - options := &azcosmos.ItemOptions{ - IfMatchEtag: &item.ETag, - } - _, err = c.containerClient.DeleteItem(ctx, pk, doc.ID, options) - if IsPreconditionFailedError(err) { - return nil // lock already acquired by someone else - } - - return err -} diff --git a/internal/database/locks_db_client.go b/internal/database/locks_db_client.go deleted file mode 100644 index 3feb8652f1e..00000000000 --- a/internal/database/locks_db_client.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2026 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package database - -import ( - "context" - - "github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos" - - "github.com/Azure/ARO-HCP/internal/utils" -) - -// LocksDBClient provides access to the Cosmos DB Locks container used for subscription-scoped concurrency. -type LocksDBClient interface { - LockClient() LockClientInterface -} - -type locksCosmosDBClient struct { - lockClient *LockClient -} - -var _ LocksDBClient = &locksCosmosDBClient{} - -// NewLocksDBClient opens the Locks container on the given async database client and builds the lock client. -func NewLocksDBClient(ctx context.Context, database *azcosmos.DatabaseClient) (LocksDBClient, error) { - locks, err := database.NewContainer(locksContainer) - if err != nil { - return nil, utils.TrackError(err) - } - - lockClient, err := NewLockClient(ctx, locks) - if err != nil { - return nil, utils.TrackError(err) - } - - return &locksCosmosDBClient{lockClient: lockClient}, nil -} - -func (d *locksCosmosDBClient) LockClient() LockClientInterface { - return d.lockClient -} diff --git a/internal/databasetesting/mock_dbclient_test.go b/internal/databasetesting/mock_dbclient_test.go index 31e840c871d..0607cc3f6a3 100644 --- a/internal/databasetesting/mock_dbclient_test.go +++ b/internal/databasetesting/mock_dbclient_test.go @@ -751,41 +751,6 @@ func TestMockResourcesDBClient_Controller_ETagConditionalReplace(t *testing.T) { }) } -func TestMockLockClient(t *testing.T) { - ctx := context.Background() - lockClient := NewMockLockClient(30 * time.Second) - - // Test GetDefaultTimeToLive - ttl := lockClient.GetDefaultTimeToLive() - if ttl != 30*time.Second { - t.Errorf("Expected TTL 30s, got %v", ttl) - } - - // Test TryAcquireLock - lock, err := lockClient.TryAcquireLock(ctx, "test-lock") - if err != nil { - t.Fatalf("Failed to acquire lock: %v", err) - } - if lock == nil { - t.Fatal("Expected lock to be acquired") - } - - // Test that same lock can't be acquired again - lock2, err := lockClient.TryAcquireLock(ctx, "test-lock") - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - if lock2 != nil { - t.Error("Expected lock to not be acquired (already held)") - } - - // Test ReleaseLock - err = lockClient.ReleaseLock(ctx, lock) - if err != nil { - t.Fatalf("Failed to release lock: %v", err) - } -} - func TestMockResourcesDBClient_addResource(t *testing.T) { ctx := context.Background() mock := NewMockResourcesDBClient() diff --git a/internal/databasetesting/mock_locks_db_client.go b/internal/databasetesting/mock_locks_db_client.go deleted file mode 100644 index 9105dce1226..00000000000 --- a/internal/databasetesting/mock_locks_db_client.go +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright 2026 Microsoft Corporation -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package databasetesting - -import ( - "context" - "fmt" - "net/http" - "sync" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/data/azcosmos" - - "github.com/Azure/ARO-HCP/internal/database" -) - -// MockLockClient implements database.LockClientInterface for testing. -type MockLockClient struct { - defaultTTL time.Duration - locks map[string]bool - mu sync.Mutex -} - -// NewMockLockClient creates a new mock lock client. -func NewMockLockClient(defaultTTL time.Duration) *MockLockClient { - return &MockLockClient{ - defaultTTL: defaultTTL, - locks: make(map[string]bool), - } -} - -func (c *MockLockClient) GetDefaultTimeToLive() time.Duration { - return c.defaultTTL -} - -func (c *MockLockClient) SetRetryAfterHeader(header http.Header) { - header.Set("Retry-After", fmt.Sprintf("%d", int(c.defaultTTL.Seconds()))) -} - -func (c *MockLockClient) AcquireLock(ctx context.Context, id string, timeout *time.Duration) (*azcosmos.ItemResponse, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if c.locks[id] { - return nil, nil - } - c.locks[id] = true - return &azcosmos.ItemResponse{}, nil -} - -func (c *MockLockClient) TryAcquireLock(ctx context.Context, id string) (*azcosmos.ItemResponse, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if c.locks[id] { - return nil, nil - } - c.locks[id] = true - return &azcosmos.ItemResponse{}, nil -} - -func (c *MockLockClient) HoldLock(ctx context.Context, item *azcosmos.ItemResponse) (context.Context, database.StopHoldLock) { - cancelCtx, cancel := context.WithCancel(ctx) - return cancelCtx, func() *azcosmos.ItemResponse { - cancel() - return item - } -} - -func (c *MockLockClient) RenewLock(ctx context.Context, item *azcosmos.ItemResponse) (*azcosmos.ItemResponse, error) { - return item, nil -} - -func (c *MockLockClient) ReleaseLock(ctx context.Context, item *azcosmos.ItemResponse) error { - return nil -} - -var _ database.LockClientInterface = &MockLockClient{} - -// MockLocksDBClient implements database.LocksDBClient for unit testing. -type MockLocksDBClient struct { - mu sync.RWMutex - lock database.LockClientInterface -} - -// NewMockLocksDBClient returns a LocksDBClient backed by an in-memory lock implementation. -func NewMockLocksDBClient() *MockLocksDBClient { - return &MockLocksDBClient{ - lock: NewMockLockClient(10), - } -} - -// SetLockClient replaces the lock implementation (e.g. for middleware tests). -func (m *MockLocksDBClient) SetLockClient(lock database.LockClientInterface) { - m.mu.Lock() - defer m.mu.Unlock() - m.lock = lock -} - -// LockClient returns the configured lock client, or nil if unset. -func (m *MockLocksDBClient) LockClient() database.LockClientInterface { - m.mu.RLock() - defer m.mu.RUnlock() - return m.lock -} - -var _ database.LocksDBClient = &MockLocksDBClient{} diff --git a/test-integration/utils/integrationutils/cosmos_testinfo.go b/test-integration/utils/integrationutils/cosmos_testinfo.go index 11a243a1a8b..51417ce3516 100644 --- a/test-integration/utils/integrationutils/cosmos_testinfo.go +++ b/test-integration/utils/integrationutils/cosmos_testinfo.go @@ -47,7 +47,6 @@ type CosmosIntegrationTestInfo struct { CosmosDatabaseClient *azcosmos.DatabaseClient resourcesDBClient database.ResourcesDBClient billingDBClient database.BillingDBClient - locksDBClient database.LocksDBClient fleetDBClient database.FleetDBClient cosmosClient *azcosmos.Client } @@ -69,10 +68,6 @@ func NewCosmosFromTestingEnv(ctx context.Context, t *testing.T) (StorageIntegrat if err != nil { return nil, fmt.Errorf("failed to create the billing database client: %w", err) } - locksDBClient, err := database.NewLocksDBClient(ctx, cosmosDatabaseClient) - if err != nil { - return nil, fmt.Errorf("failed to create the locks database client: %w", err) - } fleetDBClient, err := database.NewFleetDBClient(cosmosDatabaseClient) if err != nil { return nil, fmt.Errorf("failed to create the fleet database client: %w", err) @@ -83,7 +78,6 @@ func NewCosmosFromTestingEnv(ctx context.Context, t *testing.T) (StorageIntegrat CosmosDatabaseClient: cosmosDatabaseClient, resourcesDBClient: resourcesDBClient, billingDBClient: billingDBClient, - locksDBClient: locksDBClient, fleetDBClient: fleetDBClient, cosmosClient: cosmosClient, } @@ -128,10 +122,6 @@ func (s *CosmosIntegrationTestInfo) BillingDBClient() database.BillingDBClient { return s.billingDBClient } -func (s *CosmosIntegrationTestInfo) LocksDBClient() database.LocksDBClient { - return s.locksDBClient -} - func (s *CosmosIntegrationTestInfo) FleetDBClient() database.FleetDBClient { return s.fleetDBClient } diff --git a/test-integration/utils/integrationutils/frontend_testinfo.go b/test-integration/utils/integrationutils/frontend_testinfo.go index 6f243ff3e74..628dc8edc9e 100644 --- a/test-integration/utils/integrationutils/frontend_testinfo.go +++ b/test-integration/utils/integrationutils/frontend_testinfo.go @@ -43,7 +43,6 @@ type StorageIntegrationTestInfo interface { GetArtifactDir() string ResourcesDBClient() database.ResourcesDBClient BillingDBClient() database.BillingDBClient - LocksDBClient() database.LocksDBClient FleetDBClient() database.FleetDBClient Cleanup(ctx context.Context) diff --git a/test-integration/utils/integrationutils/mock_cosmos_testinfo.go b/test-integration/utils/integrationutils/mock_cosmos_testinfo.go index f3250b8206d..b4e1b7085cc 100644 --- a/test-integration/utils/integrationutils/mock_cosmos_testinfo.go +++ b/test-integration/utils/integrationutils/mock_cosmos_testinfo.go @@ -29,21 +29,18 @@ type MockCosmosIntegrationTestInfo struct { mockResourcesDBClient *databasetesting.MockResourcesDBClient mockBillingDBClient *databasetesting.MockBillingDBClient - mockLocksDBClient *databasetesting.MockLocksDBClient mockFleetDBClient *databasetesting.MockFleetDBClient } func NewMockCosmosFromTestingEnv(ctx context.Context, t *testing.T) (StorageIntegrationTestInfo, error) { mockResourcesDBClient := databasetesting.NewMockResourcesDBClient() mockBillingDBClient := databasetesting.NewMockBillingDBClient() - mockLocksDBClient := databasetesting.NewMockLocksDBClient() mockFleetDBClient := databasetesting.NewMockFleetDBClient() testInfo := &MockCosmosIntegrationTestInfo{ ArtifactsDir: path.Join(getArtifactDir(), t.Name()), mockResourcesDBClient: mockResourcesDBClient, mockBillingDBClient: mockBillingDBClient, - mockLocksDBClient: mockLocksDBClient, mockFleetDBClient: mockFleetDBClient, } return testInfo, nil @@ -57,10 +54,6 @@ func (m *MockCosmosIntegrationTestInfo) BillingDBClient() database.BillingDBClie return m.mockBillingDBClient } -func (m *MockCosmosIntegrationTestInfo) LocksDBClient() database.LocksDBClient { - return m.mockLocksDBClient -} - func (m *MockCosmosIntegrationTestInfo) FleetDBClient() database.FleetDBClient { return m.mockFleetDBClient } diff --git a/test-integration/utils/integrationutils/utils.go b/test-integration/utils/integrationutils/utils.go index 77deacf630e..b61e59b83b8 100644 --- a/test-integration/utils/integrationutils/utils.go +++ b/test-integration/utils/integrationutils/utils.go @@ -132,7 +132,7 @@ func NewIntegrationTestInfoFromEnv(ctx context.Context, t *testing.T, withMock b } fakeAuditClient := &FakeOTELClient{} metricsRegistry := prometheus.NewRegistry() - aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, metricsRegistry, storageIntegrationTestInfo.ResourcesDBClient(), storageIntegrationTestInfo.LocksDBClient(), clusterServiceMockInfo.MockClusterServiceClient, fakeAuditClient, "fake-location", "", false, false, true) + aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, metricsRegistry, storageIntegrationTestInfo.ResourcesDBClient(), clusterServiceMockInfo.MockClusterServiceClient, fakeAuditClient, "fake-location", "", false, false, true) // admin api setup adminListener, err := net.Listen("tcp4", "127.0.0.1:0")