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

Commit

Permalink
Add parameter to Delete Space to skip deleting OpenShift resources (#…
Browse files Browse the repository at this point in the history 
…2121)

When a user resets their environment, the front-end makes calls to the Delete Space API and Clean Tenant API. It makes these calls asynchronously, and due to both APIs acting on the same resources, I suspect this is the reason we are seeing a variety of errors in openshiftio/openshift.io#3500.

Since the Clean Tenant API cleans out the user's entire namespaces, it is not necessary in this case for Delete Space to delete anything from OpenShift. This PR adds an optional parameter skipCluster, to the Delete Space API, which if true, will not attempt to delete any deployments from OpenShift. The front-end could then use this parameter only when resetting the user's environment. An alternative would be for the front-end to synchronize between deleting spaces and calling Clean Tenant, but this would be less efficient.

Fixes (partially): openshiftio/openshift.io#3500
  • Loading branch information
@ebaron
ebaron committed Jul 11, 2018
1 parent cefe36a commit f13e094
Show file tree
Hide file tree
Showing 7 changed files with 490 additions and 15 deletions.
20 changes: 11 additions & 9 deletions controller/space.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,17 @@ func (c *SpaceController) Delete(ctx *app.DeleteSpaceContext) error {
}

// now delete the OpenShift resources associated with this space on an
// OpenShift cluster
err = deleteOpenShiftResource(c.DeploymentsClient, config, ctx.Context, spaceID)
if err != nil {
log.Error(ctx, map[string]interface{}{
"space_id": spaceID,
"error": err,
}, "could not delete OpenShift resources")
return jsonapi.JSONErrorResponse(
ctx, errors.NewInternalError(ctx, err))
// OpenShift cluster, unless otherwise specified
if ctx.SkipCluster == nil || !*ctx.SkipCluster {
err = deleteOpenShiftResource(c.DeploymentsClient, config, ctx.Context, spaceID)
if err != nil {
log.Error(ctx, map[string]interface{}{
"space_id": spaceID,
"error": err,
}, "could not delete OpenShift resources")
return jsonapi.JSONErrorResponse(
ctx, errors.NewInternalError(ctx, err))
}
}

err = application.Transactional(c.db, func(appl application.Application) error {
Expand Down
106 changes: 100 additions & 6 deletions controller/space_blackbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/dnaeon/go-vcr/cassette"
"github.com/dnaeon/go-vcr/recorder"
"github.com/fabric8-services/fabric8-wit/account"
"github.com/fabric8-services/fabric8-wit/app"
Expand Down Expand Up @@ -404,9 +405,18 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
)
spaceID := fxt.Spaces[0].ID
identity := *fxt.Identities[0]
expectedDeleteURLs := map[string]struct{}{
"http://core/api/deployments/spaces/aec5f659-0680-4633-8599-5f14f1deeabc/applications/testspace1/deployments/stage": {},
"http://core/api/deployments/spaces/aec5f659-0680-4633-8599-5f14f1deeabc/applications/testspace1/deployments/run": {},
"http://core/api/deployments/spaces/aec5f659-0680-4633-8599-5f14f1deeabc/applications/testspace2/deployments/stage": {},
"http://core/api/deployments/spaces/aec5f659-0680-4633-8599-5f14f1deeabc/applications/testspace2/deployments/run": {},
}

rDeployments, err := recorder.New("../test/data/deployments/deployments_delete_space.ok")
require.NoError(t, err)
rDeployments.SetMatcher(func(httpReq *http.Request, vcrReq cassette.Request) bool {
return checkDeleteURL(httpReq, expectedDeleteURLs, t)
})
defer rDeployments.Stop()

rCodebase, err := recorder.New("../test/data/codebases/codebases_delete_space.ok")
Expand All @@ -418,7 +428,80 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceOK(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceOK(t, svc.Context, svc, ctrl, spaceID, nil)

require.Emptyf(t, expectedDeleteURLs, "Expected DELETE request(s) not made: %v", expectedDeleteURLs)
})

s.T().Run("skip cluster - ok", func(t *testing.T) {
var err error
fxt := tf.NewTestFixture(t, s.DB,
tf.Spaces(1, func(f *tf.TestFixture, index int) error {
f.Spaces[index].ID, err = uuid.FromString("2a75fae9-3131-4dc5-8b43-283aeb2522b6")
require.NoError(t, err)
return nil
}),
tf.Codebases(1),
)
spaceID := fxt.Spaces[0].ID
identity := *fxt.Identities[0]
skipCluster := true

rDeployments, err := recorder.New("../test/data/deployments/deployments_delete_space.ok-skip-cluster")
require.NoError(t, err)
defer rDeployments.Stop()

rCodebase, err := recorder.New("../test/data/codebases/codebases_delete_space.ok-skip-cluster")
require.NoError(t, err)
defer rCodebase.Stop()

svc, ctrl := s.SecuredController(
identity,
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceOK(t, svc.Context, svc, ctrl, spaceID, &skipCluster)
})

s.T().Run("skip cluster - false", func(t *testing.T) {
var err error
fxt := tf.NewTestFixture(t, s.DB,
tf.Spaces(1, func(f *tf.TestFixture, index int) error {
f.Spaces[index].ID, err = uuid.FromString("4d19e0fb-b558-4160-8768-f41cb8169e95")
require.NoError(t, err)
return nil
}),
tf.Codebases(1),
)
spaceID := fxt.Spaces[0].ID
identity := *fxt.Identities[0]
skipCluster := false
expectedDeleteURLs := map[string]struct{}{
"http://core/api/deployments/spaces/4d19e0fb-b558-4160-8768-f41cb8169e95/applications/testspace1/deployments/stage": {},
"http://core/api/deployments/spaces/4d19e0fb-b558-4160-8768-f41cb8169e95/applications/testspace1/deployments/run": {},
"http://core/api/deployments/spaces/4d19e0fb-b558-4160-8768-f41cb8169e95/applications/testspace2/deployments/stage": {},
"http://core/api/deployments/spaces/4d19e0fb-b558-4160-8768-f41cb8169e95/applications/testspace2/deployments/run": {},
}

rDeployments, err := recorder.New("../test/data/deployments/deployments_delete_space.ok-skip-cluster-false")
require.NoError(t, err)
rDeployments.SetMatcher(func(httpReq *http.Request, vcrReq cassette.Request) bool {
return checkDeleteURL(httpReq, expectedDeleteURLs, t)
})
defer rDeployments.Stop()

rCodebase, err := recorder.New("../test/data/codebases/codebases_delete_space.ok-skip-cluster-false")
require.NoError(t, err)
defer rCodebase.Stop()

svc, ctrl := s.SecuredController(
identity,
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceOK(t, svc.Context, svc, ctrl, spaceID, &skipCluster)

require.Emptyf(t, expectedDeleteURLs, "Expected DELETE request(s) not made: %v", expectedDeleteURLs)
})

s.T().Run("delete space - auth returns 401 Unauthorized", func(t *testing.T) {
Expand Down Expand Up @@ -450,7 +533,7 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceUnauthorized(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceUnauthorized(t, svc.Context, svc, ctrl, spaceID, nil)
})

s.T().Run("delete space - auth returns 403 Forbidden", func(t *testing.T) {
Expand Down Expand Up @@ -482,7 +565,7 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceForbidden(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceForbidden(t, svc.Context, svc, ctrl, spaceID, nil)
})

s.T().Run("delete space - auth returns 404 Not Found", func(t *testing.T) {
Expand Down Expand Up @@ -514,7 +597,7 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceNotFound(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceNotFound(t, svc.Context, svc, ctrl, spaceID, nil)
})

s.T().Run("delete space - auth returns 500 Internal Server Error", func(t *testing.T) {
Expand Down Expand Up @@ -546,7 +629,7 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceInternalServerError(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceInternalServerError(t, svc.Context, svc, ctrl, spaceID, nil)
})

s.T().Run("fail - different owner", func(t *testing.T) {
Expand Down Expand Up @@ -575,10 +658,21 @@ func (s *SpaceControllerTestSuite) TestDeleteSpace() {
withDeploymentsClient(&http.Client{Transport: rDeployments.Transport}),
withCodebaseClient(&http.Client{Transport: rCodebase.Transport}),
)
test.DeleteSpaceForbidden(t, svc.Context, svc, ctrl, spaceID)
test.DeleteSpaceForbidden(t, svc.Context, svc, ctrl, spaceID, nil)
})
}

func checkDeleteURL(httpReq *http.Request, expectedDeleteURLs map[string]struct{}, t *testing.T) bool {
// Track delete requests made by this test
if httpReq.Method == http.MethodDelete {
reqURL := httpReq.URL.String()
_, pres := expectedDeleteURLs[reqURL]
require.True(t, pres, "Unexpected DELETE request %s", reqURL)
delete(expectedDeleteURLs, reqURL)
}
return true
}

func (s *SpaceControllerTestSuite) TestUpdateSpace() {

s.T().Run("ok", func(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions design/spaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ var _ = a.Resource("space", func() {
a.Description("Delete a space with the given ID.")
a.Params(func() {
a.Param("spaceID", d.UUID, "ID of the space to delete")
a.Param("skipCluster", d.Boolean,
"If true, skip deleting OpenShift objects belonging to the space")
})
a.Response(d.OK)
a.Response(d.BadRequest, JSONAPIErrors)
Expand Down
104 changes: 104 additions & 0 deletions test/data/codebases/codebases_delete_space.ok-skip-cluster-false.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
---
version: 1
interactions:
- request:
body: ""
form: {}
url: http://core/api/spaces/4d19e0fb-b558-4160-8768-f41cb8169e95/codebases
method: GET
response:
body: '{
"data": [
{
"attributes": {
"createdAt": "2018-04-25T09:19:06.547996Z",
"last_used_workspace": "",
"type": "git",
"url": "https://github.com/surajssd/byebyeworld"
},
"id": "f3f45151-3dab-4103-baee-bfc0d2a2cbba",
"links": {
"edit": "",
"related": "",
"self": ""
},
"relationships": {
"space": {
"data": {
"id": "4d19e0fb-b558-4160-8768-f41cb8169e95",
"type": "spaces"
},
"links": {
"related": "",
"self": ""
}
},
"workspaces": {
"links": {
"related": "",
"self": ""
}
}
},
"type": "codebases"
},
{
"attributes": {
"createdAt": "2018-04-25T10:15:19.548216Z",
"last_used_workspace": "",
"type": "git",
"url": "https://github.com/surajssd/heyheyworld"
},
"id": "aed02e9b-1b51-4fa8-98c9-06ef56a28c88",
"links": {
"edit": "",
"related": "",
"self": ""
},
"relationships": {
"space": {
"data": {
"id": "4d19e0fb-b558-4160-8768-f41cb8169e95",
"type": "spaces"
},
"links": {
"related": "",
"self": ""
}
},
"workspaces": {
"links": {
"related": "",
"self": ""
}
}
},
"type": "codebases"
}
],
"links": {
"first": "",
"last": ""
},
"meta": {
"totalCount": 2
}
}'
# headers:

status: 200 OK
code: 200
- request:
url: http://core/api/codebases/f3f45151-3dab-4103-baee-bfc0d2a2cbba
method: DELETE
response:
# headers:
status: 200 OK
code: 204
- request:
url: http://core/api/codebases/aed02e9b-1b51-4fa8-98c9-06ef56a28c88
method: DELETE
response:
# headers:
status: 200 OK
code: 204

0 comments on commit f13e094

Please sign in to comment.