diff --git a/api/turing/api/base_controller.go b/api/turing/api/base_controller.go index f3b837ac0..3112492f8 100644 --- a/api/turing/api/base_controller.go +++ b/api/turing/api/base_controller.go @@ -85,11 +85,3 @@ func (c BaseController) getRouterVersionFromRequestVars( } return routerVersion, nil } - -func (c BaseController) getDeployFlagFromRequestVars(vars RequestVars) (deployFlag bool, error *Response) { - deployFlag, err := getBoolFromVars(vars, "deploy") - if err != nil { - return false, BadRequest("invalid deploy flag value", err.Error()) - } - return deployFlag, nil -} diff --git a/api/turing/api/request_utils.go b/api/turing/api/request_utils.go index 8264a3818..910acccdf 100644 --- a/api/turing/api/request_utils.go +++ b/api/turing/api/request_utils.go @@ -24,14 +24,3 @@ func getIDFromVars(vars RequestVars, key string) (models.ID, error) { id, err := getIntFromVars(vars, key) return models.ID(id), err } - -// gets a deploy flag from the provided request variables. If not found, will throw -// an error. -func getBoolFromVars(vars RequestVars, key string) (bool, error) { - var v string - var ok bool - if v, ok = vars.get(key); !ok { - return false, fmt.Errorf("key %s not found in vars", key) - } - return strconv.ParseBool(v) -} diff --git a/api/turing/api/routers_api.go b/api/turing/api/routers_api.go index 0ee8c8e3f..0940a3142 100644 --- a/api/turing/api/routers_api.go +++ b/api/turing/api/routers_api.go @@ -129,17 +129,12 @@ func (c RoutersController) UpdateRouter(r *http.Request, vars RequestVars, body var errResp *Response var project *mlp.Project var router *models.Router - var toDeploy bool - if project, errResp = c.getProjectFromRequestVars(vars); errResp != nil { return errResp } if router, errResp = c.getRouterFromRequestVars(vars); errResp != nil { return errResp } - if toDeploy, errResp = c.getDeployFlagFromRequestVars(vars); errResp != nil { - return errResp - } request := body.(*request.CreateOrUpdateRouterRequest) @@ -149,8 +144,8 @@ func (c RoutersController) UpdateRouter(r *http.Request, vars RequestVars, body "Router name and environment cannot be changed after creation") } - // If the new version needs to be deployed, check if any deployment is in progress, if yes, disallow the update - if toDeploy && router.Status == models.RouterStatusPending { + // Check if any deployment is in progress, if yes, disallow the update + if router.Status == models.RouterStatusPending { return BadRequest("invalid update request", "another version is currently pending deployment") } @@ -160,32 +155,24 @@ func (c RoutersController) UpdateRouter(r *http.Request, vars RequestVars, body rVersion, err := request.BuildRouterVersion( router, c.RouterDefaults, c.AppContext.CryptoService, c.AppContext.ExperimentsService, c.EnsemblersService) if err == nil { - // Save router version as undeployed if there is no need to deploy it - if !toDeploy { - rVersion.Status = models.RouterVersionStatusUndeployed - } + // Save router version, re-assign the value of err routerVersion, err = c.RouterVersionsService.Save(rVersion) } if err != nil { - if toDeploy { - return InternalServerError("unable to update router", err.Error()) - } - return InternalServerError("unable to save router version", err.Error()) + return InternalServerError("unable to update router", err.Error()) } // Deploy the new version - if toDeploy { - go func() { - err := c.deployOrRollbackRouter(project, router, routerVersion) - if err != nil { - log.Errorf("Error deploying router %s:%s:%d: %v", - project.Name, router.Name, routerVersion.Version, err) - } - }() - return Ok(rVersion) - } - return Ok(rVersion) + go func() { + err := c.deployOrRollbackRouter(project, router, routerVersion) + if err != nil { + log.Errorf("Error deploying router %s:%s:%d: %v", + project.Name, router.Name, routerVersion.Version, err) + } + }() + + return Ok(router) } // DeleteRouter deletes a router and all its associated versions. diff --git a/api/turing/api/routers_api_test.go b/api/turing/api/routers_api_test.go index fc1064655..b2a6d7952 100644 --- a/api/turing/api/routers_api_test.go +++ b/api/turing/api/routers_api_test.go @@ -359,7 +359,7 @@ func TestUpdateRouter(t *testing.T) { routerSvc.On("Save", mock.Anything). Return(nil, errors.New("test router deployment failure")) // Router Version Service - routerVersionToUpdate := &models.RouterVersion{ + routerVersion := &models.RouterVersion{ RouterID: 4, Router: router4, ExperimentEngine: &models.ExperimentEngine{ @@ -370,20 +370,8 @@ func TestUpdateRouter(t *testing.T) { }, Status: models.RouterVersionStatusPending, } - routerVersionToSave := &models.RouterVersion{ - RouterID: 3, - Router: router3, - ExperimentEngine: &models.ExperimentEngine{ - Type: models.ExperimentEngineTypeNop, - }, - LogConfig: &models.LogConfig{ - ResultLoggerType: models.NopLogger, - }, - Status: models.RouterVersionStatusUndeployed, - } routerVersionSvc := &mocks.RouterVersionsService{} - routerVersionSvc.On("Save", routerVersionToUpdate).Return(routerVersionToUpdate, nil) - routerVersionSvc.On("Save", routerVersionToSave).Return(routerVersionToSave, nil) + routerVersionSvc.On("Save", routerVersion).Return(routerVersion, nil) // Define tests tests := map[string]struct { @@ -396,11 +384,11 @@ func TestUpdateRouter(t *testing.T) { expected: BadRequest("invalid project id", "key project_id not found in vars"), }, "failure | project not found": { - vars: RequestVars{"project_id": {"1"}, "router_id": {"1"}, "deploy": {"true"}}, + vars: RequestVars{"project_id": {"1"}, "router_id": {"1"}}, expected: NotFound("project not found", "test project error"), }, "failure | bad request (missing router_id)": { - vars: RequestVars{"project_id": {"2"}, "deploy": {"true"}}, + vars: RequestVars{"project_id": {"2"}}, expected: BadRequest("invalid router id", "key router_id not found in vars"), }, "failure | router not found": { @@ -410,15 +398,11 @@ func TestUpdateRouter(t *testing.T) { vars: RequestVars{"project_id": {"2"}, "router_id": {"1"}}, expected: NotFound("router not found", "test router error"), }, - "failure | bad request (missing deploy flag)": { - vars: RequestVars{"project_id": {"2"}, "router_id": {"2"}}, - expected: BadRequest("invalid deploy flag value", "key deploy not found in vars"), - }, "failure | invalid router config": { body: &request.CreateOrUpdateRouterRequest{ Name: "router1", }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"2"}, "deploy": {"true"}}, + vars: RequestVars{"project_id": {"2"}, "router_id": {"2"}}, expected: BadRequest( "invalid router configuration", "Router name and environment cannot be changed after creation", @@ -429,29 +413,21 @@ func TestUpdateRouter(t *testing.T) { Name: "router2", Environment: "dev", }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"2"}, "deploy": {"true"}}, + vars: RequestVars{"project_id": {"2"}, "router_id": {"2"}}, expected: BadRequest( "invalid update request", "another version is currently pending deployment", ), }, - "failure | update router version": { + "failure | build router version": { body: &request.CreateOrUpdateRouterRequest{ Name: "router3", Environment: "dev", }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"3"}, "deploy": {"true"}}, + vars: RequestVars{"project_id": {"2"}, "router_id": {"3"}}, expected: InternalServerError("unable to update router", "router config is empty"), }, - "failure | save router version": { - body: &request.CreateOrUpdateRouterRequest{ - Name: "router3", - Environment: "dev", - }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"3"}, "deploy": {"false"}}, - expected: InternalServerError("unable to save router version", "router config is empty"), - }, - "success | update router": { + "success": { body: &request.CreateOrUpdateRouterRequest{ Name: "router4", Environment: "dev", @@ -464,29 +440,10 @@ func TestUpdateRouter(t *testing.T) { }, }, }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"4"}, "deploy": {"true"}}, - expected: &Response{ - code: 200, - data: routerVersionToUpdate, - }, - }, - "success | save router version": { - body: &request.CreateOrUpdateRouterRequest{ - Name: "router3", - Environment: "dev", - Config: &request.RouterConfig{ - ExperimentEngine: &request.ExperimentEngineConfig{ - Type: "nop", - }, - LogConfig: &request.LogConfig{ - ResultLoggerType: models.NopLogger, - }, - }, - }, - vars: RequestVars{"project_id": {"2"}, "router_id": {"3"}, "deploy": {"false"}}, + vars: RequestVars{"project_id": {"2"}, "router_id": {"4"}}, expected: &Response{ code: 200, - data: routerVersionToSave, + data: router4, }, }, }