fix(#730): delete objects that are removed from templates

Author: MatousJobanek

environment/service.go

@@ -88,6 +88,10 @@ func NewService() *Service {
 	return &Service{}
 }
 
+func NewServiceForBlob(templatesRepoBlob string) *Service {
+	return &Service{templatesRepoBlob: templatesRepoBlob}
+}
+
 func NewServiceForUserData(user *authclient.UserDataAttributes) *Service {
 	service := NewService()
 	if user != nil {
@@ -170,10 +174,16 @@ func (s *Service) retrieveTemplates(tmpls []*Template) error {
 	)
 	for _, template := range tmpls {
 		if s.templatesRepoBlob != "" {
-			fileURL := fmt.Sprintf(rawFileURLTemplate, s.getRepo(), s.templatesRepoBlob, s.getPath(template))
+			commit, commitQuotas := getVersions(s.templatesRepoBlob)
+			template.DefaultParams[varCommit] = commit
+			template.DefaultParams[varCommitQuotas] = commitQuotas
+			template.Version = commit
+			if strings.Contains(template.Filename, "quotas") {
+				template.Version = commitQuotas
+			}
+
+			fileURL := fmt.Sprintf(rawFileURLTemplate, s.getRepo(), template.Version, s.getPath(template))
 			content, err = utils.DownloadFile(fileURL)
-			template.DefaultParams[varCommit] = s.templatesRepoBlob
-			template.DefaultParams[varCommitQuotas] = s.templatesRepoBlob
 		} else {
 			content, err = templates.Asset(template.Filename)
 		}
@@ -185,6 +195,16 @@ func (s *Service) retrieveTemplates(tmpls []*Template) error {
 	return nil
 }
 
+func getVersions(blob string) (string, string) {
+	if strings.Contains(blob, "_") {
+		splitVersion := strings.Split(blob, "_")
+		if len(splitVersion) == 2 {
+			return splitVersion[0], splitVersion[1]
+		}
+	}
+	return blob, blob
+}
+
 func (s *Service) getRepo() string {
 	repo := strings.TrimSpace(s.templatesRepo)
 	if repo == "" {

environment/service_test.go

@@ -162,7 +162,7 @@ func XTestDownloadFromExistingLocation(t *testing.T) {
 	}
 }
 
-func TestDownloadFromGivenBlob(t *testing.T) {
+func TestDownloadFromGivenBlobSetAsTenantConfig(t *testing.T) {
 	// given
 	defer gock.OffAll()
 	gock.New("https://raw.githubusercontent.com").
@@ -187,6 +187,68 @@ func TestDownloadFromGivenBlob(t *testing.T) {
 	assert.Equal(t, environment.GetLabelVersion(objects[0]), "987654321")
 }
 
+func TestDownloadFromGivenVersion(t *testing.T) {
+	// given
+	defer gock.OffAll()
+	gock.New("https://raw.githubusercontent.com").
+		Get("fabric8-services/fabric8-tenant/987654321/environment/templates/fabric8-tenant-deploy.yml").
+		Reply(200).
+		BodyString(defaultLocationTempl)
+	testdoubles.SetTemplateVersions()
+	service := environment.NewServiceForBlob("987654321")
+
+	// when
+	envData, err := service.GetEnvData(context.Background(), environment.TypeRun)
+
+	// then
+	require.NoError(t, err)
+	vars := map[string]string{
+		"USER_NAME": "dev",
+	}
+	objects, err := envData.Templates[0].Process(vars)
+	require.NoError(t, err)
+	assert.Len(t, objects, 1)
+	assert.Equal(t, "default-location", environment.GetLabel(objects[0], "test"))
+	assert.Equal(t, "987654321", environment.GetLabelVersion(objects[0]))
+}
+
+func TestDownloadFromGivenVersionThatContainsTwoParts(t *testing.T) {
+	// given
+	defer gock.OffAll()
+	gock.New("https://raw.githubusercontent.com").
+		Get("fabric8-services/fabric8-tenant/98765/environment/templates/fabric8-tenant-che-mt.yml").
+		Reply(200).
+		BodyString(customLocationTempl)
+	gock.New("https://raw.githubusercontent.com").
+		Get("fabric8-services/fabric8-tenant/4321/environment/templates/fabric8-tenant-che-quotas.yml").
+		Reply(200).
+		BodyString(customLocationQuotas)
+	testdoubles.SetTemplateVersions()
+	service := environment.NewServiceForBlob("98765_4321")
+
+	// when
+	envData, err := service.GetEnvData(context.Background(), environment.TypeChe)
+
+	// then
+	require.NoError(t, err)
+	vars := map[string]string{
+		"USER_NAME": "dev",
+	}
+	assert.Len(t, envData.Templates, 2)
+	objects, err := envData.Templates[0].Process(vars)
+	require.NoError(t, err)
+	assert.Len(t, objects, 1)
+	assert.Equal(t, "custom-location", environment.GetLabel(objects[0], "test"))
+	assert.Equal(t, "98765", environment.GetLabelVersion(objects[0]))
+	assert.Equal(t, "4321", environment.GetLabel(objects[0], environment.FieldVersionQuotas))
+
+	objects, err = envData.Templates[1].Process(vars)
+	require.NoError(t, err)
+	assert.Len(t, objects, 1)
+	assert.Empty(t, environment.GetLabel(objects[0], environment.FieldVersionQuotas))
+	assert.Equal(t, environment.GetLabelVersion(objects[0]), "4321")
+}
+
 func TestDownloadFromGivenBlobLocatedInCustomLocation(t *testing.T) {
 	// given
 	defer gock.OffAll()

openshift/action.go

@@ -1,6 +1,7 @@
 package openshift
 
 import (
+	"context"
 	"fmt"
 	"github.com/fabric8-services/fabric8-common/log"
 	"github.com/fabric8-services/fabric8-tenant/cluster"
@@ -84,19 +85,21 @@ type commonNamespaceAction struct {
 	method        string
 	actionOptions *ActionOptions
 	tenantRepo    tenant.Repository
+	filterFunc    FilterFunc
+	requestCtx    context.Context
 }
 
 func (c *commonNamespaceAction) MethodName() string {
 	return c.method
 }
 
-func (c *commonNamespaceAction) getOperationSets(envService EnvironmentTypeService, client Client, filterFunc FilterFunc) (*environment.EnvData, []OperationSet, error) {
-	env, objects, err := envService.GetEnvDataAndObjects(filterFunc)
+func (c *commonNamespaceAction) getOperationSets(envService EnvironmentTypeService, client Client) (*EnvAndObjectsManager, []OperationSet, error) {
+	objectManager, err := envService.GetEnvDataAndObjects()
 	if err != nil {
-		return env, nil, errors.Wrap(err, "getting environment data and objects failed")
+		return objectManager, nil, errors.Wrap(err, "getting environment data and objects failed")
 	}
 
-	operationSets := []OperationSet{NewOperationSet(c.method, objects)}
+	operationSets := []OperationSet{NewOperationSet(c.method, objectManager.GetObjects(c.filterFunc))}
 
 	object, shouldBeAdded := envService.AdditionalObject()
 	if len(object) > 0 {
@@ -112,13 +115,7 @@ func (c *commonNamespaceAction) getOperationSets(envService EnvironmentTypeServi
 	}
 
 	sort.Sort(environment.ByKind(operationSets[0].Objects))
-	return env, operationSets, nil
-}
-
-func (c *commonNamespaceAction) Filter() FilterFunc {
-	return func(objects environment.Object) bool {
-		return true
-	}
+	return objectManager, operationSets, nil
 }
 
 func (c *commonNamespaceAction) ForceMasterTokenGlobally() bool {
@@ -187,12 +184,17 @@ func (c *CreateAction) HealingStrategy() HealingFuncGenerator {
 	}
 }
 
-func NewCreateAction(tenantRepo tenant.Repository, actionOpts *ActionOptions) *CreateAction {
+func NewCreateAction(tenantRepo tenant.Repository, requestCtx context.Context, actionOpts *ActionOptions) *CreateAction {
 	return &CreateAction{
 		commonNamespaceAction: &commonNamespaceAction{
 			method:        http.MethodPost,
 			tenantRepo:    tenantRepo,
-			actionOptions: actionOpts},
+			actionOptions: actionOpts,
+			requestCtx:    requestCtx,
+			filterFunc: func(objects environment.Object) bool {
+				return true
+			},
+		},
 	}
 }
 
@@ -208,13 +210,14 @@ func (c *CreateAction) GetNamespaceEntity(nsTypeService EnvironmentTypeService)
 
 func (c *CreateAction) UpdateNamespace(env *environment.EnvData, cluster *cluster.Cluster, namespace *tenant.Namespace, failed bool) {
 	state := tenant.Ready
+	namespace.Version = env.Version()
 	if failed {
 		state = tenant.Failed
 	}
 	namespace.UpdateData(env, cluster, state)
 	err := c.tenantRepo.SaveNamespace(namespace)
 	if err != nil {
-		sentry.LogError(nil, map[string]interface{}{
+		sentry.LogError(c.requestCtx, map[string]interface{}{
 			"env_type": env.EnvType,
 			"cluster":  cluster.APIURL,
 			"tenant":   namespace.TenantID,
@@ -228,16 +231,27 @@ func (c *CreateAction) ForceMasterTokenGlobally() bool {
 }
 
 func (c *CreateAction) GetOperationSets(envService EnvironmentTypeService, client Client) (*environment.EnvData, []OperationSet, error) {
-	return c.getOperationSets(envService, client, c.Filter())
+	envAndObjectsManager, sets, err := c.getOperationSets(envService, client)
+	if err != nil {
+		return nil, sets, err
+	}
+	return envAndObjectsManager.EnvData, sets, nil
 }
 
-func NewDeleteAction(tenantRepo tenant.Repository, existingNamespaces []*tenant.Namespace, deleteOpts *DeleteActionOption) *DeleteAction {
+func NewDeleteAction(tenantRepo tenant.Repository, requestCtx context.Context, existingNamespaces []*tenant.Namespace, deleteOpts *DeleteActionOption) *DeleteAction {
+	filterFunc := isOfKind(AllKindsToClean...)
+	if deleteOpts.removeFromCluster {
+		filterFunc = isOfKind(environment.ValKindProjectRequest)
+	}
+
 	return &DeleteAction{
 		withExistingNamespacesAction: &withExistingNamespacesAction{
 			commonNamespaceAction: &commonNamespaceAction{
 				method:        http.MethodDelete,
 				tenantRepo:    tenantRepo,
 				actionOptions: deleteOpts.ActionOptions,
+				requestCtx:    requestCtx,
+				filterFunc:    filterFunc,
 			},
 			existingNamespaces: existingNamespaces,
 		},
@@ -263,7 +277,7 @@ func (d *DeleteAction) UpdateNamespace(env *environment.EnvData, cluster *cluste
 		err = d.tenantRepo.DeleteNamespace(namespace)
 	}
 	if err != nil {
-		sentry.LogError(nil, map[string]interface{}{
+		sentry.LogError(d.requestCtx, map[string]interface{}{
 			"env_type":            env.EnvType,
 			"cluster":             cluster.APIURL,
 			"tenant":              namespace.TenantID,
@@ -273,13 +287,6 @@ func (d *DeleteAction) UpdateNamespace(env *environment.EnvData, cluster *cluste
 	}
 }
 
-func (d *DeleteAction) Filter() FilterFunc {
-	if d.deleteOptions.removeFromCluster {
-		return isOfKind(environment.ValKindProjectRequest)
-	}
-	return isOfKind(AllKindsToClean...)
-}
-
 var AllToDeleteAll = []string{environment.ValKindPod, environment.ValKindReplicationController, environment.ValKindDaemonSet,
 	environment.ValKindDeployment, environment.ValKindReplicaSet, environment.ValKindStatefulSet, environment.ValKindJob,
 	environment.ValKindHorizontalPodAutoScaler, environment.ValKindCronJob, environment.ValKindDeploymentConfig,
@@ -291,10 +298,11 @@ var AllToGetAndDelete = []string{environment.ValKindService}
 var AllKindsToClean = append(AllToDeleteAll, AllToGetAndDelete...)
 
 func (d *DeleteAction) GetOperationSets(envService EnvironmentTypeService, client Client) (*environment.EnvData, []OperationSet, error) {
-	env, objectsToDelete, err := envService.GetEnvDataAndObjects(d.Filter())
+	objectManager, err := envService.GetEnvDataAndObjects()
 	if err != nil {
-		return env, nil, errors.Wrap(err, "getting environment data and objects failed")
+		return objectManager.EnvData, nil, errors.Wrap(err, "getting environment data and objects failed")
 	}
+	objectsToDelete := objectManager.GetObjects(d.filterFunc)
 	var operationSets []OperationSet
 
 	if !d.deleteOptions.removeFromCluster {
@@ -310,13 +318,13 @@ func (d *DeleteAction) GetOperationSets(envService EnvironmentTypeService, clien
 			kindToGet := NewObject(kind, envService.GetNamespaceName(), "")
 			result, err := Apply(client, http.MethodGet, kindToGet)
 			if err != nil {
-				return env, nil, errors.Wrapf(err,
+				return objectManager.EnvData, nil, errors.Wrapf(err,
 					"unable to get list of current objects of kind %s in namespace %s", kindToGet, envService.GetNamespaceName())
 			}
 			var returnedObj environment.Object
 			err = yaml.Unmarshal(result.Body, &returnedObj)
 			if err != nil {
-				return env, nil, errors.Wrapf(err, "unable unmarshal object responded from OS "+
+				return objectManager.EnvData, nil, errors.Wrapf(err, "unable unmarshal object responded from OS "+
 					"while getting list of current objects of kind %s in namespace %s", kindToGet, envService.GetNamespaceName())
 			}
 
@@ -338,7 +346,7 @@ func (d *DeleteAction) GetOperationSets(envService EnvironmentTypeService, clien
 	deleteOpSet := NewOperationSet(d.method, objectsToDelete)
 	operationSets = append(operationSets, deleteOpSet)
 
-	return env, operationSets, nil
+	return objectManager.EnvData, operationSets, nil
 }
 
 func NewObject(kind, namespaceName string, name string) environment.Object {
@@ -399,13 +407,16 @@ func (d *DeleteAction) HealingStrategy() HealingFuncGenerator {
 	})
 }
 
-func NewUpdateAction(tenantRepo tenant.Repository, existingNamespaces []*tenant.Namespace, actionOpts *ActionOptions) *UpdateAction {
+func NewUpdateAction(tenantRepo tenant.Repository, requestCtx context.Context, existingNamespaces []*tenant.Namespace, actionOpts *ActionOptions) *UpdateAction {
 	return &UpdateAction{
 		withExistingNamespacesAction: &withExistingNamespacesAction{
 			commonNamespaceAction: &commonNamespaceAction{
 				method:        http.MethodPatch,
 				tenantRepo:    tenantRepo,
-				actionOptions: actionOpts},
+				actionOptions: actionOpts,
+				requestCtx:    requestCtx,
+				filterFunc:    isNotOfKind(environment.ValKindProjectRequest),
+			},
 			existingNamespaces: existingNamespaces,
 		},
 	}
@@ -420,14 +431,15 @@ func (u *UpdateAction) GetNamespaceEntity(nsTypeService EnvironmentTypeService)
 }
 
 func (u *UpdateAction) UpdateNamespace(env *environment.EnvData, cluster *cluster.Cluster, namespace *tenant.Namespace, failed bool) {
-	state := tenant.Ready
-	if failed {
-		state = tenant.Failed
+	state := tenant.Failed
+	if !failed {
+		state = tenant.Ready
+		namespace.Version = env.Version()
 	}
 	namespace.UpdateData(env, cluster, state)
 	err := u.tenantRepo.SaveNamespace(namespace)
 	if err != nil {
-		sentry.LogError(nil, map[string]interface{}{
+		sentry.LogError(u.requestCtx, map[string]interface{}{
 			"env_type": env.EnvType,
 			"cluster":  cluster.APIURL,
 			"tenant":   namespace.TenantID,
@@ -436,12 +448,35 @@ func (u *UpdateAction) UpdateNamespace(env *environment.EnvData, cluster *cluste
 	}
 }
 
-func (u *UpdateAction) Filter() FilterFunc {
-	return isNotOfKind(environment.ValKindProjectRequest)
-}
-
 func (u *UpdateAction) GetOperationSets(envService EnvironmentTypeService, client Client) (*environment.EnvData, []OperationSet, error) {
-	return u.getOperationSets(envService, client, u.Filter())
+	envAndObjectsManager, sets, err := u.getOperationSets(envService, client)
+	if err != nil {
+		return envAndObjectsManager.EnvData, sets, err
+	}
+	previousVersion := u.getNamespaceFor(envService.GetType()).Version
+	objectsToDelete, err := envAndObjectsManager.GetMissingObjectsComparingWith(previousVersion)
+	if err != nil {
+		sentry.LogError(u.requestCtx, map[string]interface{}{
+			"env_type":         envService.GetType(),
+			"cluster":          client.MasterURL,
+			"namespace-name":   envService.GetNamespaceName(),
+			"previous-version": previousVersion,
+		}, err, "unable to retrieve objects that should be removed from the namespace")
+		return envAndObjectsManager.EnvData, sets, nil
+	}
+	if len(objectsToDelete) > 0 {
+		for index, set := range sets {
+			if set.Method == http.MethodDelete {
+				sets[index].Objects = append(sets[index].Objects, objectsToDelete...)
+				sort.Sort(sort.Reverse(environment.ByKind(sets[index].Objects)))
+				return envAndObjectsManager.EnvData, sets, nil
+			}
+		}
+		deleteSet := NewOperationSet(http.MethodDelete, objectsToDelete)
+		sort.Sort(sort.Reverse(environment.ByKind(deleteSet.Objects)))
+		sets = append(sets, deleteSet)
+	}
+	return envAndObjectsManager.EnvData, sets, nil
 }
 
 func (u *UpdateAction) HealingStrategy() HealingFuncGenerator {