Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
fix(#710): when creation/update of a namespace fails,create new ones …
Browse files Browse the repository at this point in the history 
…with new base-name (#714)
  • Loading branch information
@MatousJobanek
MatousJobanek authored Jan 2, 2019
1 parent de06a79 commit 33f5f0f
Show file tree
Hide file tree
Showing 16 changed files with 409 additions and 274 deletions.
2 changes: 1 addition & 1 deletion .make/test.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ ALL_PKGS_EXCLUDE_PATTERN = 'vendor\|app\|tool\/cli\|design\|client\|test'
GOANALYSIS_PKGS_EXCLUDE_PATTERN="vendor|app|client|tool/cli"
GOANALYSIS_DIRS=$(shell go list -f {{.Dir}} ./... | grep -v -E $(GOANALYSIS_PKGS_EXCLUDE_PATTERN))

TEST_FLAGS?=-v -p 1 -vet off
TEST_FLAGS ?= -v -p 1 -vet off

MINISHIFT_URL ?= https://$(shell minishift ip):8443
MINISHIFT_USER_NAME ?= tenant.minishift.test.$(shell date +'%H.%M.%S')
Expand Down
180 changes: 11 additions & 169 deletions controller/tenant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ package controller
import (
"context"
"fmt"
"net/http"
"sync/atomic"
"time"

"github.com/fabric8-services/fabric8-common/log"
"github.com/fabric8-services/fabric8-tenant/app"
"github.com/fabric8-services/fabric8-tenant/auth"
Expand All @@ -22,7 +18,6 @@ import (
"github.com/goadesign/goa"
goajwt "github.com/goadesign/goa/middleware/security/jwt"
errs "github.com/pkg/errors"
"gopkg.in/yaml.v2"
)

// TenantController implements the status resource.
Expand Down Expand Up @@ -112,13 +107,7 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error {
go func() {
ctx := ctx
t := tenant
err = openshift.RawInitTenant(
ctx,
openshiftConfig,
InitTenant(ctx, openshiftConfig.MasterURL, c.tenantService, t),
user.OpenShiftUsername,
nsBaseName,
user.OpenShiftUserToken)
_, err = openshift.RawInitTenant(ctx, openshiftConfig, t, user.OpenShiftUserToken, c.tenantService, true)

if err != nil {
sentry.LogError(ctx, map[string]interface{}{
Expand Down Expand Up @@ -178,7 +167,7 @@ func (c *TenantController) Update(ctx *app.UpdateTenantContext) error {
return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, fmt.Errorf("unable to update tenant configuration: %v", err)))
}

err = UpdateTenant(&TenantUpdater{}, ctx, c.tenantService, openshiftConfig, tenant, env.DefaultEnvTypes...)
err = openshift.UpdateTenant(&TenantUpdater{}, ctx, c.tenantService, openshiftConfig, tenant, user.OpenShiftUserToken, true, env.DefaultEnvTypes...)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
Expand All @@ -192,59 +181,13 @@ func (c *TenantController) Update(ctx *app.UpdateTenantContext) error {
return ctx.Accepted()
}

func UpdateTenant(updateExecutor UpdateExecutor, ctx context.Context, tenantService tenant.Service, openshiftConfig openshift.Config, t *tenant.Tenant, envTypes ...env.Type) error {
versionMapping, err := updateExecutor.Update(ctx, tenantService, openshiftConfig, t, envTypes)
if err != nil {
er := updateNamespaceEntities(tenantService, t, versionMapping, true)
if er != nil {
return fmt.Errorf("there occured two errors when doing update: \n1.[%s]\n2.[%s]", err, er)
}
return err
}

return updateNamespaceEntities(tenantService, t, versionMapping, false)
}

func updateNamespaceEntities(tenantService tenant.Service, t *tenant.Tenant, versionMapping map[env.Type]string, failed bool) error {
namespaces, err := tenantService.GetNamespaces(t.ID)
if err != nil {
return errs.Wrapf(err, "unable to get tenant namespaces")
}
var found bool
var nsVersion string
for _, ns := range namespaces {
if nsVersion, found = versionMapping[ns.Type]; found {
if failed {
ns.State = tenant.Failed
} else {
ns.State = tenant.Ready
ns.Version = nsVersion
}
ns.UpdatedBy = configuration.Commit
err := tenantService.SaveNamespace(ns)
if err != nil {
return errs.Wrapf(err, "unable to save tenant namespace %+v", ns)
}
}
}
return nil
}

type UpdateExecutor interface {
Update(ctx context.Context, tenantService tenant.Service, openshiftConfig openshift.Config, t *tenant.Tenant, envTypes []env.Type) (map[env.Type]string, error)
}

type TenantUpdater struct {
}

func (u TenantUpdater) Update(ctx context.Context, tenantService tenant.Service, openshiftConfig openshift.Config, t *tenant.Tenant, envTypes []env.Type) (map[env.Type]string, error) {
return openshift.RawUpdateTenant(
ctx,
openshiftConfig,
InitTenant(ctx, openshiftConfig.MasterURL, tenantService, t),
t.OSUsername,
t.NsBaseName,
envTypes)
func (u TenantUpdater) Update(ctx context.Context, tenantService tenant.Service, openshiftConfig openshift.Config, t *tenant.Tenant,
envTypes []env.Type, usertoken string, allowSelfHealing bool) (map[env.Type]string, error) {

return openshift.RawUpdateTenant(ctx, openshiftConfig, t, envTypes, usertoken, tenantService, allowSelfHealing)
}

// Clean runs the setup action for the tenant namespaces.
Expand Down Expand Up @@ -294,7 +237,11 @@ func (c *TenantController) Clean(ctx *app.CleanTenantContext) error {
return jsonapi.JSONErrorResponse(ctx, err)
}
if removeFromCluster {
err = c.tenantService.DeleteAll(ttoken.Subject())
err = c.tenantService.DeleteNamespaces(ttoken.Subject())
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}
err = c.tenantService.DeleteTenant(ttoken.Subject())
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}
Expand Down Expand Up @@ -325,103 +272,6 @@ func (c *TenantController) Show(ctx *app.ShowTenantContext) error {
return ctx.OK(result)
}

// InitTenant is a Callback that assumes a new tenant is being created
func InitTenant(ctx context.Context, masterURL string, service tenant.Service, currentTenant *tenant.Tenant) openshift.Callback {
var maxResourceQuotaStatusCheck int32 = 50 // technically a global retry count across all ResourceQuota on all Tenant Namespaces
var currentResourceQuotaStatusCheck int32 // default is 0
return func(statusCode int, method string, request, response map[interface{}]interface{}, versionMapping map[env.Type]string) (string, map[interface{}]interface{}) {
log.Info(ctx, map[string]interface{}{
"status": statusCode,
"method": method,
"cluster_url": masterURL,
"namespace": env.GetNamespace(request),
"name": env.GetName(request),
"kind": env.GetKind(request),
"request": yamlString(request),
"response": yamlString(response),
}, "resource requested")
if statusCode == http.StatusConflict {
if env.GetKind(request) == env.ValKindNamespace {
return "", nil
}
if env.GetKind(request) == env.ValKindProjectRequest {
return "", nil
}
if env.GetKind(request) == env.ValKindPersistenceVolumeClaim {
return "", nil
}
if env.GetKind(request) == env.ValKindServiceAccount {
return "", nil
}
return "DELETE", request
} else if statusCode == http.StatusCreated {
if env.GetKind(request) == env.ValKindProjectRequest {
name := env.GetName(request)
envType := tenant.GetNamespaceType(name, currentTenant.NsBaseName)
templatesVersion := versionMapping[envType]
service.SaveNamespace(&tenant.Namespace{
TenantID: currentTenant.ID,
Name: name,
State: tenant.Ready,
Version: templatesVersion,
Type: envType,
MasterURL: masterURL,
UpdatedBy: configuration.Commit,
})

// HACK to workaround osio applying some dsaas-user permissions async
// Should loop on a Check if allowed type of call instead
time.Sleep(time.Second * 5)

} else if env.GetKind(request) == env.ValKindNamespace {
name := env.GetName(request)
envType := tenant.GetNamespaceType(name, currentTenant.NsBaseName)
templatesVersion := versionMapping[envType]
service.SaveNamespace(&tenant.Namespace{
TenantID: currentTenant.ID,
Name: name,
State: tenant.Ready,
Version: templatesVersion,
Type: envType,
MasterURL: masterURL,
UpdatedBy: configuration.Commit,
})
} else if env.GetKind(request) == env.ValKindResourceQuota {
// trigger a check status loop
time.Sleep(time.Millisecond * 50)
return "GET", response
}
return "", nil
} else if statusCode == http.StatusOK {
if method == "DELETE" {
return "POST", request
} else if method == "GET" {
if env.GetKind(request) == env.ValKindResourceQuota {

if env.HasValidStatus(response) || atomic.LoadInt32(&currentResourceQuotaStatusCheck) >= maxResourceQuotaStatusCheck {
return "", nil
}
atomic.AddInt32(&currentResourceQuotaStatusCheck, 1)
time.Sleep(time.Millisecond * 50)
return "GET", response
}
}
return "", nil
}
log.Info(ctx, map[string]interface{}{
"status": statusCode,
"method": method,
"namespace": env.GetNamespace(request),
"cluster_url": masterURL,
"name": env.GetName(request),
"kind": env.GetKind(request),
"request": yamlString(request),
"response": yamlString(response),
}, "unhandled resource response")
return "", nil
}
}

func convertTenant(ctx context.Context, tenant *tenant.Tenant, namespaces []*tenant.Namespace, resolveCluster cluster.GetCluster) *app.Tenant {
result := app.Tenant{
ID: &tenant.ID,
Expand Down Expand Up @@ -463,11 +313,3 @@ func convertTenant(ctx context.Context, tenant *tenant.Tenant, namespaces []*ten
}
return &result
}

func yamlString(data map[interface{}]interface{}) string {
b, err := yaml.Marshal(data)
if err != nil {
return fmt.Sprintf("Could not marshal yaml %v", data)
}
return string(b)
}
30 changes: 9 additions & 21 deletions controller/tenants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

commonauth "github.com/fabric8-services/fabric8-common/auth"
"github.com/fabric8-services/fabric8-common/errors"
errs "github.com/fabric8-services/fabric8-common/errors"
"github.com/fabric8-services/fabric8-common/log"
"github.com/fabric8-services/fabric8-tenant/app"
"github.com/fabric8-services/fabric8-tenant/auth"
Expand All @@ -21,7 +22,6 @@ var SERVICE_ACCOUNTS = []string{"fabric8-jenkins-idler", "rh-che"}
type TenantsController struct {
*goa.Controller
tenantService tenant.Service
openshiftService openshift.Service
clusterService cluster.Service
authClientService auth.Service
}
Expand All @@ -31,13 +31,11 @@ func NewTenantsController(service *goa.Service,
tenantService tenant.Service,
clusterService cluster.Service,
authClientService auth.Service,
openshiftService openshift.Service,
) *TenantsController {
return &TenantsController{
Controller: service.NewController("TenantsController"),
tenantService: tenantService,
clusterService: clusterService,
openshiftService: openshiftService,
authClientService: authClientService,
}
}
Expand Down Expand Up @@ -100,38 +98,28 @@ func (c *TenantsController) Delete(ctx *app.DeleteTenantsContext) error {
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}
for _, namespace := range namespaces {
if len(namespaces) > 0 {
// fetch the cluster info
clustr, err := c.clusterService.GetCluster(ctx, namespace.MasterURL)
clustr, err := c.clusterService.GetCluster(ctx, namespaces[0].MasterURL)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
"cluster_url": namespace.MasterURL,
"cluster_url": namespaces[0].MasterURL,
"tenant_id": tenantID,
}, "unable to fetch cluster for user")
return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err))
return jsonapi.JSONErrorResponse(ctx, errs.NewInternalError(ctx, err))
}

openshiftConfig := openshift.Config{
MasterURL: namespace.MasterURL,
MasterURL: clustr.APIURL,
Token: clustr.Token,
}
log.Info(ctx, map[string]interface{}{"tenant_id": tenantID, "namespace": namespace.Name}, "deleting namespace...")
// delete the namespace in the cluster
err = c.openshiftService.DeleteNamespace(ctx, openshiftConfig, namespace.Name)
err = openshift.DeleteNamespaces(ctx, tenantID, openshiftConfig, c.tenantService)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
"cluster_url": namespace.MasterURL,
"namespace": namespace.Name,
"tenant_id": tenantID,
}, "failed to delete namespace")
return jsonapi.JSONErrorResponse(ctx, err)
return err
}
// then delete the corresponding record in the DB
}
// finally, delete the tenant record (all NS were already deleted, but that's fine)
err = c.tenantService.DeleteAll(tenantID)
err = c.tenantService.DeleteTenant(tenantID)
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}
Expand Down
34 changes: 6 additions & 28 deletions controller/tenants_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import (
"github.com/fabric8-services/fabric8-tenant/cluster"
"github.com/fabric8-services/fabric8-tenant/configuration"
"github.com/fabric8-services/fabric8-tenant/controller"
"github.com/fabric8-services/fabric8-tenant/openshift"
"github.com/fabric8-services/fabric8-tenant/environment"
"github.com/fabric8-services/fabric8-tenant/tenant"
"github.com/fabric8-services/fabric8-tenant/test"
"github.com/fabric8-services/fabric8-tenant/test/doubles"
"github.com/fabric8-services/fabric8-tenant/test/gormsupport"
"github.com/fabric8-services/fabric8-tenant/test/recorder"
"github.com/fabric8-services/fabric8-tenant/test/testfixture"
tf "github.com/fabric8-services/fabric8-tenant/test/testfixture"
"github.com/goadesign/goa"
goajwt "github.com/goadesign/goa/middleware/security/jwt"
"github.com/satori/go.uuid"
Expand Down Expand Up @@ -56,7 +56,7 @@ func (s *TenantsControllerTestSuite) TestShowTenants() {

s.T().Run("OK", func(t *testing.T) {
// given
fxt := testfixture.NewTestFixture(t, s.DB, testfixture.Tenants(1), testfixture.Namespaces(1))
fxt := tf.NewTestFixture(t, s.DB, tf.Tenants(1), tf.Namespaces(1))
// when
_, tenant := goatest.ShowTenantsOK(t, createValidSAContext("fabric8-jenkins-idler"), svc, ctrl, fxt.Tenants[0].ID)
// then
Expand Down Expand Up @@ -98,7 +98,7 @@ func (s *TenantsControllerTestSuite) TestSearchTenants() {

s.T().Run("OK", func(t *testing.T) {
// given
fxt := testfixture.NewTestFixture(t, s.DB, testfixture.Tenants(1), testfixture.Namespaces(1))
fxt := tf.NewTestFixture(t, s.DB, tf.Tenants(1), tf.Namespaces(1))
// when
_, tenant := goatest.SearchTenantsOK(t, createValidSAContext("fabric8-jenkins-idler"), svc, ctrl, fxt.Namespaces[0].MasterURL, fxt.Namespaces[0].Name)
// then
Expand Down Expand Up @@ -181,28 +181,7 @@ func (s *TenantsControllerTestSuite) TestFailedDeleteTenants() {

svc, ctrl, reset := s.newTestTenantsController()
defer reset()
fxt := testfixture.NewTestFixture(t, s.DB, testfixture.Tenants(1, func(fxt *testfixture.TestFixture, idx int) error {
id, err := uuid.FromString("5a95c51b-120a-4d03-b529-98bd7d4a5689") // force the ID to match the go-vcr cassette in the `delete-tenants.yaml` file
if err != nil {
return err
}
fxt.Tenants[0].ID = id
fxt.Tenants[0].OSUsername = "baz"
fxt.Tenants[0].NsBaseName = "baz"
return nil
}), testfixture.Namespaces(2, func(fxt *testfixture.TestFixture, idx int) error {
fxt.Namespaces[idx].TenantID = fxt.Tenants[0].ID
fxt.Namespaces[idx].MasterURL = "https://api.cluster1"
if idx == 0 {
fxt.Namespaces[idx].Name = "baz"
fxt.Namespaces[idx].Type = "user"
} else if idx == 1 {
fxt.Namespaces[idx].TenantID = fxt.Tenants[0].ID
fxt.Namespaces[idx].Name = "baz-che"
fxt.Namespaces[idx].Type = "che"
}
return nil
}))
fxt := tf.FillDB(t, s.DB, tf.AddTenantsNamed("baz"), true, tf.AddNamespaces(environment.TypeUser, environment.TypeChe).State(tenant.Ready))

// when
goatest.DeleteTenantsInternalServerError(t, createValidSAContext("fabric8-auth"), svc, ctrl, fxt.Tenants[0].ID)
Expand Down Expand Up @@ -266,8 +245,7 @@ func prepareConfigClusterAndAuthService(t *testing.T) (cluster.Service, auth.Ser
}
func (s *TenantsControllerTestSuite) newTestTenantsController() (*goa.Service, *controller.TenantsController, func()) {
clusterService, authService, _, reset := prepareConfigClusterAndAuthService(s.T())
openshiftService := openshift.NewService()
svc := goa.New("Tenants-service")
ctrl := controller.NewTenantsController(svc, tenant.NewDBService(s.DB), clusterService, authService, openshiftService)
ctrl := controller.NewTenantsController(svc, tenant.NewDBService(s.DB), clusterService, authService)
return svc, ctrl, reset
}
Loading

0 comments on commit 33f5f0f

Please sign in to comment.