fix(shipyard-controller): Proceed with service deletion if the servic…

…e is not present on the configuration service anymore (#7460)

* fixed unit test

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

* trigger dco check

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

* fixed unit tests

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

shipyard-controller/common/configurationstore.go

@@ -3,6 +3,7 @@ package common
 import (
 	"errors"
 	"net/http"
+	"strings"
 
 	apimodels "github.com/keptn/go-utils/pkg/api/models"
 	keptnapi "github.com/keptn/go-utils/pkg/api/utils"
@@ -12,6 +13,10 @@ type configStoreErrType int
 
 var ErrConfigStoreInvalidToken = errors.New("invalid git token")
 var ErrConfigStoreUpstreamNotFound = errors.New("upstream repository not found")
+var ErrServiceNotFound = errors.New("service not found")
+
+const configServiceSvcDoesNotExistErrorMsg = "service does not exists" // [sic] this is what we get from the configuration service
+const resourceServiceSvcDoesNotExistErrorMsg = "service not found"
 
 //go:generate moq -pkg common_mock -out ./fake/configurationstore_mock.go . ConfigurationStore
 type ConfigurationStore interface {
@@ -112,10 +117,26 @@ func (g GitConfigurationStore) DeleteService(projectName string, stageName strin
 }
 
 func (g GitConfigurationStore) buildErrResponse(err *apimodels.Error) error {
-	if err.Code == http.StatusFailedDependency {
+	if isServiceNotFoundErr(*err) {
+		return ErrServiceNotFound
+	} else if err.Code == http.StatusFailedDependency {
 		return ErrConfigStoreInvalidToken
 	} else if err.Code == http.StatusNotFound {
 		return ErrConfigStoreUpstreamNotFound
 	}
 	return errors.New(*err.Message)
 }
+
+func isServiceNotFoundErr(err apimodels.Error) bool {
+	if err.Message == nil {
+		// if there is no message, we cannot deduct it being a service not found error
+		return false
+	}
+	if err.Code == http.StatusBadRequest || err.Code == http.StatusNotFound {
+		errMsg := strings.ToLower(*err.Message)
+		if strings.Contains(errMsg, configServiceSvcDoesNotExistErrorMsg) || strings.Contains(errMsg, resourceServiceSvcDoesNotExistErrorMsg) {
+			return true
+		}
+	}
+	return false
+}

shipyard-controller/common/configurationstore_test.go

@@ -221,3 +221,52 @@ func TestConfigurationStore(t *testing.T) {
 	})
 
 }
+
+func Test_isServiceNotFoundErr(t *testing.T) {
+	configSvcErrMsg := configServiceSvcDoesNotExistErrorMsg
+	resourceSvcErrMsg := resourceServiceSvcDoesNotExistErrorMsg
+	type args struct {
+		err apimodels.Error
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "service not found error from configuration-service",
+			args: args{
+				err: apimodels.Error{
+					Code:    http.StatusBadRequest,
+					Message: &configSvcErrMsg,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "service not found error from resource-service",
+			args: args{
+				err: apimodels.Error{
+					Code:    http.StatusNotFound,
+					Message: &resourceSvcErrMsg,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "other error",
+			args: args{
+				err: apimodels.Error{
+					Code:    http.StatusNotFound,
+					Message: nil,
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, isServiceNotFoundErr(tt.args.err), "isServiceNotFoundErr(%v)", tt.args.err)
+		})
+	}
+}

shipyard-controller/handler/servicemanager.go

@@ -136,7 +136,16 @@ func (sm *serviceManager) DeleteService(projectName, serviceName string) error {
 	for _, stage := range stages {
 		log.Infof("Deleting service %s from stage %s", serviceName, stage.StageName)
 		if err := sm.configurationStore.DeleteService(projectName, stage.StageName, serviceName); err != nil {
-			return sm.logAndReturnError(fmt.Sprintf("could not delete service %s from stage %s: %s", serviceName, stage.StageName, err.Error()))
+			// If we get a ErrServiceNotFound, we can proceed with deleting the service from the db.
+			// For other types of errors (e.g. due to a temporary upstream repo connection issue), we return without deleting it from the db.
+			// Otherwise, it could be that the service directory is still present in the configuration service, but gone from the db, which means we cannot
+			// retry the deletion via the bridge (since the service won't show up anymore), and recreating the service will fail because we'll get a 409 from
+			// the configuration service
+			if errors.Is(err, common.ErrServiceNotFound) {
+				log.Infof("Service %s has already been deleted from stage %s", serviceName, stage.StageName)
+			} else {
+				return sm.logAndReturnError(fmt.Sprintf("could not delete service %s from stage %s: %s", serviceName, stage.StageName, err.Error()))
+			}
 		}
 		if err := sm.projectMVRepo.DeleteService(projectName, stage.StageName, serviceName); err != nil {
 			return sm.logAndReturnError(fmt.Sprintf("could not delete service %s from stage %s: %s", serviceName, stage.StageName, err.Error()))

shipyard-controller/handler/servicemanager_test.go

@@ -249,6 +249,50 @@ func TestDeleteService_DeleteServiceInConfigurationServiceFails(t *testing.T) {
 	assert.Equal(t, 0, len(projectMVRepo.DeleteServiceCalls()))
 }
 
+func TestDeleteService_DeleteServiceInConfigurationServiceReturnsServiceNotFound(t *testing.T) {
+	projectMVRepo := &db_mock.ProjectMVRepoMock{}
+	configurationStore := &common_mock.ConfigurationStoreMock{}
+	uniformRepo := &db_mock.UniformRepoMock{}
+	instance := NewServiceManager(projectMVRepo, configurationStore, uniformRepo)
+	projectMVRepo.GetProjectFunc = func(projectName string) (*apimodels.ExpandedProject, error) {
+		service := &apimodels.ExpandedService{
+			ServiceName: "service-name",
+		}
+		stage1 := &apimodels.ExpandedStage{
+			Services:  []*apimodels.ExpandedService{service},
+			StageName: "dev",
+		}
+		stage2 := &apimodels.ExpandedStage{
+			Services:  []*apimodels.ExpandedService{service},
+			StageName: "prod",
+		}
+
+		project := &apimodels.ExpandedProject{
+			ProjectName: "my-project",
+			Stages:      []*apimodels.ExpandedStage{stage1, stage2},
+		}
+		return project, nil
+	}
+	projectMVRepo.DeleteServiceFunc = func(project string, stage string, service string) error {
+		return nil
+	}
+
+	uniformRepo.DeleteServiceFromSubscriptionsFunc = func(subscriptionName string) error {
+		return nil
+	}
+
+	configurationStore.DeleteServiceFunc = func(projectName string, stageName string, serviceName string) error {
+		return common.ErrServiceNotFound
+	}
+
+	err := instance.DeleteService("my-project", "my-service")
+	assert.Nil(t, err)
+	assert.Equal(t, 2, len(configurationStore.DeleteServiceCalls()))
+	// in this case we expect the service to be deleted from the database, because it is already gone from the upstream
+	assert.Equal(t, 2, len(projectMVRepo.DeleteServiceCalls()))
+	assert.Equal(t, 2, len(uniformRepo.DeleteServiceFromSubscriptionsCalls()))
+}
+
 func TestDeleteService_DeleteServiceInDBFails(t *testing.T) {
 	projectMVRepo := &db_mock.ProjectMVRepoMock{}
 	configurationStore := &common_mock.ConfigurationStoreMock{}