Add run termination controls to ui #1039

rileyjbauer · 2019-03-25T22:08:22Z

Adds a Terminate button to the run details page, allowing users to request that the pipeline execution be stopped:

This action is irreversible and the user is asked to confirm before the command is issued:

Afterward, the Terminate button is disabled, and a warning banner is shown explaining that the run was terminated. The step that was executing when the pipeline terminated gets a special status icon indicating that it was stopped:

If there were multiple steps running when the pipeline was terminated, each will show the stopped icon:

The list pages will show terminated runs as Failed, and will not indicate that they are terminated due to #1040

This change is

rileyjbauer · 2019-03-25T22:09:01Z

Fixes #413

Ark-kun · 2019-03-26T01:14:59Z

/test kubeflow-pipeline-e2e-test

frontend/src/Css.tsx

yebrahim · 2019-03-26T02:29:49Z

frontend/src/lib/Buttons.ts

+    callback: (_: string[], success: boolean) => void): void {
+    this._dialogActionHandler(
+      ids,
+      'Do you want to terminate this run? This action cannot be undone.',


I think this text can use some more elaboration. Maybe it should indicate what happens to running pods.

How about:
Do you want to terminate this run? This action cannot be undone. The run is terminated by setting the activeDeadlineSeconds of the Argo workflow to 0. This terminates the workflow without deleting the pods.

The run is terminated by setting the activeDeadlineSeconds of the Argo workflow to 0. This terminates the workflow without deleting the pods.

I'd say that's too much implementation info.

How about
"Do you want to terminate this run?
All the running tasks will be terminated."

I agree, there's no need to surface Argo here.
Do running tasks (pods) really terminate? Argo just doesn't schedule any more pods AFAIR.

Yes, the pods are terminated:
https://kubernetes.io/docs/concepts/workloads/controllers/jobs-run-to-completion/#job-termination-and-cleanup

Do you want to terminate this run? This action cannot be undone. This will terminate any running pods, but they will not be deleted. ?

Is Argo using a k8s job? I didn't think so. Did you verify that any long running pods are killed?

Pods are terminated before they complete, not after

yebrahim · 2019-03-26T02:37:09Z

frontend/src/pages/Status.tsx

    case NodePhase.UNKNOWN:
      return false;
    default:
      return false;
  }
 }

-export function statusToBgColor(status?: NodePhase): string {
+export function statusToBgColor(status?: NodePhase, errorMessage?: string): string {


nit: Would it be cleaner if the status is reconciled with the error message before being passed here, rather than have this function do the reconciliation?

Neither is great because what we're doing isn't great. I think this approach has a slightly lower chance of mistakes (e.g. failing to provide error / failing to call disambiguation function) being made because the extra param serves as a reminder (i think i can give it a better name), but I agree that it's not ideal and am willing to have the disambiguation function called outside of these functions

yebrahim · 2019-03-26T02:38:25Z

frontend/src/pages/Page.tsx

@@ -49,6 +49,10 @@ export abstract class Page<P, S> extends React.Component<P & PageProps, S> {
    this._isMounted = false;
  }

+  public componentDidMount(): void {


Curious why this is needed, doesn't it mean cleanup isn't done properly somewhere?

Yes, the banner sticks when navigating from the RunDetails page back to the experiment/run list page using the toolbar nav arrow. We could pass the clearBanner function to the Toolbar and call it when the arrow is clicked, but is that better?

Shouldn't the unmount method on RunDetails clear the banner in that case?

Yes, that also fixes it and I'll add it there.

Any reason to remove this code though?

clearBanner is already called in the unmount method right? I was wondering why that doesn't take care of it.
It's just a little confusing to me to see two initialization code blocks (constructor and componentDidMount), I think it should be avoided, but it's not a strong reason.

Yeah, I'm not sure why it doesn't take care of it

yebrahim · 2019-03-26T02:40:33Z

frontend/src/pages/RunDetails.tsx

@@ -374,6 +390,7 @@ class RunDetails extends Page<RunDetailsProps, RunDetailsState> {
        buttons.restore(idGetter, true, () => this.refresh()) :
        buttons.archive(idGetter, true, () => this.refresh());
      actions.splice(2, 1, newButton);


Is this index still correct now that we have one more button before Archive/Restore?

Good call. No, it was removing the refresh button.

Do we need a refresh button though now that we have autorefresh? it would seem not

Ark-kun · 2019-03-28T23:23:57Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T00:14:06Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T01:29:04Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T04:21:46Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T06:18:17Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T16:43:29Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T19:54:20Z

/test kubeflow-pipeline-e2e-test

rileyjbauer · 2019-03-29T21:12:29Z

/test kubeflow-pipeline-e2e-test

Ark-kun · 2019-03-30T05:47:13Z

/test kubeflow-pipeline-e2e-test

IronPan · 2019-04-01T16:32:02Z

/lgtm
/approve

k8s-ci-robot · 2019-04-01T16:32:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: IronPan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~backend/OWNERS~~ [IronPan]
~~frontend/OWNERS~~ [IronPan]
~~test/frontend-integration-test/OWNERS~~ [IronPan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* handle pipelineloop status When using `embedded-status: minimal`, the nested pipelineloop status is missing. Add logic in persistence agent to retrieve runs and taskruns status for nested pipelineloop. Signed-off-by: Yihong Wang <yh.wang@ibm.com> * Add an arg to specify the kinds Use an arg to specify the kinds which contain childReferences information. Persistence agent uses this list to retrieve taskrun/run status and embedded them into the final PipelineRun yaml. Signed-off-by: Yihong Wang <yh.wang@ibm.com> Signed-off-by: Yihong Wang <yh.wang@ibm.com>

rileyjbauer assigned IronPan and ajayalfred Mar 25, 2019

k8s-ci-robot requested review from vicaire, yebrahim and IronPan March 25, 2019 22:08

rileyjbauer added the area/frontend label Mar 25, 2019

k8s-ci-robot added the size/XXL label Mar 25, 2019

yebrahim reviewed Mar 26, 2019

View reviewed changes

rileyjbauer added 5 commits March 28, 2019 22:13

Update swagger definitions

ab23911

WIP - Adds ability to terminate runs to frontend

c3adc64

Update snapshots

919d3e2

Adds tests. Also changes warning message color to orange rather than red

5e4d4f3

Remove refresh button from run details page

3c7f648

rileyjbauer force-pushed the add-termination-controls-to-ui branch from b07f28e to 3c7f648 Compare March 29, 2019 05:16

rileyjbauer changed the title ~~Add termination controls to ui~~ Add run termination controls to ui Mar 29, 2019

Elaborate terminate confirmation message

5d194a4

rileyjbauer added 2 commits March 29, 2019 10:46

Minor fixes

9163169

Remove references to refresh button from integration tests

7ef2e35

k8s-ci-robot added the lgtm label Apr 1, 2019

k8s-ci-robot added the approved label Apr 1, 2019

k8s-ci-robot merged commit 94925ff into kubeflow:master Apr 1, 2019

rileyjbauer mentioned this pull request Apr 1, 2019

Ability to stop runs #413

Closed

hongye-sun mentioned this pull request Apr 4, 2019

[Feature] Supports parameterized S3Artifactory for Pipeline and ContainerOp in kfp package #1064

Merged

rileyjbauer deleted the add-termination-controls-to-ui branch May 6, 2019 22:14

magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023

Fix small typo (kubeflow#1039)

0606993

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add run termination controls to ui #1039

Add run termination controls to ui #1039

rileyjbauer commented Mar 25, 2019 •

edited

Loading

rileyjbauer commented Mar 25, 2019

Ark-kun commented Mar 26, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019

Ark-kun Mar 26, 2019

Ark-kun Mar 26, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019 •

edited

Loading

yebrahim Mar 27, 2019

rileyjbauer Mar 28, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019

yebrahim Mar 26, 2019

rileyjbauer Mar 28, 2019 •

edited

Loading

yebrahim Mar 26, 2019

rileyjbauer Mar 26, 2019

Ark-kun commented Mar 28, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

Ark-kun commented Mar 30, 2019

IronPan commented Apr 1, 2019

k8s-ci-robot commented Apr 1, 2019

Add run termination controls to ui #1039

Add run termination controls to ui #1039

Conversation

rileyjbauer commented Mar 25, 2019 • edited Loading

rileyjbauer commented Mar 25, 2019

Ark-kun commented Mar 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rileyjbauer Mar 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rileyjbauer Mar 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ark-kun commented Mar 28, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

rileyjbauer commented Mar 29, 2019

Ark-kun commented Mar 30, 2019

IronPan commented Apr 1, 2019

k8s-ci-robot commented Apr 1, 2019

rileyjbauer commented Mar 25, 2019 •

edited

Loading

rileyjbauer Mar 26, 2019 •

edited

Loading

rileyjbauer Mar 28, 2019 •

edited

Loading