From 8212b446411639a4ee3b0631dc15cad67d1a8ce1 Mon Sep 17 00:00:00 2001 From: Matous Jobanek Date: Tue, 5 Feb 2019 10:02:10 +0000 Subject: [PATCH] fix(#730): delete objects that are removed from templates --- environment/service.go | 26 ++- environment/service_test.go | 64 ++++++- openshift/action.go | 113 +++++++---- openshift/action_test.go | 211 +++++++++++++-------- openshift/removed_objects.go | 110 +++++++++++ openshift/removed_objects_whitebox_test.go | 169 +++++++++++++++++ openshift/service.go | 10 +- openshift/service_test.go | 40 +++- openshift/types.go | 91 +++++++-- openshift/types_test.go | 149 ++++++++++++++- openshift/types_whitebox_test.go | 2 +- test/doubles/test_doubles.go | 5 +- 12 files changed, 841 insertions(+), 149 deletions(-) create mode 100644 openshift/removed_objects.go create mode 100644 openshift/removed_objects_whitebox_test.go diff --git a/environment/service.go b/environment/service.go index 6584c1d..0843592 100644 --- a/environment/service.go +++ b/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 == "" { diff --git a/environment/service_test.go b/environment/service_test.go index 54ecda0..832aa93 100644 --- a/environment/service_test.go +++ b/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() diff --git a/openshift/action.go b/openshift/action.go index d7c1c39..976d7a1 100644 --- a/openshift/action.go +++ b/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 { action := c.method @@ -111,13 +114,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 { @@ -186,12 +183,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 + }, + }, } } @@ -207,13 +209,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, @@ -227,16 +230,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(AllToGetAndDelete...) + 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, }, @@ -262,7 +276,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, @@ -272,13 +286,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(AllToGetAndDelete...) -} - var AllToGetAndDelete = []string{environment.ValKindService, environment.ValKindPod, environment.ValKindReplicationController, environment.ValKindDaemonSet, environment.ValKindDeployment, environment.ValKindReplicaSet, environment.ValKindStatefulSet, environment.ValKindJob, environment.ValKindHorizontalPodAutoScaler, environment.ValKindCronJob, environment.ValKindDeploymentConfig, @@ -286,16 +293,17 @@ var AllToGetAndDelete = []string{environment.ValKindService, environment.ValKind environment.ValKindPersistentVolumeClaim, environment.ValKindConfigMap} func (d *DeleteAction) GetOperationSets(envService EnvironmentTypeService, client Client) (*environment.EnvData, []OperationSet, error) { - env, toDelete, 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") } + toDelete := objectManager.GetObjects(d.filterFunc) var operationSets []OperationSet if !d.deleteOptions.removeFromCluster { var err error toDelete, err = getCleanObjects(client, envService.GetNamespaceName()) if err != nil { - return env, nil, err + return objectManager.EnvData, nil, err } operationSets = append(operationSets, NewOperationSet(http.MethodDelete, toDelete)) operationSets = append(operationSets, NewOperationSet(EnsureDeletion, toDelete)) @@ -304,7 +312,7 @@ func (d *DeleteAction) GetOperationSets(envService EnvironmentTypeService, clien } sort.Sort(sort.Reverse(environment.ByKind(toDelete))) - return env, operationSets, nil + return objectManager.EnvData, operationSets, nil } func getCleanObjects(client Client, namespaceName string) (environment.Objects, error) { @@ -406,13 +414,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, }, } @@ -427,14 +438,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, @@ -443,12 +455,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 { diff --git a/openshift/action_test.go b/openshift/action_test.go index 9245f84..346f38b 100644 --- a/openshift/action_test.go +++ b/openshift/action_test.go @@ -45,7 +45,7 @@ func (s *ActionTestSuite) TestCreateAction() { defer reset() // when - create := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()) + create := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()) // then s.T().Run("method name should match", func(t *testing.T) { @@ -54,7 +54,12 @@ func (s *ActionTestSuite) TestCreateAction() { s.T().Run("filter method should always return true", func(t *testing.T) { for _, obj := range getObjectsOfAllKinds() { - assert.True(t, create.Filter()(obj)) + _, sets, err := create.GetOperationSets(NewOneObjectService(obj), openshift.Client{}) + // then + require.NoError(t, err) + require.Len(t, sets, 1) + assert.Len(t, sets[0].Objects, 1) + assert.Equal(t, obj, sets[0].Objects[0]) } }) @@ -190,7 +195,7 @@ func (s *ActionTestSuite) TestCreateAction() { userModifier := testdoubles.AddUser("developer") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then assert.NoError(t, err) @@ -216,7 +221,7 @@ func (s *ActionTestSuite) TestCreateAction() { userModifier := testdoubles.AddUser("dev") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then assert.NoError(t, err) @@ -243,7 +248,7 @@ func (s *ActionTestSuite) TestCreateAction() { userModifier := testdoubles.AddUser("developertofail") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then test.AssertError(t, err, @@ -277,7 +282,7 @@ func (s *ActionTestSuite) TestCreateAction() { userModifier := testdoubles.AddUser("anotherdev") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then test.AssertError(t, err, @@ -294,7 +299,7 @@ func (s *ActionTestSuite) TestCreateAction() { errorChan <- fmt.Errorf("first dummy error") close(errorChan) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().DisableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().DisableSelfHealing()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then test.AssertError(t, err, test.HasMessageContaining("first dummy error")) @@ -305,7 +310,7 @@ func (s *ActionTestSuite) TestCreateAction() { errorChan := make(chan error, 10) close(errorChan) // when - err := openshift.NewCreateAction(repo, openshift.CreateOpts().EnableSelfHealing()). + err := openshift.NewCreateAction(repo, nil, openshift.CreateOpts().EnableSelfHealing()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then assert.NoError(t, err) @@ -324,32 +329,14 @@ func (s *ActionTestSuite) TestDeleteAction() { client := openshift.NewClient(nil, test.ClusterURL, tokenProducer) // when - delete := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()) - deleteFromCluster := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()) + delete := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()) + deleteFromCluster := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()) // then s.T().Run("method name should match", func(t *testing.T) { assert.Equal(t, "DELETE", delete.MethodName()) }) - s.T().Run("verify filter method", func(t *testing.T) { - for _, obj := range getObjectsOfAllKinds() { - if environment.GetKind(obj) == "ProjectRequest" { - assert.False(t, delete.Filter()(obj), obj.ToString()) - assert.True(t, deleteFromCluster.Filter()(obj), obj.ToString()) - } else { - assert.False(t, deleteFromCluster.Filter()(obj), obj.ToString()) - if environment.GetKind(obj) == "PersistentVolumeClaim" || environment.GetKind(obj) == "ConfigMap" || - environment.GetKind(obj) == "Service" || environment.GetKind(obj) == "DeploymentConfig" || environment.GetKind(obj) == "Route" || - environment.GetKind(obj) == "Job" || environment.GetKind(obj) == "Deployment" { - assert.True(t, delete.Filter()(obj), obj.ToString()) - } else { - assert.False(t, delete.Filter()(obj), obj.ToString()) - } - } - } - }) - s.T().Run("GetOperationSets method should do reverse sorted and delete all objects for clean", func(t *testing.T) { // given defer gock.OffAll() @@ -455,13 +442,10 @@ func (s *ActionTestSuite) TestDeleteAction() { // then assert.NoError(t, err) assert.Len(t, sets, 1) - length := len(allKinds) assert.Equal(t, "DELETE", sets[0].Method) sorted := sets[0].Objects - require.Len(t, sorted, length) - assert.Equal(t, environment.ValKindProjectRequest, environment.GetKind(sorted[length-1])) - assert.Equal(t, environment.ValKindRole, environment.GetKind(sorted[length-2])) - assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(sorted[length-3])) + require.Len(t, sorted, 1) + assert.Equal(t, environment.ValKindProjectRequest, environment.GetKind(sorted[0])) }) s.T().Run("it should require master token globally", func(t *testing.T) { @@ -578,7 +562,7 @@ func (s *ActionTestSuite) TestDeleteAction() { userModifier := testdoubles.AddUser("developer") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then assert.NoError(t, err) @@ -604,7 +588,7 @@ func (s *ActionTestSuite) TestDeleteAction() { userModifier := testdoubles.AddUser("developer") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then assert.NoError(t, err) @@ -630,7 +614,7 @@ func (s *ActionTestSuite) TestDeleteAction() { userModifier := testdoubles.AddUser("anotherdev") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then test.AssertError(t, err, @@ -659,7 +643,7 @@ func (s *ActionTestSuite) TestDeleteAction() { userModifier := testdoubles.AddUser("anotherdev") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().EnableSelfHealing().RemoveFromCluster()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then test.AssertError(t, err, @@ -675,7 +659,7 @@ func (s *ActionTestSuite) TestDeleteAction() { errorChan <- fmt.Errorf("first dummy error") close(errorChan) // when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().DisableSelfHealing().RemoveFromCluster()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().DisableSelfHealing().RemoveFromCluster()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then test.AssertError(t, err, test.HasMessageContaining("first dummy error")) @@ -687,7 +671,7 @@ func (s *ActionTestSuite) TestDeleteAction() { errorChan <- fmt.Errorf("first dummy error") close(errorChan) // and also when - err := openshift.NewDeleteAction(repo, fxt.Namespaces, openshift.DeleteOpts().DisableSelfHealing()). + err := openshift.NewDeleteAction(repo, nil, fxt.Namespaces, openshift.DeleteOpts().DisableSelfHealing()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then test.AssertError(t, err, test.HasMessageContaining("first dummy error")) @@ -721,7 +705,7 @@ func (s *ActionTestSuite) TestUpdateAction() { defer reset() // when - update := openshift.NewUpdateAction(repo, namespaces, openshift.UpdateOpts().EnableSelfHealing()) + update := openshift.NewUpdateAction(repo, nil, namespaces, openshift.UpdateOpts().EnableSelfHealing()) // then s.T().Run("method name should match", func(t *testing.T) { @@ -730,45 +714,70 @@ func (s *ActionTestSuite) TestUpdateAction() { s.T().Run("filter method should always return true except for project request", func(t *testing.T) { for _, obj := range getObjectsOfAllKinds() { + _, sets, err := update.GetOperationSets(NewOneObjectService(obj), openshift.Client{}) + require.NoError(t, err) + require.Len(t, sets, 2) if environment.GetKind(obj) == "ProjectRequest" { - assert.False(t, update.Filter()(obj)) + assert.Empty(t, sets[0].Objects) } else { - assert.True(t, update.Filter()(obj)) + assert.Len(t, sets[0].Objects, 1) + assert.Equal(t, obj, sets[0].Objects[0]) } } }) s.T().Run("GetOperationSets should not add additional set and should sort the objects", func(t *testing.T) { + // given + openshift.CacheOfRemovedObjects = &openshift.RemovedObjectsCache{} // when _, sets, err := update.GetOperationSets(NewAllTypesService(nil, true), openshift.Client{}) // then assert.NoError(t, err) - require.Len(t, sets, 1) + require.Len(t, sets, 2) assert.Equal(t, "PATCH", sets[0].Method) sorted := sets[0].Objects - require.Len(t, sorted, len(allKinds)) - assert.Equal(t, environment.ValKindProjectRequest, environment.GetKind(sorted[0])) - assert.Equal(t, environment.ValKindRole, environment.GetKind(sorted[1])) - assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(sorted[2])) + require.Len(t, sorted, len(allKinds)-1) + assert.Equal(t, environment.ValKindRole, environment.GetKind(sorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(sorted[1])) + assert.Equal(t, environment.ValKindLimitRange, environment.GetKind(sorted[2])) + + assert.Equal(t, "DELETE", sets[1].Method) + deleteSorted := sets[1].Objects + require.Len(t, deleteSorted, 3) + assert.Equal(t, environment.ValKindRoleBinding, environment.GetKind(deleteSorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(deleteSorted[1])) + assert.Equal(t, environment.ValKindRole, environment.GetKind(deleteSorted[2])) }) s.T().Run("GetOperationSets should add additional object to existing set and sort the objects", func(t *testing.T) { + // given + openshift.CacheOfRemovedObjects = &openshift.RemovedObjectsCache{} // when _, sets, err := update.GetOperationSets(NewAllTypesService(myDummyRole, true), openshift.Client{}) // then assert.NoError(t, err) - require.Len(t, sets, 1) + require.Len(t, sets, 2) assert.Equal(t, "PATCH", sets[0].Method) patchSorted := sets[0].Objects - require.Len(t, patchSorted, len(allKinds)+1) - assert.Equal(t, environment.ValKindProjectRequest, environment.GetKind(patchSorted[0])) + require.Len(t, patchSorted, len(allKinds)) + assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[0])) assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[1])) - assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[2])) - assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(patchSorted[3])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(patchSorted[2])) + assert.Equal(t, environment.ValKindLimitRange, environment.GetKind(patchSorted[3])) + + assert.Equal(t, "DELETE", sets[1].Method) + deleteSorted := sets[1].Objects + require.Len(t, deleteSorted, 3) + assert.Equal(t, environment.ValKindRoleBinding, environment.GetKind(deleteSorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(deleteSorted[1])) + assert.Equal(t, environment.ValKindRole, environment.GetKind(deleteSorted[2])) + }) s.T().Run("GetOperationSets should add additional object to new set and sort the objects", func(t *testing.T) { + // given + openshift.CacheOfRemovedObjects = &openshift.RemovedObjectsCache{} // when _, sets, err := update.GetOperationSets(NewAllTypesService(myDummyRole, false), openshift.Client{}) // then @@ -776,15 +785,34 @@ func (s *ActionTestSuite) TestUpdateAction() { require.Len(t, sets, 2) assert.Equal(t, "PATCH", sets[0].Method) patchSorted := sets[0].Objects - require.Len(t, patchSorted, len(allKinds)) - assert.Equal(t, environment.ValKindProjectRequest, environment.GetKind(patchSorted[0])) - assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[1])) - assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(patchSorted[2])) + require.Len(t, patchSorted, len(allKinds)-1) + assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(patchSorted[1])) + assert.Equal(t, environment.ValKindLimitRange, environment.GetKind(patchSorted[2])) assert.Equal(t, "DELETE", sets[1].Method) deleteSorted := sets[1].Objects - require.Len(t, deleteSorted, 1) - assert.Equal(t, myDummyRole, deleteSorted[0]) + require.Len(t, deleteSorted, 4) + assert.Equal(t, environment.ValKindRoleBinding, environment.GetKind(deleteSorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(deleteSorted[1])) + assert.Equal(t, environment.ValKindRole, environment.GetKind(deleteSorted[2])) + assert.Equal(t, environment.ValKindRole, environment.GetKind(deleteSorted[3])) + }) + + s.T().Run("GetOperationSets should not add missing objects nor fail when receives error", func(t *testing.T) { + // given + openshift.CacheOfRemovedObjects = &openshift.RemovedObjectsCache{} + // when + _, sets, err := update.GetOperationSets(NewAllTypesServiceWithError(true), openshift.Client{}) + // then + assert.NoError(t, err) + require.Len(t, sets, 1) + assert.Equal(t, "PATCH", sets[0].Method) + patchSorted := sets[0].Objects + require.Len(t, patchSorted, len(allKinds)-1) + assert.Equal(t, environment.ValKindRole, environment.GetKind(patchSorted[0])) + assert.Equal(t, environment.ValKindRoleBindingRestriction, environment.GetKind(patchSorted[1])) + assert.Equal(t, environment.ValKindLimitRange, environment.GetKind(patchSorted[2])) }) s.T().Run("it should require master token globally", func(t *testing.T) { @@ -793,7 +821,7 @@ func (s *ActionTestSuite) TestUpdateAction() { for _, envType := range environment.DefaultEnvTypes { // given - envService, envData := gewEnvServiceWithData(s.T(), envType, config) + envService, _ := gewEnvServiceWithData(s.T(), envType, config) // verify getting namespace - it should return only if exists namespace, err := update.GetNamespaceEntity(envService) @@ -815,6 +843,9 @@ func (s *ActionTestSuite) TestUpdateAction() { // verify namespace update to ready s.T().Run("update namespace to ready", func(t *testing.T) { + // given + testdoubles.SetTemplateVersions() + _, envData := gewEnvServiceWithData(s.T(), envType, config) // when update.UpdateNamespace(envData, &cluster.Cluster{APIURL: test.ClusterURL}, namespace, false) // then @@ -822,20 +853,24 @@ func (s *ActionTestSuite) TestUpdateAction() { HasNumberOfNamespaces(2). HasNamespaceOfTypeThat(envType). HasState(tenant.Ready). - HasMasterURL(test.ClusterURL) + HasMasterURL(test.ClusterURL). + HasCurrentCompleteVersion() }) // verify namespace update to failed s.T().Run("update namespace to failed", func(t *testing.T) { + // given + testdoubles.SetTemplateSameVersion("xxx") + _, envData := gewEnvServiceWithData(s.T(), envType, config) // when update.UpdateNamespace(envData, &cluster.Cluster{APIURL: test.ClusterURL}, namespace, true) // then - assertion.AssertTenant(t, repo). HasNumberOfNamespaces(2). HasNamespaceOfTypeThat(envType). HasState(tenant.Failed). - HasMasterURL(test.ClusterURL) + HasMasterURL(test.ClusterURL). + HasCurrentCompleteVersion() }) } @@ -880,7 +915,7 @@ func (s *ActionTestSuite) TestUpdateAction() { userModifier := testdoubles.AddUser("developer") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewUpdateAction(repo, namespaces, openshift.UpdateOpts().EnableSelfHealing()). + err := openshift.NewUpdateAction(repo, nil, namespaces, openshift.UpdateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then assert.NoError(t, err) @@ -909,7 +944,7 @@ func (s *ActionTestSuite) TestUpdateAction() { userModifier := testdoubles.AddUser("anotherdev") serviceBuilder := testdoubles.NewOSService(config, userModifier, repo) // when - err := openshift.NewUpdateAction(repo, namespaces, openshift.UpdateOpts().EnableSelfHealing()). + err := openshift.NewUpdateAction(repo, nil, namespaces, openshift.UpdateOpts().EnableSelfHealing()). HealingStrategy()(serviceBuilder)(fmt.Errorf("some error")) // then test.AssertError(t, err, @@ -927,7 +962,7 @@ func (s *ActionTestSuite) TestUpdateAction() { errorChan <- fmt.Errorf("first dummy error") close(errorChan) // when - err := openshift.NewUpdateAction(repo, namespaces, openshift.UpdateOpts().DisableSelfHealing()). + err := openshift.NewUpdateAction(repo, nil, namespaces, openshift.UpdateOpts().DisableSelfHealing()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then test.AssertError(t, err, test.HasMessageContaining("first dummy error")) @@ -938,7 +973,7 @@ func (s *ActionTestSuite) TestUpdateAction() { errorChan := make(chan error, 10) close(errorChan) // when - err := openshift.NewUpdateAction(repo, namespaces, openshift.UpdateOpts().EnableSelfHealing()). + err := openshift.NewUpdateAction(repo, nil, namespaces, openshift.UpdateOpts().EnableSelfHealing()). ManageAndUpdateResults(errorChan, []environment.Type{environment.TypeChe}, returnErrHealing) // then assert.NoError(t, err) @@ -952,18 +987,17 @@ func gewEnvServiceWithData(t *testing.T, envType environment.Type, config *confi return "HMs8laMmBSsJi8hpMDOtiglbXJ-2eyymE1X46ax5wX8" }) service := openshift.NewEnvironmentTypeService(envType, osContext, environment.NewService()) - data, _, err := service.GetEnvDataAndObjects(func(objects environment.Object) bool { - return true - }) + envDataMngr, err := service.GetEnvDataAndObjects() assert.NoError(t, err) - return service, data + return service, envDataMngr.EnvData } type allTypesService struct { *openshift.CommonEnvTypeService - allObjects environment.Objects - additionalObject environment.Object - shouldBeAdded bool + allObjects environment.Objects + additionalObject environment.Object + shouldBeAdded bool + objForVersionReturnErr bool } func NewAllTypesService(additionalObject environment.Object, shouldBeAdded bool) allTypesService { @@ -974,8 +1008,35 @@ func NewAllTypesService(additionalObject environment.Object, shouldBeAdded bool) } } -func (s allTypesService) GetEnvDataAndObjects(filter openshift.FilterFunc) (*environment.EnvData, environment.Objects, error) { - return nil, s.allObjects, nil +func NewAllTypesServiceWithError(objForVersionReturnErr bool) allTypesService { + return allTypesService{ + allObjects: getObjectsOfAllKinds(), + objForVersionReturnErr: objForVersionReturnErr, + } +} + +func NewOneObjectService(obj environment.Object) allTypesService { + return allTypesService{ + allObjects: environment.Objects{obj}, + } +} + +func (s allTypesService) GetEnvDataAndObjects() (*openshift.EnvAndObjectsManager, error) { + return openshift.NewEnvAndObjectsManager(&environment.EnvData{}, s.allObjects, + func(version string) (data *environment.EnvData, objects environment.Objects, e error) { + if s.objForVersionReturnErr { + return nil, nil, fmt.Errorf("some error") + } + return &environment.EnvData{}, environment.Objects{ + openshift.NewObject(environment.ValKindRole, s.GetNamespaceName(), "r-to-remove"), + openshift.NewObject(environment.ValKindRoleBinding, s.GetNamespaceName(), "rbr-to-remove"), + openshift.NewObject(environment.ValKindRoleBindingRestriction, s.GetNamespaceName(), "rbr-to-remove"), + }, nil + }, s.GetType(), s.GetNamespaceName()), nil +} + +func (s allTypesService) GetType() environment.Type { + return environment.TypeJenkins } func (s allTypesService) GetNamespaceName() string { diff --git a/openshift/removed_objects.go b/openshift/removed_objects.go new file mode 100644 index 0000000..2a9e069 --- /dev/null +++ b/openshift/removed_objects.go @@ -0,0 +1,110 @@ +package openshift + +import ( + "github.com/fabric8-services/fabric8-tenant/environment" + "sync" +) + +var CacheOfRemovedObjects = &RemovedObjectsCache{ + removedObjectsResolvers: make(map[string]map[environment.Type]*removedObjectsResolver), +} + +type RemovedObjectsCache struct { + mux sync.Mutex + removedObjectsResolvers map[string]map[environment.Type]*removedObjectsResolver +} + +type removedObjectsResolver struct { + mux sync.Mutex + resolved bool + allCurrentObjects environment.Objects + removedObjects []removedObject + envType environment.Type + objectsToCompare objectsToCompareRetrieval +} + +type objectsForVersionRetrieval func(version string) (*environment.EnvData, environment.Objects, error) +type objectsToCompareRetrieval func() (*environment.EnvData, environment.Objects, error) + +func (c *RemovedObjectsCache) GetResolver(envType environment.Type, version string, objectsForVersion objectsForVersionRetrieval, + allCurrentObjects environment.Objects) *removedObjectsResolver { + + c.mux.Lock() + defer c.mux.Unlock() + + if c.removedObjectsResolvers == nil { + c.removedObjectsResolvers = make(map[string]map[environment.Type]*removedObjectsResolver) + } + + if forVersion, forVersionExists := c.removedObjectsResolvers[version]; forVersionExists { + if removedObjectsResolver, forTypeExists := forVersion[envType]; forTypeExists { + return removedObjectsResolver + } + } else { + c.removedObjectsResolvers[version] = make(map[environment.Type]*removedObjectsResolver) + } + resolver := &removedObjectsResolver{ + envType: envType, + allCurrentObjects: allCurrentObjects, + objectsToCompare: func() (data *environment.EnvData, objects environment.Objects, e error) { + return objectsForVersion(version) + }} + c.removedObjectsResolvers[version][envType] = resolver + return resolver +} + +func (c *RemovedObjectsCache) RemoveResolver(envType environment.Type, version string) { + + c.mux.Lock() + defer c.mux.Unlock() + + if forVersion, forVersionExists := c.removedObjectsResolvers[version]; forVersionExists { + delete(forVersion, envType) + } +} + +func (r *removedObjectsResolver) Resolve() ([]removedObject, error) { + r.mux.Lock() + defer r.mux.Unlock() + + if r.resolved { + return r.removedObjects, nil + } + + _, objects, err := r.objectsToCompare() + if err != nil { + return nil, err + } + var removedObjects []removedObject + for _, obj := range objects { + found := false + for _, currentObj := range r.allCurrentObjects { + if environment.GetKind(obj) == environment.GetKind(currentObj) && + environment.GetName(obj) == environment.GetName(currentObj) && + environment.GetNamespace(obj) == environment.GetNamespace(currentObj) { + found = true + break + } + } + if !found { + removedObjects = append(removedObjects, removedObject{ + kind: environment.GetKind(obj), + name: environment.GetName(obj), + }) + } + } + + r.removedObjects = removedObjects + r.resolved = true + + return removedObjects, nil +} + +type removedObject struct { + kind string + name string +} + +func (o removedObject) toObject(namespaceName string) environment.Object { + return NewObject(o.kind, namespaceName, o.name) +} diff --git a/openshift/removed_objects_whitebox_test.go b/openshift/removed_objects_whitebox_test.go new file mode 100644 index 0000000..3dff448 --- /dev/null +++ b/openshift/removed_objects_whitebox_test.go @@ -0,0 +1,169 @@ +package openshift + +import ( + "fmt" + "github.com/fabric8-services/fabric8-tenant/environment" + "github.com/fabric8-services/fabric8-tenant/test" + "github.com/stretchr/testify/require" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func singleObjectResolver(obj environment.Object) objectsForVersionRetrieval { + return func(version string) (data *environment.EnvData, objects environment.Objects, e error) { + return &environment.EnvData{}, environment.Objects{obj}, nil + } +} + +func slowObjectResolver(duration time.Duration, obj environment.Object) objectsForVersionRetrieval { + return func(version string) (data *environment.EnvData, objects environment.Objects, e error) { + time.Sleep(duration) + return &environment.EnvData{}, environment.Objects{obj}, nil + } +} + +func errorResolver() objectsForVersionRetrieval { + return func(version string) (data *environment.EnvData, objects environment.Objects, e error) { + return nil, nil, fmt.Errorf("some error") + } +} + +func TestCacheReturnCachedDataSingleThread(t *testing.T) { + // given + c := RemovedObjectsCache{} + + toReturn := NewObject(environment.ValKindRoleBinding, "ns-name", "rb-name") + otherObj := NewObject(environment.ValKindRoleBinding, "ns-name", "rb-name") + + // when + first, err := c.GetResolver(environment.TypeChe, "123abc", singleObjectResolver(toReturn), environment.Objects{}).Resolve() + require.NoError(t, err) + second, err := c.GetResolver(environment.TypeChe, "123abc", singleObjectResolver(otherObj), environment.Objects{}).Resolve() + require.NoError(t, err) + + // then + require.Len(t, first, 1) + assertThatObjectsAreSame(t, toReturn, first[0].toObject("ns-name")) + require.Len(t, second, 1) + assertThatObjectsAreSame(t, toReturn, second[0].toObject("ns-name")) +} + +func TestCacheReturnCachedDataMultiThread(t *testing.T) { + // given + c := RemovedObjectsCache{} + + wg := sync.WaitGroup{} + trigger := sync.WaitGroup{} + trigger.Add(1) + + fetch := 10 + + // when + for _, envType := range environment.DefaultEnvTypes { + + obj := NewObject(environment.ValKindRole, "ns-name", fmt.Sprintf("r-%s", envType)) + + for i := 0; i < fetch; i++ { + wg.Add(1) + go func(env environment.Type, toReturn environment.Object) { + defer wg.Done() + trigger.Wait() + objs, err := c.GetResolver(env, "123abc", slowObjectResolver(1*time.Second, toReturn), environment.Objects{}).Resolve() + + // then + require.NoError(t, err) + require.Len(t, objs, 1) + assertThatObjectsAreSame(t, toReturn, objs[0].toObject("ns-name")) + }(envType, obj) + + } + for i := 0; i < fetch; i++ { + wg.Add(1) + go func(env environment.Type, index int) { + defer wg.Done() + version := fmt.Sprintf("%d-version", index) + versioned := NewObject(environment.ValKindRole, "ns-name", fmt.Sprintf("r-%s-%d", env, index)) + + trigger.Wait() + objs, err := c.GetResolver(env, version, slowObjectResolver(1*time.Second, versioned), environment.Objects{}).Resolve() + + // then + require.NoError(t, err) + require.Len(t, objs, 1) + assert.Equal(t, versioned, objs[0].toObject("ns-name")) + }(envType, i) + } + } + trigger.Done() + wg.Wait() +} + +func TestCacheWhenResolverReturnsErrorForTheSecondCall(t *testing.T) { + // given + c := RemovedObjectsCache{} + + toReturn := NewObject(environment.ValKindRoleBinding, "ns-name", "rb-name") + + // when + first, err := c.GetResolver(environment.TypeChe, "123abc", singleObjectResolver(toReturn), environment.Objects{}).Resolve() + require.NoError(t, err) + second, err := c.GetResolver(environment.TypeChe, "123abc", errorResolver(), environment.Objects{}).Resolve() + require.NoError(t, err) + + // then + require.Len(t, first, 1) + assertThatObjectsAreSame(t, toReturn, first[0].toObject("ns-name")) + require.Len(t, second, 1) + assertThatObjectsAreSame(t, toReturn, second[0].toObject("ns-name")) +} + +func TestCacheWhenResolverReturnsErrorForTheFirstCallAndIsRemoved(t *testing.T) { + // given + c := RemovedObjectsCache{} + + toReturn := NewObject(environment.ValKindRoleBinding, "ns-name", "rb-name") + + // when + first, err := c.GetResolver(environment.TypeChe, "123abc", errorResolver(), environment.Objects{}).Resolve() + + // then + assert.Error(t, err, test.HasMessage("some error")) + assert.Nil(t, first) + + // and when + c.RemoveResolver(environment.TypeChe, "123abc") + second, err := c.GetResolver(environment.TypeChe, "123abc", singleObjectResolver(toReturn), environment.Objects{}).Resolve() + require.NoError(t, err) + + // then + require.Len(t, second, 1) + assertThatObjectsAreSame(t, toReturn, second[0].toObject("ns-name")) +} + +func TestRemovedObject(t *testing.T) { + // given + rmObject := removedObject{ + kind: environment.ValKindRole, + name: "my-role", + } + + // when + obj := rmObject.toObject("my-namespace") + + // then + require.Len(t, obj, 2) + assert.Equal(t, environment.ValKindRole, environment.GetKind(obj)) + require.Contains(t, obj, "metadata") + require.Len(t, obj["metadata"], 2) + assert.Equal(t, "my-namespace", environment.GetNamespace(obj)) + assert.Equal(t, "my-role", environment.GetName(obj)) +} + +func assertThatObjectsAreSame(t *testing.T, expected environment.Object, actual environment.Object) { + assert.Equal(t, environment.GetKind(expected), environment.GetKind(actual)) + assert.Equal(t, environment.GetName(expected), environment.GetName(actual)) + assert.Equal(t, environment.GetNamespace(expected), environment.GetNamespace(actual)) +} diff --git a/openshift/service.go b/openshift/service.go index 628d303..dcf08b1 100644 --- a/openshift/service.go +++ b/openshift/service.go @@ -69,15 +69,18 @@ func NewServiceContext(callerCtx context.Context, config *configuration.Data, cl } func (b *ServiceBuilder) Create(nsTypes []environment.Type, actionOpts *ActionOptions) error { - return b.service.processAndApplyAll(nsTypes, NewCreateAction(b.service.tenantRepository, actionOpts)) + return b.service.processAndApplyAll(nsTypes, + NewCreateAction(b.service.tenantRepository, b.service.context.requestCtx, actionOpts)) } func (b *ServiceBuilder) Update(nsTypes []environment.Type, existingNamespaces []*tenant.Namespace, actionOpts *ActionOptions) error { - return b.service.processAndApplyAll(nsTypes, NewUpdateAction(b.service.tenantRepository, existingNamespaces, actionOpts)) + return b.service.processAndApplyAll(nsTypes, + NewUpdateAction(b.service.tenantRepository, b.service.context.requestCtx, existingNamespaces, actionOpts)) } func (b *ServiceBuilder) Delete(nsTypes []environment.Type, existingNamespaces []*tenant.Namespace, deleteOpts *DeleteActionOption) error { - return b.service.processAndApplyAll(nsTypes, NewDeleteAction(b.service.tenantRepository, existingNamespaces, deleteOpts)) + return b.service.processAndApplyAll(nsTypes, + NewDeleteAction(b.service.tenantRepository, b.service.context.requestCtx, existingNamespaces, deleteOpts)) } func (s *Service) processAndApplyAll(nsTypes []environment.Type, action NamespaceAction) error { @@ -146,7 +149,6 @@ func processAndApplyNs(nsTypeWait *sync.WaitGroup, nsTypeService EnvironmentType if err != nil { errorChan <- errors.Wrapf(err, "the after callback of a namespace %s failed for the type %s", action.MethodName(), nsTypeService.GetNamespaceName()) } - namespace.Version = env.Version() action.UpdateNamespace(env, &cluster, namespace, failed || err != nil) } diff --git a/openshift/service_test.go b/openshift/service_test.go index c0812a2..ece6bde 100644 --- a/openshift/service_test.go +++ b/openshift/service_test.go @@ -410,7 +410,7 @@ func (s *ServiceTestSuite) TestCleanWhenGetForServicesReturns500() { defer reset() gock.New(test.ClusterURL). - Get("/api/v1/namespaces/john-jenkins/services"). + Get("/api/v1/namespaces/john[^/]*/services"). Persist(). Reply(500) fxt := tf.FillDB(s.T(), s.DB, tf.AddSpecificTenants(tf.SingleWithName("john")), tf.AddDefaultNamespaces()) @@ -686,3 +686,41 @@ func (s *ServiceTestSuite) TestCreateNewNamespacesWithNormalBaseNameWhenFailsRes require.NoError(s.T(), err) assert.Len(s.T(), namespaces, 5) } + +func (s *ServiceTestSuite) TestUpdateWithDeletesOfRemovedObjects() { + // given + defer gock.OffAll() + config, reset := test.LoadTestConfig(s.T()) + defer reset() + testdoubles.SetTemplateVersions() + + calls := 0 + deleteCalls := 0 + testdoubles.MockPatchRequestsToOS(&calls, test.ClusterURL) + mockCallsToGetTemplates(s.T(), "0000", "developer", true) + gock.New(test.ClusterURL). + Delete("/oapi/v1/namespaces/developer.*/rolebindings/view-rb-to-remove"). + SetMatcher(test.SpyOnCalls(&deleteCalls)). + Times(5). + Reply(200) + + userCreator := testdoubles.AddUser("developer").WithToken("12345") + + fxt := tf.FillDB(s.T(), s.DB, tf.AddSpecificTenants(tf.SingleWithName("developer")), tf.AddDefaultNamespaces().Outdated()) + tnnt := fxt.Tenants[0] + service := testdoubles.NewOSService( + config, + userCreator, + tenant.NewTenantRepository(s.DB, tnnt.ID)) + + // when + err := service.Update(environment.DefaultEnvTypes, fxt.Namespaces, openshift.UpdateOpts().EnableSelfHealing()) + + // then + require.NoError(s.T(), err) + assert.Equal(s.T(), testdoubles.ExpectedNumberOfCallsWhenPatch(s.T(), config, environment.DefaultEnvTypes...), calls) + assert.Equal(s.T(), 5, deleteCalls) + namespaces, err := tenant.NewTenantRepository(s.DB, tnnt.ID).GetNamespaces() + require.NoError(s.T(), err) + assert.Len(s.T(), namespaces, 5) +} diff --git a/openshift/types.go b/openshift/types.go index ead6c1c..06939bf 100644 --- a/openshift/types.go +++ b/openshift/types.go @@ -17,7 +17,7 @@ import ( type EnvironmentTypeService interface { GetType() environment.Type GetNamespaceName() string - GetEnvDataAndObjects(filter FilterFunc) (*environment.EnvData, environment.Objects, error) + GetEnvDataAndObjects() (*EnvAndObjectsManager, error) GetCluster() cluster.Cluster AfterCallback(client *Client, action string) error GetTokenProducer(forceMasterTokenGlobally bool) TokenProducer @@ -26,7 +26,7 @@ type EnvironmentTypeService interface { func NewEnvironmentTypeService(envType environment.Type, context *ServiceContext, envService *environment.Service) EnvironmentTypeService { service := &CommonEnvTypeService{ - name: envType, + envType: envType, context: context, envService: envService, } @@ -42,32 +42,46 @@ func NewEnvironmentTypeService(envType environment.Type, context *ServiceContext } type CommonEnvTypeService struct { - name environment.Type + envType environment.Type context *ServiceContext envService *environment.Service } func (t *CommonEnvTypeService) GetType() environment.Type { - return t.name + return t.envType } func (t *CommonEnvTypeService) GetNamespaceName() string { - return fmt.Sprintf("%s-%s", t.context.nsBaseName, t.name) + return fmt.Sprintf("%s-%s", t.context.nsBaseName, t.envType) } -func (t *CommonEnvTypeService) GetEnvDataAndObjects(filter FilterFunc) (*environment.EnvData, environment.Objects, error) { - envData, err := t.envService.GetEnvData(t.context.requestCtx, t.name) +func (t *CommonEnvTypeService) GetEnvDataAndObjects() (*EnvAndObjectsManager, error) { + return t.getEnvDataAndObjects(t.GetNamespaceName()) +} + +func (t *CommonEnvTypeService) getEnvDataAndObjects(namespaceName string) (*EnvAndObjectsManager, error) { + envData, objects, err := getEnvDataAndObjects(t.envService, t.envType, *t.context) + envAndObjectsManager := NewEnvAndObjectsManager(envData, objects, + func(version string) (*environment.EnvData, environment.Objects, error) { + return getEnvDataAndObjects(environment.NewServiceForBlob(version), t.envType, *t.context) + }, t.envType, namespaceName, + ) + return envAndObjectsManager, err +} + +func getEnvDataAndObjects(envService *environment.Service, envType environment.Type, context ServiceContext) (*environment.EnvData, environment.Objects, error) { + envData, err := envService.GetEnvData(context.requestCtx, envType) if err != nil { return nil, nil, err } - objects, err := t.getObjects(envData, filter) + objects, err := getObjects(envData, envType, context) return envData, objects, err } -func (t *CommonEnvTypeService) getObjects(env *environment.EnvData, filter FilterFunc) (environment.Objects, error) { +func getObjects(env *environment.EnvData, envType environment.Type, context ServiceContext) (environment.Objects, error) { var objs environment.Objects - cluster := t.context.clusterForType(t.name) - vars := environment.CollectVars(t.context.openShiftUsername, t.context.nsBaseName, cluster.User, t.context.config) + cluster := context.clusterForType(envType) + vars := environment.CollectVars(context.openShiftUsername, context.nsBaseName, cluster.User, context.config) for _, template := range env.Templates { if os.Getenv("DISABLE_OSO_QUOTAS") == "true" && strings.Contains(template.Filename, "quotas") { @@ -78,17 +92,13 @@ func (t *CommonEnvTypeService) getObjects(env *environment.EnvData, filter Filte if err != nil { return nil, err } - for _, obj := range objects { - if filter(obj) { - objs = append(objs, obj) - } - } + objs = append(objs, objects...) } return objs, nil } func (t *CommonEnvTypeService) GetCluster() cluster.Cluster { - return t.context.clusterForType(t.name) + return t.context.clusterForType(t.envType) } func (t *CommonEnvTypeService) AfterCallback(client *Client, action string) error { @@ -154,6 +164,10 @@ func (t *UserNamespaceTypeService) GetNamespaceName() string { return t.context.nsBaseName } +func (t *UserNamespaceTypeService) GetEnvDataAndObjects() (*EnvAndObjectsManager, error) { + return t.getEnvDataAndObjects(t.GetNamespaceName()) +} + func CreateAdminRoleBinding(namespace string) environment.Object { objs, err := environment.ParseObjects(adminRole) if err == nil { @@ -165,3 +179,46 @@ func CreateAdminRoleBinding(namespace string) environment.Object { } return environment.Object{} } + +type EnvAndObjectsManager struct { + EnvData *environment.EnvData + allObjects environment.Objects + objectsForVersion objectsForVersionRetrieval + envType environment.Type + namespaceName string +} + +func NewEnvAndObjectsManager(envData *environment.EnvData, allObjects environment.Objects, + objectsForVersion objectsForVersionRetrieval, envType environment.Type, namespaceName string) *EnvAndObjectsManager { + return &EnvAndObjectsManager{ + EnvData: envData, + allObjects: allObjects, + objectsForVersion: objectsForVersion, + envType: envType, + namespaceName: namespaceName, + } +} + +func (m *EnvAndObjectsManager) GetMissingObjectsComparingWith(version string) (environment.Objects, error) { + removedObjects, err := CacheOfRemovedObjects.GetResolver(m.envType, version, m.objectsForVersion, m.allObjects).Resolve() + if err != nil { + CacheOfRemovedObjects.RemoveResolver(m.envType, version) + return nil, err + } + var objs environment.Objects + for _, removedObj := range removedObjects { + objs = append(objs, removedObj.toObject(m.namespaceName)) + } + + return objs, nil +} + +func (m *EnvAndObjectsManager) GetObjects(filterFunc FilterFunc) environment.Objects { + var filtered environment.Objects + for _, obj := range m.allObjects { + if filterFunc(obj) { + filtered = append(filtered, obj) + } + } + return filtered +} diff --git a/openshift/types_test.go b/openshift/types_test.go index b5f54e5..5814a42 100644 --- a/openshift/types_test.go +++ b/openshift/types_test.go @@ -6,14 +6,35 @@ import ( "github.com/fabric8-services/fabric8-tenant/cluster" "github.com/fabric8-services/fabric8-tenant/environment" "github.com/fabric8-services/fabric8-tenant/openshift" + "github.com/fabric8-services/fabric8-tenant/tenant" "github.com/fabric8-services/fabric8-tenant/test" "github.com/fabric8-services/fabric8-tenant/test/doubles" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gopkg.in/h2non/gock.v1" + "io/ioutil" "strings" "testing" ) +var prObjTemplate = ` +- apiVersion: v1 + kind: RoleBinding + metadata: + labels: + app: fabric8-tenant + provider: fabric8 + version: 123abc + version-quotas: 123abc + name: view-rb-to-remove + namespace: %s + roleRef: + name: view + subjects: + - kind: ServiceAccount + name: cd-bot +` + func TestEnvironmentTypeService(t *testing.T) { // given config, reset := test.LoadTestConfig(t) @@ -43,13 +64,13 @@ func TestEnvironmentTypeService(t *testing.T) { // then assert.Equal(t, envType, service.GetType()) assert.Equal(t, fmt.Sprintf("http://starter-for-type-%s.com", envType.String()), service.GetCluster().APIURL) - envData, objects, err := service.GetEnvDataAndObjects(func(objects environment.Object) bool { - return true - }) + envDataMngr, err := service.GetEnvDataAndObjects() assert.NoError(t, err) - assert.Equal(t, envType, envData.EnvType) - assert.NotEmpty(t, envData.Templates) - assert.NotEmpty(t, objects) + assert.Equal(t, envType, envDataMngr.EnvData.EnvType) + assert.NotEmpty(t, envDataMngr.EnvData.Templates) + assert.NotEmpty(t, envDataMngr.GetObjects(func(objects environment.Object) bool { + return true + })) if envType != environment.TypeChe { object, add := service.AdditionalObject() @@ -111,6 +132,122 @@ func TestEnvironmentTypeService(t *testing.T) { assert.NoError(t, err) }) }) + + t.Run("EnvAndObjectManager should return missing object", func(t *testing.T) { + defer gock.OffAll() + mockCallsToGetTemplates(t, "123abc", "developer1", true) + for envType := range environment.RetrieveMappedTemplates() { + service := openshift.NewEnvironmentTypeService(envType, ctx, envService) + + // when + envAndObjectsManager, err := service.GetEnvDataAndObjects() + require.NoError(t, err) + objectsToDelete, err := envAndObjectsManager.GetMissingObjectsComparingWith("123abc") + + // then + require.NoError(t, err) + assert.Len(t, objectsToDelete, 1) + assert.Equal(t, environment.ValKindRoleBinding, environment.GetKind(objectsToDelete[0])) + assert.Equal(t, "view-rb-to-remove", environment.GetName(objectsToDelete[0])) + assert.Equal(t, tenant.ConstructNamespaceName(envType, "developer1"), environment.GetNamespace(objectsToDelete[0])) + } + }) + + t.Run("EnvAndObjectManager should not return any object", func(t *testing.T) { + defer gock.OffAll() + mockCallsToGetTemplates(t, "1234abc", "developer1", false) + for envType := range environment.RetrieveMappedTemplates() { + // given + service := openshift.NewEnvironmentTypeService(envType, ctx, envService) + + // when + envAndObjectsManager, err := service.GetEnvDataAndObjects() + require.NoError(t, err) + objectsToDelete, err := envAndObjectsManager.GetMissingObjectsComparingWith("1234abc") + + // then + require.NoError(t, err) + assert.Len(t, objectsToDelete, 0) + } + }) + + t.Run("EnvAndObjectManager should return error when there is a failure during template download", func(t *testing.T) { + defer gock.OffAll() + for envType, tmpls := range environment.RetrieveMappedTemplates() { + // given + for _, tmpl := range tmpls { + gock.New("https://raw.githubusercontent.com"). + Get("/fabric8-services/fabric8-tenant/12345abc/environment/templates/" + tmpl.Filename). + Reply(404) + } + service := openshift.NewEnvironmentTypeService(envType, ctx, envService) + + // when + envAndObjectsManager, err := service.GetEnvDataAndObjects() + require.NoError(t, err) + _, err = envAndObjectsManager.GetMissingObjectsComparingWith("12345abc") + + // then + test.AssertError(t, err, test.HasMessage("server responded with error 404")) + } + }) + + t.Run("EnvAndObjectManager should not return error when there is a failure for the first download and not for the second one", func(t *testing.T) { + defer gock.OffAll() + // given + for envType, tmpls := range environment.RetrieveMappedTemplates() { + for _, tmpl := range tmpls { + gock.New("https://raw.githubusercontent.com"). + Get("/fabric8-services/fabric8-tenant/123456abc/environment/templates/" + tmpl.Filename). + Reply(404) + } + service := openshift.NewEnvironmentTypeService(envType, ctx, envService) + envAndObjectsManager, err := service.GetEnvDataAndObjects() + require.NoError(t, err) + _, err = envAndObjectsManager.GetMissingObjectsComparingWith("123456abc") + test.AssertError(t, err, test.HasMessage("server responded with error 404")) + } + gock.OffAll() + mockCallsToGetTemplates(t, "123456abc", "developer1", true) + + for envType := range environment.RetrieveMappedTemplates() { + service := openshift.NewEnvironmentTypeService(envType, ctx, envService) + + // when + envAndObjectsManager, err := service.GetEnvDataAndObjects() + require.NoError(t, err) + objectsToDelete, err := envAndObjectsManager.GetMissingObjectsComparingWith("123456abc") + + // then + require.NoError(t, err, envType.String()) + assert.Len(t, objectsToDelete, 1) + assert.Equal(t, environment.ValKindRoleBinding, environment.GetKind(objectsToDelete[0])) + assert.Equal(t, "view-rb-to-remove", environment.GetName(objectsToDelete[0])) + assert.Equal(t, tenant.ConstructNamespaceName(envType, "developer1"), environment.GetNamespace(objectsToDelete[0])) + } + }) +} + +func mockCallsToGetTemplates(t *testing.T, version, nsBaseName string, addRb bool) { + for envType, tmpls := range environment.RetrieveMappedTemplates() { + namespaceName := tenant.ConstructNamespaceName(envType, nsBaseName) + rbToRemove := fmt.Sprintf(prObjTemplate, namespaceName) + for _, tmpl := range tmpls { + // given + bytes, err := ioutil.ReadFile("../environment/templates/" + tmpl.Filename) + require.NoError(t, err) + body := string(bytes) + if addRb && !strings.Contains(tmpl.Filename, "quotas") { + body = strings.Replace(body, "parameters:", rbToRemove+"\nparameters:", 1) + } + + path := fmt.Sprintf("/fabric8-services/fabric8-tenant/%s/environment/templates/%s", version, tmpl.Filename) + gock.New("https://raw.githubusercontent.com"). + Get(path). + Reply(200). + BodyString(body) + } + } } func TestPresenceOfTemplateObjects(t *testing.T) { diff --git a/openshift/types_whitebox_test.go b/openshift/types_whitebox_test.go index 06628b5..08c91e1 100644 --- a/openshift/types_whitebox_test.go +++ b/openshift/types_whitebox_test.go @@ -34,7 +34,7 @@ func TestAdditionalObjectForChe(t *testing.T) { service := &CheNamespaceTypeService{ CommonEnvTypeService: &CommonEnvTypeService{ - name: environment.TypeChe, + envType: environment.TypeChe, context: ctx, envService: environment.NewService(), }, diff --git a/test/doubles/test_doubles.go b/test/doubles/test_doubles.go index 8444925..c780dd5 100644 --- a/test/doubles/test_doubles.go +++ b/test/doubles/test_doubles.go @@ -153,10 +153,11 @@ func SingleTemplatesObjects(t *testing.T, config *configuration.Data, envType en }) nsTypeService := openshift.NewEnvironmentTypeService(envType, ctx, envService) - _, objects, err := nsTypeService.GetEnvDataAndObjects(func(objects environment.Object) bool { + envAndObjsMngr, err := nsTypeService.GetEnvDataAndObjects() + require.NoError(t, err) + objects := envAndObjsMngr.GetObjects(func(objects environment.Object) bool { return true }) - require.NoError(t, err) return objects }