From fefef1f3ed394cb846fd6579425e9d7e0e83a760 Mon Sep 17 00:00:00 2001 From: Mariem Baccari Date: Mon, 19 Oct 2026 14:02:02 +0200 Subject: [PATCH] [scd] Create attached subscription after checking for conflicts --- pkg/scd/operations/operational_intents.go | 65 +++++++++++------------ 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/pkg/scd/operations/operational_intents.go b/pkg/scd/operations/operational_intents.go index d93d1a2ab..bfa12e679 100644 --- a/pkg/scd/operations/operational_intents.go +++ b/pkg/scd/operations/operational_intents.go @@ -326,6 +326,19 @@ func (vp *validOIRParams) toOIR(manager dssmodels.Manager, attachedSub *scdmodel } } +// newImplicitSubscription builds a brand new implicit subscription based on the provided parameters. +func (v *validOIRParams) newImplicitSubscription(manager dssmodels.Manager) *scdmodels.Subscription { + return &scdmodels.Subscription{ + ID: v.ImplicitSubscription.ID, + Manager: manager, + CellsVolume4D: v.Volume, + USSBaseURL: v.ImplicitSubscription.BaseURL, + NotifyForOperationalIntents: true, + NotifyForConstraints: v.ImplicitSubscription.ForConstraints, + ImplicitSubscription: true, + } +} + // validateAndReturnOIRUpsertParams checks that the parameters for an Operational Intent Reference upsert are valid. // Note that this does NOT check for anything related to access controls: any error returned should be labeled // as a dsserr.BadRequest. @@ -501,22 +514,6 @@ func newOIRUpsert(entityid restapi.EntityID, ovn restapi.EntityOVN, params *rest return manager, validParams, nil } -// createAndStoreNewImplicitSubscription will create a brand new implicit subscription based on the provided parameters, -// store it and return it. -func createAndStoreNewImplicitSubscription(ctx context.Context, r repos.Repository, manager dssmodels.Manager, validParams *validOIRParams) (*scdmodels.Subscription, error) { - subToUpsert := scdmodels.Subscription{ - ID: validParams.ImplicitSubscription.ID, - Manager: manager, - CellsVolume4D: validParams.Volume, - USSBaseURL: validParams.ImplicitSubscription.BaseURL, - NotifyForOperationalIntents: true, - NotifyForConstraints: validParams.ImplicitSubscription.ForConstraints, - ImplicitSubscription: true, - } - - return r.UpsertSubscription(ctx, &subToUpsert) -} - // validateKeyAndProvideConflictResponse ensures that the provided key contains all the necessary OVNs relevant for the area covered by the OperationalIntent. // - If all required keys are provided, (nil, nil) will be returned. // - If keys are missing, the conflict response will be returned (conflict, nil). @@ -600,15 +597,16 @@ func validateKeyAndProvideConflictResponse( // ensureSubscriptionCoversOIR ensures that the subscription covers the requested geo-temporal extent, extending it if both possible and required, // or failing otherwise. // After this method returns successfully, the subscription will cover the requested geo-temporal extent. -func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *scdmodels.Subscription, params *validOIRParams) (*scdmodels.Subscription, error) { - +// If the subscription was modified, true will be returned, otherwise false will be returned. +// A call to UpdateSubscription is required to persist the changes to the subscription. +func ensureSubscriptionCoversOIR(sub *scdmodels.Subscription, params *validOIRParams) (bool, error) { updateSub := false if sub.StartTime != nil && sub.StartTime.After(*params.Volume.StartTime) { if sub.ImplicitSubscription { sub.StartTime = params.Volume.StartTime updateSub = true } else { - return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not begin until after the OperationalIntent starts") + return false, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not begin until after the OperationalIntent starts") } } if sub.EndTime != nil && sub.EndTime.Before(*params.Volume.EndTime) { @@ -616,7 +614,7 @@ func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *s sub.EndTime = params.Volume.EndTime updateSub = true } else { - return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription ends before the OperationalIntent ends") + return false, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription ends before the OperationalIntent ends") } } if !sub.Cells.Contains(params.Volume.Cells) { @@ -624,18 +622,11 @@ func ensureSubscriptionCoversOIR(ctx context.Context, r repos.Repository, sub *s sub.Cells = s2.CellUnionFromUnion(sub.Cells, params.Volume.Cells) updateSub = true } else { - return nil, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not cover entire spatial area of the OperationalIntent") - } - } - if updateSub { - upsertedSub, err := r.UpsertSubscription(ctx, sub) - if err != nil { - return nil, stacktrace.Propagate(err, "Failed to update existing Subscription") + return false, stacktrace.NewErrorWithCode(dsserr.BadRequest, "Subscription does not cover entire spatial area of the OperationalIntent") } - return upsertedSub, nil } - return sub, nil + return updateSub, nil } // PutOperationalIntentReferenceResult is the result of an Operational Intent Reference put operation. @@ -733,6 +724,7 @@ func executePutOperationalIntentReference(ctx context.Context, repo repos.Reposi // attachedSub is the subscription that will end up being attached to the OIR // it defaults to the previous subscription (which may be nil), and may be updated if required by the parameters attachedSub := previousSub + upsertAttachedSub := false if validParams.SubscriptionID.Empty() { // No subscription ID was provided: // check if an implicit subscription should be created, otherwise do nothing @@ -740,9 +732,8 @@ func executePutOperationalIntentReference(ctx context.Context, repo repos.Reposi // Parameters for a new implicit subscription have been passed: we will create // a new implicit subscription even if another subscription was attached to this OIR before, // regardless of whether it was an implicit subscription or not. - if attachedSub, err = createAndStoreNewImplicitSubscription(ctx, repo, manager, validParams); err != nil { - return nil, stacktrace.Propagate(err, "Failed to create implicit subscription") - } + attachedSub = validParams.newImplicitSubscription(manager) + upsertAttachedSub = true } else { // If no subscription ID is provided and no implicit subscription is requested, // the OIR should have no attached subscription @@ -779,7 +770,7 @@ func executePutOperationalIntentReference(ctx context.Context, repo repos.Reposi } // We need to ensure the subscription covers the OIR's geo-temporal extent - attachedSub, err = ensureSubscriptionCoversOIR(ctx, repo, attachedSub, validParams) + upsertAttachedSub, err = ensureSubscriptionCoversOIR(attachedSub, validParams) if err != nil { return nil, stacktrace.Propagate(err, "Failed to ensure subscription covers OIR") } @@ -795,6 +786,14 @@ func executePutOperationalIntentReference(ctx context.Context, repo repos.Reposi } } + // Upsert the attached subscription after making sure that no conflicts exist + if upsertAttachedSub { + attachedSub, err = repo.UpsertSubscription(ctx, attachedSub) + if err != nil { + return nil, stacktrace.Propagate(err, "Failed to upsert Subscription attached to OperationalIntent") + } + } + // Construct the new OperationalIntent op := validParams.toOIR(manager, attachedSub, version, pastOVNs)