Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Cluster Operator upgrade activity #3765

Merged
merged 1 commit into from Jan 15, 2020
Merged

Add Cluster Operator upgrade activity #3765

merged 1 commit into from Jan 15, 2020

Conversation

rawagner
Copy link
Contributor

co_upgrade

Update/Upgrade wording fix for dashboards and cluster operators page will follow

@andybraren

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. component/core Related to console core functionality component/dashboard Related to dashboard component/metal3 Related to metal3-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared labels Dec 12, 2019
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 12, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2019
@rawagner
Copy link
Contributor Author

Or something like this ? @andybraren
upgrade_alt

@andybraren
Copy link
Contributor

My thought would be to align with the same format for all other resource-related activities, which I believe is:

  • The time the upgrade started (if available, no problem if not)
  • The same Status label that's shown in the OLM/CO List views (I'm not quite sure where to find this, so I'll assume it's "Upgrading" for now)
  • A Status Popover if upgrades could take a long time and we have more information to share with the user (possibly "Progressing: not all deployment replicas are ready" in this case, but that message doesn't seem super useful to me unless it changes as it progresses)
  • The resource badge and name of the OLM/CO operator below the status

Does this PR replace #3575 by the way?

concept-operator-activity

@rawagner
Copy link
Contributor Author

rawagner commented Dec 13, 2019 •
edited

Thanks, i will update the code accodingly

My thought would be to align with the same format for all other resource-related activities, which I believe is:

  • The time the upgrade started (if available, no problem if not)
  • The same Status label that's shown in the OLM/CO List views (I'm not quite sure where to find this, so I'll assume it's "Upgrading" for now)
  • A Status Popover if upgrades could take a long time and we have more information to share with the user (possibly "Progressing: not all deployment replicas are ready" in this case, but that message doesn't seem super useful to me unless it changes as it progresses)

Yea, im not sure if we can get other messages, i will try to find out and possiblity just remove it

  • The resource badge and name of the OLM/CO operator below the status

Does this PR replace #3575 by the way?

it does not. This is for Cluster operators and #3575 is for CVS (OLM operators). I will update #3575 to look the same too.

concept-operator-activity

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2019
@rawagner
Copy link
Contributor Author

PR updated
co_upgrade_new

andybraren
andybraren approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@andybraren andybraren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

@rawagner rawagner force-pushed the co_upgrade_activity branch 2 times, most recently from 32c3e91 to 0277e60 Compare January 7, 2020 09:32
@rawagner
Copy link
Contributor Author

rawagner commented Jan 7, 2020

/assign @jtomasek please take a look

jtomasek
jtomasek approved these changes Jan 7, 2020
View reviewed changes
Copy link

@jtomasek jtomasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 nit inline

frontend/packages/console-app/src/components/dashboards-page/ClusterOperatorUpgradeActivity.tsx Outdated
<ActivityItem>Upgrading</ActivityItem>
<ResourceLink
kind={referenceForModel(ClusterOperatorModel)}
name={resource.metadata.name}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name={resource.metadata.name}
name={getName(resource)}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@jtomasek
Copy link

jtomasek commented Jan 7, 2020

/retest

1 similar comment
@rawagner
Copy link
Contributor Author

rawagner commented Jan 8, 2020

/retest

@petr-muller petr-muller mentioned this pull request Jan 8, 2020
Closed
@jtomasek
Copy link

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andybraren, jtomasek, rawagner

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 89701de into openshift:master Jan 15, 2020
@spadgett spadgett added this to the v4.4 milestone Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtomasek jtomasek jtomasek approved these changes

@andybraren andybraren andybraren approved these changes

@flofuchs flofuchs Awaiting requested review from flofuchs

Assignees

@jtomasek jtomasek

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dashboard Related to dashboard component/metal3 Related to metal3-plugin component/sdk Related to console-plugin-sdk component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v4.4
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@rawagner @andybraren @jtomasek @openshift-ci-robot @openshift-bot @spadgett @openshift-merge-robot