fix(shipyard-controller): Do not reset subscriptions when updating di…

…stributor/integration version (#7046) * fix(shipyard-controller): Do not overwrite existing subscriptions in case of version update Signed-off-by: Florian Bacher <[email protected]> * added assertion to check if subscriptions are still there after version upgrade Signed-off-by: Florian Bacher <[email protected]> * added assertion to check if subscriptions are still there after version upgrade Signed-off-by: Florian Bacher <[email protected]> * fixed typo Signed-off-by: Florian Bacher <[email protected]> * fixed sonar warnings Signed-off-by: Florian Bacher <[email protected]> * added comment Signed-off-by: Florian Bacher <[email protected]> * pr review Signed-off-by: Florian Bacher <[email protected]> * added unit test for repo implementation Signed-off-by: Florian Bacher <[email protected]> * ci: Add pipeline to nightly check docker image digests (#6598) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Minor pipeline improvements (#6613) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Only push docker images if not from a forked repo (#6618) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix digest checker pipeline (#6619) Co-authored-by: Giovanni Liva <[email protected]> Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix CI pipeline not pushing master images to registry (#6634) Signed-off-by: Moritz Wiesinger <[email protected]> * feat: Release helm charts on GitHub pages (#6559) BREAKING CHANGES: The Keptn Helm charts are now served from the [keptn/helm-charts](https://github.com/keptn/helm-charts) repository through GitHub pages. They are accessible by using https://charts.keptn.sh as a helm chart repository. Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Remove unneeded pipelines and config files (#6642) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix master builds not pushing docker images (#6643) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix master builds not pushing docker images (#6659) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix intergation tests taking master artifacts from bot commits (#6670) Signed-off-by: Moritz Wiesinger <[email protected]> * test: Integration Tests on static GKE clusters (#6632) Co-authored-by: Giovanni Liva <[email protected]> Signed-off-by: Moritz Wiesinger <[email protected]> * chore: Add k8s resource stats to release notes (#6718) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix dev helm charts not having a unique image name (#6816) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Disable manual triggers of the CI pipeline (#6819) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Always use datetimed version for integration tests (#6820) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Ensure that build matrix runs as far as possible (#6875) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Only upload dev helm chart for non-fork PRs (#6763) (cherry picked from commit 134d487) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Push dev images to dev dockerhub registry, adjust helm charts (#6857) (cherry picked from commit 59a7762) Signed-off-by: Moritz Wiesinger <[email protected]> * chore: Removed makefile and all usages of it (#6804) (cherry picked from commit e55355f) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix cleanup in integration tests to also include clusterroles and clusterrolebindings (#6931) (cherry picked from commit 3dd71e5) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix release pipeline not uploading helm charts correctly (#6944) (cherry picked from commit 0609c7d) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix integration test pipeline not starting if there were 404ing CI pipeline runs in the source branch (#6966) (cherry picked from commit 23c4c37) Signed-off-by: Moritz Wiesinger <[email protected]> * ci: Fix resource cleanup in integration tests when resource arrays are empty (#6971) (cherry picked from commit 2abb21c) Signed-off-by: Moritz Wiesinger <[email protected]> * added comment Signed-off-by: Florian Bacher <[email protected]> * fix(shipyard-controller): Do not overwrite existing subscriptions in case of version update Signed-off-by: Florian Bacher <[email protected]> * added assertion to check if subscriptions are still there after version upgrade Signed-off-by: Florian Bacher <[email protected]> * added assertion to check if subscriptions are still there after version upgrade Signed-off-by: Florian Bacher <[email protected]> * fixed typo Signed-off-by: Florian Bacher <[email protected]> * fixed sonar warnings Signed-off-by: Florian Bacher <[email protected]> * added comment Signed-off-by: Florian Bacher <[email protected]> * pr review Signed-off-by: Florian Bacher <[email protected]> * added unit test for repo implementation Signed-off-by: Florian Bacher <[email protected]> * added comment Signed-off-by: Florian Bacher <[email protected]> Co-authored-by: Moritz Wiesinger <[email protected]> Co-authored-by: Giovanni Liva <[email protected]>
keptn · Mar 3, 2022 · a5a62ee · a5a62ee
1 parent b991724
commit a5a62ee
Show file tree

Hide file tree

Showing 7 changed files with 193 additions and 13 deletions.
diff --git a/shipyard-controller/db/mock/uniformrepo_mock.go b/shipyard-controller/db/mock/uniformrepo_mock.go
diff --git a/shipyard-controller/db/mongodb_uniform_repo.go b/shipyard-controller/db/mongodb_uniform_repo.go
@@ -16,6 +16,12 @@ import (
 
 const uniformCollectionName = "keptnUniform"
 
+const lastSeenProperty = "metadata.lastseen"
+const integrationVersionProperty = "metadata.integrationversion"
+const distributorVersionProperty = "metadata.distributorversion"
+
+const couldNotGetCollectionErrMsg = "could not get collection: %s"
+
 var ErrUniformRegistrationAlreadyExists = errors.New("uniform integration already exists")
 var ErrUniformRegistrationNotFound = errors.New("uniform integration not found")
 
@@ -217,12 +223,45 @@ func (mdbrepo *MongoDBUniformRepo) UpdateLastSeen(integrationID string) (*models
 	now := time.Now().UTC()
 	collection, ctx, cancel, err := mdbrepo.getCollectionAndContext()
 	if err != nil {
-		return nil, fmt.Errorf("could not get collection: %s", err.Error())
+		return nil, fmt.Errorf(couldNotGetCollectionErrMsg, err.Error())
+	}
+	defer cancel()
+
+	filter := bson.D{{"_id", integrationID}}
+	update := bson.D{{"$set", bson.D{{lastSeenProperty, now}}}}
+
+	opts := options.FindOneAndUpdate().SetReturnDocument(options.After)
+	result := collection.FindOneAndUpdate(ctx, filter, update, opts)
+	if result.Err() != nil {
+		if errors.Is(result.Err(), mongo.ErrNoDocuments) {
+			return nil, ErrUniformRegistrationNotFound
+		}
+		return nil, result.Err()
+	}
+
+	updatedIntegration := &models.Integration{}
+	err = result.Decode(updatedIntegration)
+	if err != nil {
+		return nil, err
+	}
+	return updatedIntegration, nil
+
+}
+
+func (mdbrepo *MongoDBUniformRepo) UpdateVersionInfo(integrationID, integrationVersion, distributorVersion string) (*models.Integration, error) {
+	now := time.Now().UTC()
+	collection, ctx, cancel, err := mdbrepo.getCollectionAndContext()
+	if err != nil {
+		return nil, fmt.Errorf(couldNotGetCollectionErrMsg, err.Error())
 	}
 	defer cancel()
 
 	filter := bson.D{{"_id", integrationID}}
-	update := bson.D{{"$set", bson.D{{"metadata.lastseen", now}}}}
+	update := bson.D{{"$set", bson.D{
+		{integrationVersionProperty, integrationVersion},
+		{distributorVersionProperty, distributorVersion},
+		{lastSeenProperty, now},
+	}}}
 
 	opts := options.FindOneAndUpdate().SetReturnDocument(options.After)
 	result := collection.FindOneAndUpdate(ctx, filter, update, opts)
@@ -245,11 +284,11 @@ func (mdbrepo *MongoDBUniformRepo) UpdateLastSeen(integrationID string) (*models
 func (mdbrepo *MongoDBUniformRepo) SetupTTLIndex(duration time.Duration) error {
 	collection, ctx, cancel, err := mdbrepo.getCollectionAndContext()
 	if err != nil {
-		return fmt.Errorf("could not get collection: %s", err.Error())
+		return fmt.Errorf(couldNotGetCollectionErrMsg, err.Error())
 	}
 	defer cancel()
 
-	return SetupTTLIndex(ctx, "metadata.lastseen", duration, collection)
+	return SetupTTLIndex(ctx, lastSeenProperty, duration, collection)
 }
 
 func (mdbrepo *MongoDBUniformRepo) getSearchOptions(params models.GetUniformIntegrationsParams) bson.M {

diff --git a/shipyard-controller/db/mongodb_uniform_repo_test.go b/shipyard-controller/db/mongodb_uniform_repo_test.go
@@ -394,3 +394,56 @@ func TestMongoDBUniformRepo_RemoveByServiceName(t *testing.T) {
 	}
 
 }
+
+func TestMongoDBUniformRepo_UpdateVersionInfo(t *testing.T) {
+	testIntegration := models.Integration{
+		ID:   "i1",
+		Name: "integration1",
+		MetaData: keptnmodels.MetaData{
+			IntegrationVersion: "1",
+			DistributorVersion: "1",
+		},
+		Subscriptions: []keptnmodels.EventSubscription{
+			{
+				Event: "sh.keptn.event.test.triggered",
+				Filter: keptnmodels.EventSubscriptionFilter{
+					Projects: []string{"pr1"},
+					Services: []string{"sv1", "sv2"},
+					Stages:   []string{"st1", "st2"},
+				},
+			},
+			{
+				Event: "sh.keptn.event.test",
+				Filter: keptnmodels.EventSubscriptionFilter{
+					Projects: []string{"pr2"},
+					Services: []string{"sv1"},
+					Stages:   []string{"st1", "st2"},
+				},
+			},
+			{
+				Event: "sh.keptn.event",
+				Filter: keptnmodels.EventSubscriptionFilter{
+					Projects: []string{"pr4"},
+					Services: []string{"sv1", "sv2"},
+					Stages:   []string{"st1", "st2"},
+				},
+			},
+		},
+	}
+
+	mdbrepo := NewMongoDBUniformRepo(GetMongoDBConnectionInstance())
+
+	// insert our integration entities
+	err := mdbrepo.CreateOrUpdateUniformIntegration(testIntegration)
+
+	require.Nil(t, err)
+
+	updated, err := mdbrepo.UpdateVersionInfo(testIntegration.ID, "int-2", "distr-2")
+
+	require.Nil(t, err)
+	require.Equal(t, "distr-2", updated.MetaData.DistributorVersion)
+	require.Equal(t, "int-2", updated.MetaData.IntegrationVersion)
+
+	// make sure the subscriptions are not touched by the version update
+	require.Len(t, updated.Subscriptions, 3)
+}
diff --git a/shipyard-controller/db/repos.go b/shipyard-controller/db/repos.go
@@ -26,6 +26,7 @@ type UniformRepo interface {
 	GetSubscription(integrationID, subscriptionID string) (*models.Subscription, error)
 	GetSubscriptions(integrationID string) ([]models.Subscription, error)
 	UpdateLastSeen(integrationID string) (*models.Integration, error)
+	UpdateVersionInfo(integrationID, integrationVersion, distributorVersion string) (*models.Integration, error)
 }
 
 //go:generate moq --skip-ensure -pkg db_mock -out ./mock/tasksequencerepo_mock.go . TaskSequenceRepo

diff --git a/shipyard-controller/handler/uniformintegrationhandler.go b/shipyard-controller/handler/uniformintegrationhandler.go
@@ -122,7 +122,11 @@ func (rh *UniformIntegrationHandler) Register(c *gin.Context) {
 	if err != nil {
 		// if the integration already exists, update only needed fields
 		if errors.Is(err, db.ErrUniformRegistrationAlreadyExists) {
-			updateExistingIntegration(rh, integration, hash)
+			err2 := rh.updateExistingIntegration(integration)
+			if err2 != nil {
+				SetInternalServerErrorResponse(err2, c)
+				return
+			}
 			c.JSON(http.StatusOK, &models.RegisterResponse{
 				ID: integration.ID,
 			})
@@ -137,17 +141,22 @@ func (rh *UniformIntegrationHandler) Register(c *gin.Context) {
 	})
 }
 
-func updateExistingIntegration(rh *UniformIntegrationHandler, integration *models.Integration, hash string) {
+func (rh *UniformIntegrationHandler) updateExistingIntegration(integration *models.Integration) error {
+	var err error
+	result, err := rh.uniformRepo.GetUniformIntegrations(models.GetUniformIntegrationsParams{ID: integration.ID})
 
-	result, _ := rh.uniformRepo.GetUniformIntegrations(models.GetUniformIntegrationsParams{ID: hash})
+	if err != nil {
+		return err
+	}
 
 	// update uniform only if there is a version upgrade or downgrade
-
 	if result[0].MetaData.IntegrationVersion != integration.MetaData.IntegrationVersion || result[0].MetaData.DistributorVersion != integration.MetaData.DistributorVersion {
-		rh.uniformRepo.CreateOrUpdateUniformIntegration(*integration)
+		// only update the version information instead of overwriting the complete integration
+		_, err = rh.uniformRepo.UpdateVersionInfo(integration.ID, integration.MetaData.IntegrationVersion, integration.MetaData.DistributorVersion)
 	} else {
-		_, _ = rh.uniformRepo.UpdateLastSeen(integration.ID)
+		_, err = rh.uniformRepo.UpdateLastSeen(integration.ID)
 	}
+	return err
 }
 
 // Unregister deletes a uniform integration

diff --git a/shipyard-controller/handler/uniformintegrationhandler_test.go b/shipyard-controller/handler/uniformintegrationhandler_test.go
@@ -170,11 +170,13 @@ func TestUniformIntegrationHandler_Register(t *testing.T) {
 			wantFuncs:       []string{"GetUniformIntegrationsCalls", "UpdateLastSeenCalls"},
 		},
 		{
-			name: "create existing registration with different version",
+			name: "create existing registration with different version - should call UpdateVersionInfo func",
 			fields: fields{
 				integrationManager: &db_mock.UniformRepoMock{
-					CreateUniformIntegrationFunc:         func(integration models.Integration) error { return db.ErrUniformRegistrationAlreadyExists },
-					CreateOrUpdateUniformIntegrationFunc: func(integration models.Integration) error { return nil },
+					CreateUniformIntegrationFunc: func(integration models.Integration) error { return db.ErrUniformRegistrationAlreadyExists },
+					UpdateVersionInfoFunc: func(integrationID string, integrationVersion string, distributorVersion string) (*models.Integration, error) {
+						return nil, nil
+					},
 					GetUniformIntegrationsFunc: func(filter models.GetUniformIntegrationsParams) ([]models.Integration, error) {
 						return []models.Integration{*myValidIntegration}, nil
 					},
@@ -185,6 +187,24 @@ func TestUniformIntegrationHandler_Register(t *testing.T) {
 			wantIntegration: myValidIntegrationUpdated,
 			wantFuncs:       []string{"GetUniformIntegrationsCalls", "CreateOrUpdateUniformIntegrationCalls"},
 		},
+		{
+			name: "create existing registration with different version - update fails",
+			fields: fields{
+				integrationManager: &db_mock.UniformRepoMock{
+					CreateUniformIntegrationFunc: func(integration models.Integration) error { return db.ErrUniformRegistrationAlreadyExists },
+					UpdateVersionInfoFunc: func(integrationID string, integrationVersion string, distributorVersion string) (*models.Integration, error) {
+						return nil, errors.New("update failed")
+					},
+					GetUniformIntegrationsFunc: func(filter models.GetUniformIntegrationsParams) ([]models.Integration, error) {
+						return []models.Integration{*myValidIntegration}, nil
+					},
+				},
+			},
+			request:         httptest.NewRequest("POST", "/uniform/registration", bytes.NewBuffer(validPayloadUpdated)),
+			wantStatus:      http.StatusInternalServerError,
+			wantIntegration: myValidIntegrationUpdated,
+			wantFuncs:       []string{"GetUniformIntegrationsCalls", "CreateOrUpdateUniformIntegrationCalls"},
+		},
 		{
 			name: "create registration fails",
 			fields: fields{

diff --git a/test/go-tests/test_uniform.go b/test/go-tests/test_uniform.go
@@ -233,6 +233,7 @@ func Test_UniformRegistration_TestAPI(t *testing.T) {
 	// update version of distributor
 	updatedUniformIntegration := uniformIntegration
 	updatedUniformIntegration.MetaData.DistributorVersion = "0.8.4"
+	updatedUniformIntegration.Subscriptions = []keptnmodels.EventSubscription{}
 
 	resp, err = ApiPOSTRequest("/controlPlane/v1/uniform/registration", updatedUniformIntegration, 3)
 	require.Nil(t, err)
@@ -245,6 +246,8 @@ func Test_UniformRegistration_TestAPI(t *testing.T) {
 	err = resp.ToJSON(&integrations)
 
 	require.Equal(t, "0.8.4", integrations[0].MetaData.DistributorVersion)
+	// make sure no subscription has been deleted due to the version update
+	require.Len(t, integrations[0].Subscriptions, 1)
 
 	// delete the integration
 	resp, err = ApiDELETERequest("/controlPlane/v1/uniform/registration/"+registrationResponse.ID, 3)