Skip to content

kubevirt: Add VM dashboard #2600

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
Merged
openshift-merge-robot merged 1 commit into from
Sep 27, 2019
Merged

kubevirt: Add VM dashboard #2600

openshift-merge-robot merged 1 commit into from
Sep 27, 2019

Conversation

@mareklibra
Copy link
Contributor

@mareklibra mareklibra commented Sep 5, 2019 •
edited
Loading

So far with just a single "Details" card. Additional cards will follow.

Depends on:

vmDetails1

vmDetails2

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. component/kubevirt Related to kubevirt-plugin component/ceph Related to ceph-storage-plugin component/core Related to console core functionality component/noobaa Related to noobaa-storage-plugin component/shared Related to console-shared size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 5, 2019
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 5, 2019

#2520 is merged, will rebase here

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2019
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 5, 2019

/retest

@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 6, 2019

rebased

rawagner
rawagner reviewed Sep 6, 2019
View reviewed changes
frontend/packages/kubevirt-plugin/src/components/vms/vm-dashboard.tsx Outdated
Copy link
Contributor

@rawagner rawagner Sep 6, 2019

Choose a reason for hiding this comment

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

why are all these props optional ?

Copy link
Contributor Author

@mareklibra mareklibra Sep 17, 2019

Choose a reason for hiding this comment

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

Firehose can inject props as undefined, no matter they are optional=false.

@mareklibra mareklibra mentioned this pull request Sep 18, 2019
Merged
2 tasks
@mareklibra mareklibra changed the base branch from master to master-4.3 September 19, 2019 11:35
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 19, 2019

Rebased to master-4.3.
@rawagner , can you please have a look?

@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 20, 2019

/retest

@mareklibra mareklibra mentioned this pull request Sep 20, 2019
Merged
2 tasks
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 23, 2019

rebased

@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 23, 2019

/assign @spadgett

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 23, 2019
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 23, 2019

To simplify code-review, I have split the code. The "shared" part is in #2808 now.

This was referenced Sep 23, 2019
Merged
Closed
Merged
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 23, 2019

/unassign @spadgett

@spadgett spadgett changed the base branch from master-4.3 to master September 26, 2019 14:38
@mareklibra
Copy link
Contributor Author

mareklibra commented Sep 27, 2019

rebased as #2808 is merged now

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2019
rawagner
rawagner suggested changes Sep 27, 2019
View reviewed changes
...end/packages/kubevirt-plugin/src/components/dashboards-page/vm-dashboard/vm-datails-card.tsx Outdated
Copy link
Contributor

@rawagner rawagner Sep 27, 2019

Choose a reason for hiding this comment

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

this should show Not available (with text-secondary css) instead of DASH. Maybe just reuse error field ?

Copy link
Contributor Author

@mareklibra mareklibra Sep 27, 2019

Choose a reason for hiding this comment

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

Done, reusing the error

frontend/packages/kubevirt-plugin/src/components/ip-addresses.tsx Outdated
Copy link
Contributor

@rawagner rawagner Sep 27, 2019

Choose a reason for hiding this comment

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

Not available with text-secondary css

frontend/packages/kubevirt-plugin/src/components/ip-addresses.tsx Outdated
Copy link
Contributor

@rawagner rawagner Sep 27, 2019

Choose a reason for hiding this comment

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

we dont have a better type for vmStatus ?

Copy link
Contributor Author

@mareklibra mareklibra Sep 27, 2019

Choose a reason for hiding this comment

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

done

@openshift-ci-robot openshift-ci-robot added component/dashboard Related to dashboard and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 27, 2019
mareklibra
mareklibra commented Sep 27, 2019
View reviewed changes
frontend/packages/kubevirt-plugin/src/components/vms/vm-resource.tsx Outdated
Copy link
Contributor Author

@mareklibra mareklibra Sep 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

In a follow-up, I will update this page to render Not available instead of the dash (consistently per whole page).

@mareklibra
kubevirt: Add VM dashboard
d05e0bb 
So far with just a single "Details" card. Additional will follow.

The Dashboard is set as the default tab (instead of the Overview).
@rawagner
Copy link
Contributor

rawagner commented Sep 27, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 27, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mareklibra, rawagner

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

The pull request process is described here

Details 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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 27, 2019

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit bdf78a7 into openshift:master Sep 27, 2019
@mareklibra mareklibra mentioned this pull request Sep 30, 2019
Merged
@spadgett spadgett added this to the v4.3 milestone Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gnehapk gnehapk Awaiting requested review from gnehapk

@kyoto kyoto Awaiting requested review from kyoto

1 more reviewer

@rawagner rawagner rawagner requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rawagner rawagner

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/ceph Related to ceph-storage-plugin component/core Related to console core functionality component/dashboard Related to dashboard component/kubevirt Related to kubevirt-plugin component/noobaa Related to noobaa-storage-plugin 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.3

Development

Successfully merging this pull request may close these issues.

6 participants

@mareklibra @rawagner @openshift-ci-robot @openshift-bot @spadgett @openshift-merge-robot