Skip to content

[BUG] Fix undeploy router error #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deadlycoconuts merged 14 commits into from
Apr 22, 2022
Merged

[BUG] Fix undeploy router error #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
57476b9
Fix bugs in sample SDK script
Apr 21, 2022
4ae558a
Refactor UndeployRouterVersion to take in cleanup flag
Apr 21, 2022
8752cb5
Update DeploymentService mock class
Apr 21, 2022
758bc87
Fix bug in deployment controller test
Apr 21, 2022
513b0e7
Refactor if else blocks
Apr 21, 2022
9529560
Add tests for IsKnativeServiceInNamespace and IsSecretinNamespace
Apr 21, 2022
23e8eda
Rename test method to make it consistent with the method tested
Apr 21, 2022
eceafaf
Refactor k8s deletion methods into separate methods
Apr 21, 2022
79c97b9
Fix bug in deleting deployments and services
Apr 21, 2022
cff134e
Fix typo in router name
Apr 22, 2022
9604e44
Rename default route from nothing to control
Apr 22, 2022
b6e2901
Make undeploying a pending router status a cleanup job
Apr 22, 2022
879d140
Refactor code to use ignoreNotFound flag
Apr 22, 2022
4d5df83
Fix go mod file
Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions api/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ github.com/aws/aws-sdk-go v1.27.1/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN
github.com/aws/aws-sdk-go v1.28.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
github.com/aws/aws-sdk-go v1.29.11/go.mod h1:1KvfttTE3SPKMpo8g2c6jL3ZKfXtFvKscTgahTma5Xg=
github.com/aws/aws-sdk-go v1.29.32/go.mod h1:1KvfttTE3SPKMpo8g2c6jL3ZKfXtFvKscTgahTma5Xg=
github.com/aws/aws-sdk-go v1.29.34 h1:yrzwfDaZFe9oT4AmQeNNunSQA7c0m2chz0B43+bJ1ok=
github.com/aws/aws-sdk-go v1.29.34/go.mod h1:1KvfttTE3SPKMpo8g2c6jL3ZKfXtFvKscTgahTma5Xg=
github.com/aws/aws-sdk-go v1.43.44 h1:t+97cY4ScE/czlNlK5iikUGi7w1fC0uop1OUalDIRT4=
github.com/aws/aws-sdk-go v1.43.44/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo=
github.com/aws/aws-xray-sdk-go v1.0.0-rc.5/go.mod h1:XtMKdBQfpVut+tJEwI7+dJFRxxRdxHDyVNp2tHXRq04=
github.com/bazelbuild/bazel-gazelle v0.0.0-20181012220611-c728ce9f663e/go.mod h1:uHBSeeATKpVazAACZBDPL/Nk/UhQDDsJWDlqYJo8/Us=
github.com/bazelbuild/buildtools v0.0.0-20180226164855-80c7f0d45d7e/go.mod h1:5JP0TXzWDHXv8qvxRC4InIazwdyDseBDbzESUMKk1yU=
Expand Down Expand Up @@ -860,8 +861,11 @@ github.com/jinzhu/now v1.1.1/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/
github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/jmespath/go-jmespath v0.0.0-20160803190731-bd40a432e4c7/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
github.com/jmespath/go-jmespath v0.3.0 h1:OS12ieG61fsCg5+qLJ+SsW9NicxNkg3b25OyT2yCeUc=
github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik=
github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=
github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks=
github.com/joefitzgerald/rainbow-reporter v0.1.0/go.mod h1:481CNgqmVHQZzdIbN52CupLJyoVwB10FQ/IQlF1pdL8=
github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqxOKXbg=
Expand Down Expand Up @@ -1580,6 +1584,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f h1:OfiFi4JbukWwe3lzw+xunroH1mnC1e2Gy5cxNJApiSY=
golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd h1:O7DYs+zxREGLKzKoMQrtrEacpb0ZVXA5rIwylE2Xchk=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/oauth2 v0.0.0-20180603041954-1e0a3fa8ba9a/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20180724155351-3d292e4d0cdc/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
Expand Down Expand Up @@ -1691,10 +1697,13 @@ golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220209214540-3681064d5158 h1:rm+CHSpPEEW2IsXUib1ThaHIjuBVZjxNgSKmBLFfD4c=
golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
11 changes: 8 additions & 3 deletions api/turing/api/deployment_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (c RouterDeploymentController) deployOrRollbackRouter(
// Save the error from the failed deployment
errorStrings = append(errorStrings, err.Error())
// Remove cluster resources from the failed deployment attempt
err = c.DeploymentService.UndeployRouterVersion(project, environment, routerVersion, eventsCh)
err = c.DeploymentService.UndeployRouterVersion(project, environment, routerVersion, eventsCh, true)
if err != nil {
errorStrings = append(errorStrings, err.Error())
eventsCh.Write(models.NewErrorEvent(
Expand Down Expand Up @@ -102,7 +102,7 @@ func (c RouterDeploymentController) deployOrRollbackRouter(
if err != nil {
errorStrings = append(errorStrings, err.Error())
} else {
err = c.DeploymentService.UndeployRouterVersion(project, environment, currVersion, eventsCh)
err = c.DeploymentService.UndeployRouterVersion(project, environment, currVersion, eventsCh, false)
if err != nil {
errorStrings = append(errorStrings, err.Error())
}
Expand Down Expand Up @@ -313,7 +313,12 @@ func (c RouterDeploymentController) undeployRouter(
if routerVersion.Status == models.RouterVersionStatusPending ||
routerVersion.Status == models.RouterVersionStatusDeployed {
// Remove cluster resources
err = c.DeploymentService.UndeployRouterVersion(project, environment, routerVersion, eventsCh)
if routerVersion.Status == models.RouterVersionStatusPending {
err = c.DeploymentService.UndeployRouterVersion(project, environment, routerVersion, eventsCh, true)
} else if routerVersion.Status == models.RouterVersionStatusDeployed {
err = c.DeploymentService.UndeployRouterVersion(project, environment, routerVersion, eventsCh, false)
}

if err != nil {
errorStrings = append(errorStrings, err.Error())
}
Expand Down
12 changes: 6 additions & 6 deletions api/turing/api/deployment_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func TestRollbackVersionSuccess(t *testing.T) {
ds := &mocks.DeploymentService{}
ds.On("DeployRouterVersion", project, environment, newVer, testSvcAcct,
"", "", mock.Anything, json.RawMessage(nil), mock.Anything).Return("", errors.New("error"))
ds.On("UndeployRouterVersion", project, environment, newVer, mock.Anything).
ds.On("UndeployRouterVersion", project, environment, newVer, mock.Anything, true).
Return(nil)

es := &mocks.EventService{}
Expand Down Expand Up @@ -301,7 +301,7 @@ func TestRollbackVersionSuccess(t *testing.T) {
// is correct, and the endpoint value remains unchanged. Also test that the statuses -
// the new vers's deployment status should be failed and the router and the current
// ver should be deployed.
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, newVer, mock.Anything)
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, newVer, mock.Anything, true)
assert.Equal(t, models.ID(200), router.CurrRouterVersion.ID)
assert.Equal(t, "current-endpoint", router.Endpoint)
assert.Equal(t, models.RouterVersionStatusDeployed, router.CurrRouterVersion.Status)
Expand Down Expand Up @@ -370,8 +370,8 @@ func TestUndeployRouterSuccess(t *testing.T) {
Return([]*models.RouterVersion{routerVersion, pendingRouterVersion}, nil)

ds := &mocks.DeploymentService{}
ds.On("UndeployRouterVersion", project, environment, routerVersion, mock.Anything).Return(nil)
ds.On("UndeployRouterVersion", project, environment, pendingRouterVersion, mock.Anything).Return(nil)
ds.On("UndeployRouterVersion", project, environment, routerVersion, mock.Anything, false).Return(nil)
ds.On("UndeployRouterVersion", project, environment, pendingRouterVersion, mock.Anything, true).Return(nil)
ds.On("DeleteRouterEndpoint", project, environment, &models.RouterVersion{Router: router}).Return(nil)

es := &mocks.EventService{}
Expand Down Expand Up @@ -400,8 +400,8 @@ func TestUndeployRouterSuccess(t *testing.T) {
assert.Equal(t, models.RouterVersionStatusUndeployed, router.CurrRouterVersion.Status)
assert.Equal(t, "", router.Endpoint)
// Assert calls
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, routerVersion, mock.Anything)
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, pendingRouterVersion, mock.Anything)
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, routerVersion, mock.Anything, false)
ds.AssertCalled(t, "UndeployRouterVersion", project, environment, pendingRouterVersion, mock.Anything, true)
ds.AssertCalled(t, "DeleteRouterEndpoint", project, environment, &models.RouterVersion{Router: router})
rs.AssertCalled(t, "Save", modifiedRouter)
}
4 changes: 2 additions & 2 deletions api/turing/batch/ensembling/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,12 @@ func (c *ensemblingController) createSecret(request *CreateEnsemblingJobRequest,
}

func (c *ensemblingController) cleanup(jobName string, namespace string) {
err := c.clusterController.DeleteSecret(jobName, namespace)
err := c.clusterController.DeleteSecret(jobName, namespace, false)
if err != nil {
log.Warnf("failed deleting secret %s in namespace %s: %v", jobName, namespace, err)
}

err = c.clusterController.DeleteConfigMap(jobName, namespace)
err = c.clusterController.DeleteConfigMap(jobName, namespace, false)
if err != nil {
log.Warnf("failed deleting job spec %s in namespace %s: %v", jobName, namespace, err)
}
Expand Down
36 changes: 18 additions & 18 deletions api/turing/batch/ensembling/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ func TestCreate(t *testing.T) {
clusterController: func() cluster.Controller {
ctrler := &clustermock.Controller{}
ctrler.On("CreateNamespace", mock.Anything).Return(fmt.Errorf("hi"))
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -251,8 +251,8 @@ func TestCreate(t *testing.T) {
nil,
fmt.Errorf("hi"),
)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -291,8 +291,8 @@ func TestCreate(t *testing.T) {
nil,
fmt.Errorf("hi"),
)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -342,8 +342,8 @@ func TestCreate(t *testing.T) {
mock.Anything,
mock.Anything,
).Return(nil, fmt.Errorf("hi"))
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -394,8 +394,8 @@ func TestCreate(t *testing.T) {
mock.Anything,
mock.Anything,
).Return(nil, nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -448,8 +448,8 @@ func TestCreate(t *testing.T) {
).Return(nil, nil)
ctrler.On("CreateSecret", mock.Anything, mock.Anything).Return(nil, nil)
ctrler.On("ApplyConfigMap", mock.Anything, mock.Anything).Return(fmt.Errorf("hi"))
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -504,8 +504,8 @@ func TestCreate(t *testing.T) {
ctrler.On("CreateSecret", mock.Anything, mock.Anything).Return(nil, nil)
ctrler.On("ApplyConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("CreateSparkApplication", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("hi"))
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)
return ctrler
},
mlpService: func() service.MLPService {
Expand Down Expand Up @@ -661,8 +661,8 @@ func TestDelete(t *testing.T) {
"success | delete spark application": {
clusterController: func() cluster.Controller {
ctrler := &clustermock.Controller{}
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)

ctrler.On("GetSparkApplication", mock.Anything, mock.Anything).Return(
&apisparkv1beta2.SparkApplication{},
Expand All @@ -678,8 +678,8 @@ func TestDelete(t *testing.T) {
"success | no such job": {
clusterController: func() cluster.Controller {
ctrler := &clustermock.Controller{}
ctrler.On("DeleteSecret", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything).Return(nil)
ctrler.On("DeleteSecret", mock.Anything, mock.Anything, false).Return(nil)
ctrler.On("DeleteConfigMap", mock.Anything, mock.Anything, false).Return(nil)

ctrler.On("GetSparkApplication", mock.Anything, mock.Anything).Return(
nil,
Expand Down
Loading