CONSOLE-2458: Add checkbox for cleaning up operand when uninstalling operator #9142

dtaylor113 · 2021-06-03T18:43:58Z

Updated Uninstall Operator Modal:

cherry-picked CONSOLE-2458: Add endpoint for listing operators operands #8949
integrated with 'list-operands' endpoint
added Operands table and 'delete all operands' checkbox to modal body
Upon submit, added k8kill cmds to delete all operands
Updated Error reporting and Alerts

BEFORE SUBMIT

console>kubectl operator list-operands argocd-operator -n openshift-operators
APIVERSION            KIND          NAMESPACE            NAME                                     AGE
argoproj.io/v1alpha1  AppProject    daves-project        default                                  3m
argoproj.io/v1alpha1  AppProject    openshift-operators  default                                  2m46s
argoproj.io/v1alpha1  ArgoCD        daves-project        example-argocd                           3m37s
argoproj.io/v1alpha1  ArgoCD        openshift-operators  example-argocd-in-os-operators-ns        2m51s
argoproj.io/v1alpha1  ArgoCDExport  daves-project        example-argocdexport                     3m28s
argoproj.io/v1alpha1  ArgoCDExport  openshift-operators  example-argocdexport-in-os-operators-ns  2m43s

console>kubectl get AppProject -A
NAMESPACE             NAME      AGE
daves-project         default   7m21s
openshift-operators   default   7m7s

AFTER SUBMIT

console>kubectl operator list-operands argocd-operator -n openshift-operators
list operands: could not find underlying CSV for operator argocd-operator.openshift-operators

console>kubectl get AppProject -A
No resources found

AFTER SUBMIT (with 'delete all operands' NOT checked!)

console>kubectl operator list-operands argocd-operator -n openshift-operators
list operands: could not find underlying CSV for operator argocd-operator.openshift-operators

console>kubectl get AppProject -A
NAMESPACE             NAME      AGE
daves-project         default   7m21s
openshift-operators   default   7m7s

AFTER SUBMIT Errors and Alerts

Please see these screenshots of all possible submit results.

spadgett

Thanks @dtaylor113. Great to see progress on this.

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

frontend/packages/operator-lifecycle-manager/src/components/operand/index.tsx

jhadvig

Code looks good 👍
Found few nits.

frontend/packages/operator-lifecycle-manager/src/style.scss

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

dtaylor113 · 2021-06-08T19:46:58Z

Hi, I've addressed:

new useOperandsWatcher
replaced the full resource table component with simple inline table
misc. comments
error handling and alerting during submit and removing operand instances

TheRealJon

Nice work! Most of my comments are just nits on best practices and consistency stuff. I do think the small helper components that you declared inside the modal component should probably be moved into separate components so that React can do it's job and re-render those only as necessary.

frontend/packages/console-shared/src/hooks/useOperands.ts

frontend/packages/console-shared/src/hooks/promise-handler.ts

frontend/packages/console-shared/src/hooks/useOperands.ts

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

frontend/packages/operator-lifecycle-manager/src/style.scss

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

dtaylor113 · 2021-06-23T20:23:39Z

Hi @TheRealJon, I believe I have address all of your review comments -thanks

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

spadgett · 2021-09-02T18:40:27Z

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

+  cancel?: () => void;
+};
+
+type operandErrorsProps = { operand: K8sResourceCommon; errorMessage: string };


I wouldn't call this props since it's not a components. Types should have a uppercase first letter.

Suggested change

type operandErrorsProps = { operand: K8sResourceCommon; errorMessage: string };

type OperandError = { operand: K8sResourceCommon; errorMessage: string };

spadgett · 2021-09-02T18:40:47Z

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

+  );
+};
+
+const OperandErrorsList: React.FC<OperandErrorsListProps> = ({


Suggested change

const OperandErrorsList: React.FC<OperandErrorsListProps> = ({

const OperandErrorList: React.FC<OperandErrorListProps> = ({

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

spadgett · 2021-09-02T18:45:09Z

Hi @yanpzhan, currently resources are sorted by name, we are considering changing to sort by kind then namespace then name.

Let's not block the feature on this. We can handle this in a follow on.

spadgett · 2021-09-02T18:45:54Z

Looks good overall. Just some minor comments, but we should be able to go ahead and test.

dtaylor113 · 2021-09-02T20:25:28Z

Hi @yanpzhan, test case lgtm -thanks

spadgett

One nit, otherwise LGTM. We'll need QE approval. Thanks @dtaylor113 !

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx

spadgett · 2021-09-02T21:35:07Z

@dtaylor113 Will need rebase

yanpzhan · 2021-09-03T05:24:36Z

/label qe-approved

spadgett · 2021-09-03T11:50:58Z

/hold cancel

spadgett · 2021-09-03T11:51:20Z

@dtaylor113 Needs rebase :/

…kubectl-operator package

dtaylor113 · 2021-09-03T14:06:51Z

@spadgett, rebased

spadgett

/lgtm

spadgett · 2021-09-03T17:07:06Z

/retest

…dded Operands table, 'delete all' checkbox, and k8kill cmds to delete operands

spadgett

/lgtm

openshift-ci · 2021-09-03T19:05:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett, TheRealJon

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:

~~OWNERS~~ [TheRealJon,spadgett]

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

dtaylor113 requested review from spadgett, jhadvig and TheRealJon June 3, 2021 18:43

spadgett reviewed Jun 3, 2021

View reviewed changes

jhadvig reviewed Jun 3, 2021

View reviewed changes

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from e85f048 to a27de04 Compare June 8, 2021 18:55

openshift-ci bot added the component/shared Related to console-shared label Jun 8, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from a27de04 to e4aeb4d Compare June 8, 2021 19:39

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from e4aeb4d to e598e23 Compare June 9, 2021 13:46

openshift deleted a comment from openshift-ci bot Jun 9, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch 2 times, most recently from 3201869 to 9da920f Compare June 9, 2021 15:38

openshift deleted a comment from openshift-ci bot Jun 9, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch 2 times, most recently from 8b86d44 to ca83a1e Compare June 18, 2021 15:27

openshift deleted a comment from openshift-ci bot Jun 18, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from ca83a1e to 3567227 Compare June 18, 2021 18:39

openshift deleted a comment from openshift-ci bot Jun 18, 2021

TheRealJon reviewed Jun 18, 2021

View reviewed changes

dtaylor113 force-pushed the operand-deletion-during-uninstall branch 3 times, most recently from 54034ce to 6463150 Compare June 23, 2021 20:18

openshift-ci bot added the kind/cypress Related to Cypress e2e integration testing label Jun 25, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from ac4e59c to 4b80bbb Compare June 25, 2021 20:04

spadgett reviewed Sep 2, 2021

View reviewed changes

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from e34245f to 7447329 Compare September 2, 2021 20:19

spadgett reviewed Sep 2, 2021

View reviewed changes

frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx Outdated Show resolved Hide resolved

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from 7447329 to 592b08a Compare September 2, 2021 20:33

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2021

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Sep 3, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2021

jhadvig added 2 commits September 3, 2021 09:39

Add endpoint for listing operators operands + Add operator-framework/…

172fecc

…kubectl-operator package

Use user token for listing operator's operands

88b3a1e

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from 592b08a to 4b4d504 Compare September 3, 2021 14:05

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2021

spadgett approved these changes Sep 3, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from 4b4d504 to 687db79 Compare September 3, 2021 17:54

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021

Uninstall Operator Modal: integrated with 'list-operands' endpoint, a…

682f7cb

…dded Operands table, 'delete all' checkbox, and k8kill cmds to delete operands

dtaylor113 force-pushed the operand-deletion-during-uninstall branch from 687db79 to 682f7cb Compare September 3, 2021 18:43

spadgett approved these changes Sep 3, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021

openshift-merge-robot merged commit 66b8801 into openshift:master Sep 4, 2021

CONSOLE-2458: Add checkbox for cleaning up operand when uninstalling operator #9142

CONSOLE-2458: Add checkbox for cleaning up operand when uninstalling operator #9142

dtaylor113 commented Jun 3, 2021 •

edited

spadgett left a comment

jhadvig left a comment •

edited

dtaylor113 commented Jun 8, 2021 •

edited

TheRealJon left a comment

dtaylor113 commented Jun 23, 2021

spadgett Sep 2, 2021

dtaylor113 Sep 2, 2021 •

edited

spadgett Sep 2, 2021

dtaylor113 Sep 2, 2021 •

edited

spadgett commented Sep 2, 2021

spadgett commented Sep 2, 2021

dtaylor113 commented Sep 2, 2021

spadgett left a comment

spadgett commented Sep 2, 2021

yanpzhan commented Sep 3, 2021

spadgett commented Sep 3, 2021

spadgett commented Sep 3, 2021

dtaylor113 commented Sep 3, 2021

spadgett left a comment

spadgett commented Sep 3, 2021

spadgett left a comment

openshift-ci bot commented Sep 3, 2021

	type operandErrorsProps = { operand: K8sResourceCommon; errorMessage: string };
	type OperandError = { operand: K8sResourceCommon; errorMessage: string };

	const OperandErrorsList: React.FC<OperandErrorsListProps> = ({
	const OperandErrorList: React.FC<OperandErrorListProps> = ({

CONSOLE-2458: Add checkbox for cleaning up operand when uninstalling operator #9142

CONSOLE-2458: Add checkbox for cleaning up operand when uninstalling operator #9142

Conversation

dtaylor113 commented Jun 3, 2021 • edited

BEFORE SUBMIT

AFTER SUBMIT

AFTER SUBMIT (with 'delete all operands' NOT checked!)

AFTER SUBMIT Errors and Alerts

spadgett left a comment

Choose a reason for hiding this comment

jhadvig left a comment • edited

Choose a reason for hiding this comment

dtaylor113 commented Jun 8, 2021 • edited

TheRealJon left a comment

Choose a reason for hiding this comment

dtaylor113 commented Jun 23, 2021

spadgett Sep 2, 2021

Choose a reason for hiding this comment

dtaylor113 Sep 2, 2021 • edited

Choose a reason for hiding this comment

spadgett Sep 2, 2021

Choose a reason for hiding this comment

dtaylor113 Sep 2, 2021 • edited

Choose a reason for hiding this comment

spadgett commented Sep 2, 2021

spadgett commented Sep 2, 2021

dtaylor113 commented Sep 2, 2021

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Sep 2, 2021

yanpzhan commented Sep 3, 2021

spadgett commented Sep 3, 2021

spadgett commented Sep 3, 2021

dtaylor113 commented Sep 3, 2021

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Sep 3, 2021

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Sep 3, 2021

dtaylor113 commented Jun 3, 2021 •

edited

jhadvig left a comment •

edited

dtaylor113 commented Jun 8, 2021 •

edited

dtaylor113 Sep 2, 2021 •

edited

dtaylor113 Sep 2, 2021 •

edited