-
Notifications
You must be signed in to change notification settings - Fork 703
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.
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
Added backend changes for stop experiment #4227
Changes from 1 commit
f493734
6a19bff
b26fada
82026be
955c09f
3abe422
6acd480
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"context" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"strconv" | ||
"time" | ||
|
||
|
@@ -1316,3 +1317,63 @@ func (c *ChaosExperimentHandler) validateDuplicateExperimentName(ctx context.Con | |
|
||
return nil | ||
} | ||
|
||
func (c *ChaosExperimentHandler) StopExperimentRuns(ctx context.Context, projectID string, experimentID string, experimentRunID *string, r *store.StateData) (bool, error) { | ||
|
||
var experimentRunsID []string | ||
|
||
tkn := ctx.Value(authorization.AuthKey).(string) | ||
username, err := authorization.GetUsername(tkn) | ||
|
||
query := bson.D{ | ||
{"experiment_id", experimentID}, | ||
{"project_id", projectID}, | ||
{"is_removed", false}, | ||
} | ||
experiment, err := c.chaosExperimentOperator.GetExperiment(context.TODO(), query) | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
// if experimentID is provided & no expRunID is present (stop all the corresponding experiment runs) | ||
if experimentRunID == nil { | ||
|
||
// if experiment is of cron type, disable it | ||
if experiment.CronSyntax != "" { | ||
|
||
err = c.DisableCronExperiment(username, experiment, projectID, r) | ||
if err != nil { | ||
return false, err | ||
} | ||
} | ||
Comment on lines
+1342
to
+1348
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see the use case where users might want to temporarily suspend all ongoing runs but any future runs should still happen. We should give the user the option to explicitly disable the future runs imo instead of disabling by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gdsoumya This case is handled in the backend, so if a user sends a both experimentID and experimentRunID, only the current run will be stopped. but if experimentRunID is nil, it will stop all the running experiment and disable the cron if present. However, in future we are planning to add one more API to just enable/disable the cron. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think then in the future when the cron specific endpoint is introduced the behavior of this endpoint should be changed so that it doesn't stop the cron imo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, will remove this one with the addition of cron API |
||
|
||
// Fetching all the experiment runs in the experiment | ||
expRuns, err := dbChaosExperimentRun.NewChaosExperimentRunOperator(c.mongodbOperator).GetExperimentRuns(bson.D{ | ||
{"experiment_id", experimentID}, | ||
{"is_removed", false}, | ||
}) | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
for _, runs := range expRuns { | ||
if (runs.Phase == string(model.ExperimentRunStatusRunning) || runs.Phase == string(model.ExperimentRunStatusTimeout)) && !runs.Completed { | ||
experimentRunsID = append(experimentRunsID, runs.ExperimentRunID) | ||
} | ||
} | ||
|
||
// Check if experiment run count is 0 and if it's not a cron experiment | ||
if len(experimentRunsID) == 0 && experiment.CronSyntax == "" { | ||
return false, fmt.Errorf("no running or timeout experiments found") | ||
namkyu1999 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} else if experimentRunID != nil && *experimentRunID != "" { | ||
experimentRunsID = []string{*experimentRunID} | ||
} | ||
|
||
err = c.chaosExperimentRunService.ProcessExperimentRunStop(ctx, query, experimentRunID, experiment, username, projectID, r) | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
return true, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the call to the actual StopExperimentRuns is missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it was in WIP, have added the implementation.