Skip to content

Commit 5877552

Browse files
committed
Implemented optimistic locking for Deployments connector updates/patch.
1 parent 8c8e09d commit 5877552

File tree

3 files changed

+30
-11
lines changed

3 files changed

+30
-11
lines changed

internal/connector/internal/handlers/connector_admin.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -395,12 +395,11 @@ func (h *ConnectorAdminHandler) DeleteConnector(writer http.ResponseWriter, requ
395395

396396
// check force flag to force deletion of connector and deployments
397397
if parseBoolParam(request.URL.Query().Get("force")) {
398-
serviceError = h.ConnectorsService.ForceDelete(request.Context(), connectorId)
398+
return nil, h.ConnectorsService.ForceDelete(request.Context(), connectorId)
399399
} else {
400400
ctx := request.Context()
401401
return nil, HandleConnectorDelete(ctx, h.ConnectorsService, h.NamespaceService, connectorId)
402402
}
403-
return nil, serviceError
404403
},
405404
}
406405

@@ -605,6 +604,7 @@ func (h *ConnectorAdminHandler) PatchConnectorDeployment(writer http.ResponseWri
605604
// Handle the fields that support being updated...
606605
var updatedDeployment dbapi.ConnectorDeployment
607606
updatedDeployment.ID = existingDeployment.ID
607+
updatedDeployment.Version = existingDeployment.Version
608608
if len(resource.Spec.ShardMetadata) != 0 {
609609
// channel update
610610
updateRevision, err := workers.GetShardMetadataRevision(resource.Spec.ShardMetadata)

internal/connector/internal/handlers/connector_validation.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ package handlers
22

33
import (
44
"context"
5+
"strings"
6+
57
"github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/internal/connector/internal/api/dbapi"
68
"k8s.io/apimachinery/pkg/util/validation"
7-
"strings"
89

910
"github.com/bf2fc6cc711aee1a0c2a/kas-fleet-manager/pkg/shared/utils/arrays"
1011

@@ -36,7 +37,7 @@ func connectorValidationFunction(connectorTypesService services.ConnectorTypesSe
3637
return func() *errors.ServiceError {
3738
ct, err := connectorTypesService.Get(*connectorTypeId)
3839
if err != nil {
39-
return errors.BadRequest("YYY invalid connector type id %v : %s", connectorTypeId, err)
40+
return errors.BadRequest("Invalid connector type id %v : %s", connectorTypeId, err)
4041
}
4142

4243
if !arrays.Contains(ct.ChannelNames(), string(*channel)) {

internal/connector/internal/services/connector_cluster.go

+25-7
Original file line numberDiff line numberDiff line change
@@ -396,11 +396,20 @@ func (k *connectorClusterService) GetClientID(clusterID string) (string, error)
396396
// SaveDeployment creates a connector deployment in the database
397397
func (k *connectorClusterService) SaveDeployment(ctx context.Context, resource *dbapi.ConnectorDeployment) *errors.ServiceError {
398398
dbConn := k.connectionFactory.New()
399+
if resource.Version != 0 {
400+
dbConn = dbConn.Where("version = ?", resource.Version)
401+
}
399402

400-
if err := dbConn.Save(resource).Error; err != nil {
403+
saveResult := dbConn.Save(resource)
404+
if err := saveResult.Error; err != nil {
401405
return services.HandleCreateError(`Connector deployment`, err)
402406
}
407+
if saveResult.RowsAffected == 0 {
408+
return errors.Conflict("Optimistic locking: resource version changed from %v", resource.Version)
409+
}
403410

411+
//refresh version
412+
dbConn = k.connectionFactory.New()
404413
if err := dbConn.Where("id = ?", resource.ID).Select("version").First(&resource).Error; err != nil {
405414
return services.HandleGetError(`Connector deployment`, "id", resource.ID, err)
406415
}
@@ -416,11 +425,20 @@ func (k *connectorClusterService) SaveDeployment(ctx context.Context, resource *
416425

417426
func (k *connectorClusterService) UpdateDeployment(resource *dbapi.ConnectorDeployment) *errors.ServiceError {
418427
dbConn := k.connectionFactory.New()
428+
429+
if resource.Version != 0 {
430+
dbConn = dbConn.Where("version = ?", resource.Version)
431+
}
432+
419433
updates := dbConn.Where("id = ?", resource.ID).
420434
Updates(resource)
421435
if err := updates.Error; err != nil {
422-
return services.HandleUpdateError(`Connector namespace`, err)
436+
return services.HandleUpdateError(`Connector deployment`, err)
437+
}
438+
if updates.RowsAffected == 0 {
439+
return errors.Conflict("Optimistic locking: resource version changed from %v", resource.Version)
423440
}
441+
424442
return nil
425443
}
426444

@@ -494,7 +512,7 @@ func (k *connectorClusterService) ListConnectorDeployments(ctx context.Context,
494512
func (k *connectorClusterService) UpdateConnectorDeploymentStatus(ctx context.Context, deploymentStatus dbapi.ConnectorDeploymentStatus) *errors.ServiceError {
495513
dbConn := k.connectionFactory.New()
496514

497-
// lets get the connector id of the deployment..
515+
// let's get the connector id of the deployment...
498516
deployment := dbapi.ConnectorDeployment{}
499517
if err := dbConn.Unscoped().Select("connector_id", "deleted_at").
500518
Where("id = ?", deploymentStatus.ID).
@@ -505,7 +523,7 @@ func (k *connectorClusterService) UpdateConnectorDeploymentStatus(ctx context.Co
505523
return services.HandleGoneError("Connector deployment", "id", deploymentStatus.ID)
506524
}
507525

508-
if err := dbConn.Model(&deploymentStatus).Where("id = ? and version <= ?", deploymentStatus.ID, deploymentStatus.Version).Save(&deploymentStatus).Error; err != nil {
526+
if err := dbConn.Where("id = ? and version <= ?", deploymentStatus.ID, deploymentStatus.Version).Save(&deploymentStatus).Error; err != nil {
509527
return errors.Conflict("failed to update deployment status: %s, probably a stale deployment status version was used: %d", err.Error(), deploymentStatus.Version)
510528
}
511529

@@ -531,8 +549,8 @@ func (k *connectorClusterService) UpdateConnectorDeploymentStatus(ctx context.Co
531549
}
532550
}
533551

534-
// update the connector status
535-
if err := dbConn.Where("id = ?", deployment.ConnectorID).Updates(&connectorStatus).Error; err != nil {
552+
// update the connector status, don't updated deleted statues
553+
if err := dbConn.Where("deleted_at IS NULL AND id = ?", deployment.ConnectorID).Updates(&connectorStatus).Error; err != nil {
536554
return services.HandleUpdateError("Connector status", err)
537555
}
538556

@@ -564,7 +582,7 @@ func (k *connectorClusterService) FindAvailableNamespace(owner string, orgID str
564582

565583
func (k *connectorClusterService) GetDeploymentByConnectorId(ctx context.Context, connectorID string) (resource dbapi.ConnectorDeployment, serr *errors.ServiceError) {
566584

567-
if err := k.connectionFactory.New().Preload(clause.Associations).
585+
if err := k.connectionFactory.New().
568586
Joins("Status").Joins("ConnectorShardMetadata").Joins("Connector").
569587
Where("connector_id = ?", connectorID).First(&resource).Error; err != nil {
570588
return resource, services.HandleGetError("Connector deployment", "connector_id", connectorID, err)

0 commit comments

Comments
 (0)