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
Bug 1793893: Make clear what uninstalling an operator that works for all namespaces does #4137
Conversation
@jhadvig: This pull request references Bugzilla bug 1793893, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
This might be good enough since it will work for OpenShift clusters, but isn't strictly correct.
Oops, my code comment got lost. The problem is that @shawn-hurley Do you know? |
@@ -58,6 +58,15 @@ export const UninstallOperatorModal = withHandlePromise((props: UninstallOperato | |||
.catch(_.noop); | |||
}; | |||
|
|||
const uninstallMsg = | |||
props.subscription.metadata.namespace === 'openshift-operators' ? ( | |||
'all the installed namespaces' |
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.
'all the installed namespaces' | |
'all namespaces' |
We might want to bold this text or make it stronger somehow.
'all the installed namespaces' | ||
) : ( | ||
<> | ||
<i>{props.subscription.metadata.namespace}</i> namespace |
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>{props.subscription.metadata.namespace}</i> namespace | |
the <i>{props.subscription.metadata.namespace}</i> namespace |
cc @itsptk |
FYI @dmesser |
I think the only way to tell, would be to get the OperatorGroup for the namespace of the subscription and determine if it is all namespaces or not. I believe that you could use the |
@spadgett PR updated per our discussion |
frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx
Outdated
Show resolved
Hide resolved
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.
/approve
/lgtm
Thanks @jhadvig 👍
/retest Please review the full test history for this PR and help us cut down flakes. |
/lgtm cancel |
…all namespaces does
@spadgett updated |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig, spadgett 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:
Approvers can indicate their approval by writing |
@jhadvig @spadgett I think there will be a concerted effort to make all the delete modals to be more like the PF4 standard in the future, but for now I'd say to make this one more similar to other delete modals in the console, let's drop the question mark on the first title. So just: Uninstall Operator https://www.patternfly.org/v4/design-guidelines/usage-and-behavior/modal |
@jhadvig: All pull requests linked via external trackers have merged. Bugzilla bug 1793893 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
When a operator with and
AllNamespaces
install mode, which is installed intoopenshift-operators
is uninstalled from any of the installed namespaces, we should inform about the fact that it will be uninstalled from all the namespaces, not just that one particular:In case the operator is installed just to a single namespace we will show the original message:
/assign @spadgett