feat: [CDE-753]: Fix backend handling of gitspace deletion requests (#3745)

* feat: [CDE-753]: Fix lint
* feat: [CDE-753]: Fix backend handling of gitspace deletion requests

app/gitspace/infrastructure/trigger_infra_event.go

@@ -100,7 +100,7 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 		return err
 	}
 
-	_, configMetadata, err := i.getAllParamsFromDB(ctx, gitspaceConfig.InfraProviderResource, infraProvider)
+	allParams, configMetadata, err := i.getAllParamsFromDB(ctx, gitspaceConfig.InfraProviderResource, infraProvider)
 	if err != nil {
 		return fmt.Errorf("could not get all params from DB while provisioning: %w", err)
 	}
@@ -126,6 +126,8 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 				gitspaceConfig,
 				opts.RequiredGitspacePorts,
 				stoppedInfra,
+				configMetadata,
+				allParams,
 			)
 		}
 		return i.provisionExistingInfrastructure(
@@ -145,9 +147,10 @@ func (i InfraProvisioner) TriggerInfraEventWithOpts(
 				*infra,
 				opts.CanDeleteUserData,
 				configMetadata,
+				allParams,
 			)
 		}
-		return infraProvider.Deprovision(ctx, *infra, opts.CanDeleteUserData, configMetadata)
+		return infraProvider.Deprovision(ctx, *infra, opts.CanDeleteUserData, configMetadata, allParams)
 
 	case enum.InfraEventCleanup:
 		return infraProvider.CleanupInstanceResources(ctx, *infra)
@@ -167,6 +170,8 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 	gitspaceConfig types.GitspaceConfig,
 	requiredGitspacePorts []types.GitspacePort,
 	stoppedInfra types.Infrastructure,
+	configMetadata map[string]any,
+	allParams []types.InfraProviderParameter,
 ) error {
 	// Logic for new provisioning...
 	infraProvisionedLatest, _ := i.infraProvisionedStore.FindLatestByGitspaceInstanceID(
@@ -185,12 +190,8 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 	}
 
 	infraProviderResource := gitspaceConfig.InfraProviderResource
-	allParams, configMetadata, err := i.getAllParamsFromDB(ctx, infraProviderResource, infraProvider)
-	if err != nil {
-		return fmt.Errorf("could not get all params from DB while provisioning: %w", err)
-	}
 
-	err = infraProvider.ValidateParams(allParams)
+	err := infraProvider.ValidateParams(allParams)
 	if err != nil {
 		return fmt.Errorf("invalid provisioning params %v: %w", infraProviderResource.Metadata, err)
 	}
@@ -210,6 +211,7 @@ func (i InfraProvisioner) provisionNewInfrastructure(
 		InputParameters:            allParams,
 		ConfigMetadata:             configMetadata,
 		InstanceInfo:               stoppedInfra.InstanceInfo,
+		Status:                     enum.InfraStatusPending,
 	}
 	responseMetadata, err := json.Marshal(infrastructure)
 	if err != nil {
@@ -315,6 +317,7 @@ func (i InfraProvisioner) deprovisionNewInfrastructure(
 	infra types.Infrastructure,
 	canDeleteUserData bool,
 	configMetadata map[string]any,
+	params []types.InfraProviderParameter,
 ) error {
 	infraProvisionedLatest, err := i.infraProvisionedStore.FindLatestByGitspaceInstanceID(
 		ctx, gitspaceConfig.GitspaceInstance.ID)
@@ -328,7 +331,7 @@ func (i InfraProvisioner) deprovisionNewInfrastructure(
 		return nil
 	}
 
-	err = infraProvider.Deprovision(ctx, infra, canDeleteUserData, configMetadata)
+	err = infraProvider.Deprovision(ctx, infra, canDeleteUserData, configMetadata, params)
 	if err != nil {
 		return fmt.Errorf("unable to trigger deprovision infra %+v: %w", infra, err)
 	}

app/gitspace/orchestrator/orchestrator_trigger.go

@@ -344,17 +344,18 @@ func (o Orchestrator) TriggerCleanupInstanceResources(ctx context.Context, gitsp
 	return nil
 }
 
-// TriggerDeleteGitspace removes the Gitspace container and triggers infra deprovisioning to deprovision
+// TriggerStopAndDeleteGitspace removes the Gitspace container and triggers infra deprovisioning to deprovision
 // the infra resources.
 // canDeleteUserData = false -> trigger deprovision of all resources except storage associated to user data.
 // canDeleteUserData = true -> trigger deprovision of all resources.
-func (o Orchestrator) TriggerDeleteGitspace(
+func (o Orchestrator) TriggerStopAndDeleteGitspace(
 	ctx context.Context,
 	gitspaceConfig types.GitspaceConfig,
 	canDeleteUserData bool,
 ) error {
 	infra, err := o.getProvisionedInfra(ctx, gitspaceConfig,
 		[]enum.InfraStatus{
+			enum.InfraStatusPending,
 			enum.InfraStatusProvisioned,
 			enum.InfraStatusStopped,
 			enum.InfraStatusDestroyed,
@@ -368,6 +369,8 @@ func (o Orchestrator) TriggerDeleteGitspace(
 	}
 	if err = o.stopAndRemoveGitspaceContainer(ctx, gitspaceConfig, *infra, canDeleteUserData); err != nil {
 		log.Warn().Msgf("error stopping and removing gitspace container: %v", err)
+		// If stop fails, delete the gitspace anyway
+		return o.triggerDeleteGitspace(ctx, gitspaceConfig, infra, canDeleteUserData)
 	}
 
 	// TODO: Add a job for cleanup of infra if stop fails
@@ -375,34 +378,18 @@ func (o Orchestrator) TriggerDeleteGitspace(
 		"Checking and force deleting the infra if required for gitspace instance %s",
 		gitspaceConfig.GitspaceInstance.Identifier,
 	)
-	ticker := time.NewTicker(60 * time.Second)
-	timeout := time.After(15 * time.Minute)
-	defer ticker.Stop()
-	ch := make(chan error)
-
-	for {
-		select {
-		case msg := <-ch:
-			if msg == nil {
-				return msg
+	if err = o.waitForGitspaceCleanupOrTimeout(ctx, gitspaceConfig, 15*time.Minute, 60*time.Second); err != nil {
+		return o.triggerDeleteGitspace(ctx, gitspaceConfig, infra, canDeleteUserData)
 	}
-		case <-ticker.C:
-			instance, err := o.gitspaceInstanceStore.Find(
-				ctx,
-				gitspaceConfig.GitspaceInstance.ID,
-			)
-			if err != nil {
-				return fmt.Errorf(
-					"failed to find gitspace instance %s: %w",
-					gitspaceConfig.GitspaceInstance.Identifier,
-					err,
-				)
-			}
-			if instance.State == enum.GitspaceInstanceStateDeleted ||
-				instance.State == enum.GitspaceInstanceStateCleaned {
 	return nil
 }
-		case <-timeout:
+
+func (o Orchestrator) triggerDeleteGitspace(
+	ctx context.Context,
+	gitspaceConfig types.GitspaceConfig,
+	infra *types.Infrastructure,
+	canDeleteUserData bool,
+) error {
 	opts := infrastructure.InfraEventOpts{CanDeleteUserData: true}
 	err := o.infraProvisioner.TriggerInfraEventWithOpts(
 		ctx,
@@ -425,6 +412,39 @@ func (o Orchestrator) TriggerDeleteGitspace(
 	}
 	return nil
 }
+
+func (o Orchestrator) waitForGitspaceCleanupOrTimeout(
+	ctx context.Context,
+	gitspaceConfig types.GitspaceConfig,
+	timeoutDuration time.Duration,
+	tickInterval time.Duration,
+) error {
+	ticker := time.NewTicker(tickInterval)
+	defer ticker.Stop()
+
+	timeout := time.After(timeoutDuration)
+
+	for {
+		select {
+		case <-ticker.C:
+			instance, err := o.gitspaceInstanceStore.Find(ctx, gitspaceConfig.GitspaceInstance.ID)
+			if err != nil {
+				return fmt.Errorf(
+					"failed to find gitspace instance %s: %w",
+					gitspaceConfig.GitspaceInstance.Identifier,
+					err,
+				)
+			}
+			if instance.State == enum.GitspaceInstanceStateDeleted ||
+				instance.State == enum.GitspaceInstanceStateCleaned {
+				return nil
+			}
+		case <-timeout:
+			return fmt.Errorf(
+				"timeout waiting for gitspace cleanup for instance %s",
+				gitspaceConfig.GitspaceInstance.Identifier,
+			)
+		}
 	}
 }
 

app/services/gitspace/action_delete.go

@@ -89,7 +89,7 @@ func (c *Service) RemoveGitspace(ctx context.Context, config types.GitspaceConfi
 		)
 	}
 
-	if err := c.orchestrator.TriggerDeleteGitspace(ctx, config, canDeleteUserData); err != nil {
+	if err := c.orchestrator.TriggerStopAndDeleteGitspace(ctx, config, canDeleteUserData); err != nil {
 		log.Ctx(ctx).Err(err).Msgf("error during triggering delete for gitspace instance %s",
 			config.GitspaceInstance.Identifier)
 		config.GitspaceInstance.State = enum.GitspaceInstanceStateError

infraprovider/docker_provider.go

@@ -208,6 +208,7 @@ func (d DockerProvider) Deprovision(
 	infra types.Infrastructure,
 	canDeleteUserData bool,
 	_ map[string]any,
+	_ []types.InfraProviderParameter,
 ) error {
 	if canDeleteUserData {
 		err := d.deleteVolume(ctx, infra)

infraprovider/infra_provider.go

@@ -67,6 +67,7 @@ type InfraProvider interface {
 		infra types.Infrastructure,
 		canDeleteUserData bool,
 		configMetadata map[string]any,
+		params []types.InfraProviderParameter,
 	) error
 
 	// AvailableParams provides a schema to define the infrastructure.