metal3: add node maintenance actions #1967

atiratree · 2019-07-09T14:57:47Z

detect availability according to NodeMaintenance CRD
start maintenance dialog
enhance & refactor status
small refactoring
reverted the old flatten pattern to support status filtering and to
prevent duplicate resource lookup

depends on
~~- [] #1964~~

@jtomasek @honza

frontend/packages/metal3-plugin/src/components/host-menu-actions.tsx

frontend/packages/metal3-plugin/src/components/host-detail.tsx

atiratree · 2019-07-10T08:47:03Z

Screenshots

Start Maintenance

--

Stop Maintenance

--

Stopping Maintenance

jtomasek · 2019-07-10T10:22:02Z

/assign

jtomasek

Looks pretty good!

frontend/packages/metal3-plugin/src/constants.ts

frontend/packages/metal3-plugin/src/selectors/nm.ts

frontend/packages/metal3-plugin/src/components/host-detail.tsx

frontend/packages/metal3-plugin/src/components/host.tsx

frontend/packages/metal3-plugin/src/components/machine-cell.tsx

frontend/packages/metal3-plugin/src/utils/host-status.ts

frontend/packages/metal3-plugin/src/components/host-detail.tsx

vojtechszocs · 2019-07-12T14:17:58Z

frontend/packages/metal3-plugin/src/components/host-detail.tsx

FYI, this can also be solved via dynamic array declaration pattern:

const resources: FirehoseResource[] = [ { /* stuff */ }, ...(hasNodeMaintenanceCapability && [{ /* stuff */ }] || []), ];

But I'm also OK with proposed code.

frontend/packages/metal3-plugin/src/components/host-detail.tsx

frontend/packages/metal3-plugin/src/components/host-menu-actions.tsx

vojtechszocs · 2019-07-12T14:39:49Z

frontend/packages/metal3-plugin/src/components/host.tsx

I find both ...host property spread and host property inclusion a bit weird.

@suomiy IIRC this is to ensure some basic stuff like IDs gets included, right?

yes it is (UID for example) and the host to have the original unchanged object

vojtechszocs · 2019-07-12T14:41:40Z

frontend/packages/metal3-plugin/src/components/host.tsx

This same code is also in host-detail module. Maybe create a function to cut down code duplication?

I think it is better to leave it separate. One might want to add something else to the stateToProps function and forget that somebody else is also using it.

Well, I didn't mean the whole hostsPageStateToProps function 😃 just the hasNodeMaintenanceCapability computation from k8s object.

Anyway, I'm OK with this as-is.

frontend/packages/metal3-plugin/src/components/host.tsx

vojtechszocs · 2019-07-12T14:49:33Z

frontend/packages/metal3-plugin/src/components/host.tsx

This object { hasNodeMaintenanceCapability: boolean; } declaration is repeated on many places across many files.

Isn't it better to declare (in types.ts) and consistently reuse

type HostCustomData = { hasNodeMaintenanceCapability: boolean; };

again, IMO it is better to leave it for better extendability

frontend/packages/metal3-plugin/src/selectors/nm.ts

atiratree · 2019-07-15T10:42:59Z

/retest

atiratree · 2019-07-15T10:43:12Z

hopefully fixed and responded to all comments

atiratree · 2019-07-15T13:45:42Z

added correct icon and fixed dialog messages

atiratree · 2019-07-15T14:06:52Z

/retest

frontend/packages/metal3-plugin/src/components/host-detail.tsx

frontend/packages/metal3-plugin/src/components/host.tsx

vojtechszocs · 2019-07-15T17:29:27Z

frontend/packages/metal3-plugin/src/components/host.tsx

Well, I didn't mean the whole hostsPageStateToProps function 😃 just the hasNodeMaintenanceCapability computation from k8s object.

Anyway, I'm OK with this as-is.

frontend/packages/metal3-plugin/src/components/modals/start-node-maintenance-modal.tsx

frontend/packages/metal3-plugin/src/utils/host-status.ts

vojtechszocs · 2019-07-16T15:27:41Z

frontend/packages/metal3-plugin/src/components/host-menu-actions.tsx

Hm, so KubeVirt plugin defines the NodeMaintenanceModel but Metal3 plugin realizes the corresponding actions?

Why isn't NodeMaintenanceModel moved to Metal3 plugin?

ah, you are right; that is much cleaner choice

- detect availability according to NodeMaintenance CRD - start maintenance dialog - enhance & refactor status - small refactoring - reverted the old flatten pattern to support status filtering and to prevent duplicate resource lookup

atiratree · 2019-07-17T12:32:42Z

/test e2e-aws-console
/test e2e-aws

atiratree · 2019-07-17T14:07:30Z

/test e2e-aws-console
/test e2e-aws

vojtechszocs · 2019-07-17T16:35:10Z

frontend/packages/metal3-plugin/src/models.ts

  crd: true,
 };
+
+export const NodeMaintenanceModel: K8sKind = {


vojtechszocs · 2019-07-17T16:36:54Z

/lgtm
/approve

openshift-ci-robot · 2019-07-17T16:37:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jtomasek, suomiy, vojtechszocs

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:

~~frontend/OWNERS~~ [vojtechszocs]

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

vojtechszocs · 2019-07-17T16:37:41Z

Dependant PR #1966 is acked and should get merged, please rebase if necessary.

openshift-bot · 2019-07-17T17:13:06Z

/retest

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

openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 9, 2019

openshift-ci-robot requested review from alecmerdler and jelkosz July 9, 2019 14:58

This was referenced Jul 9, 2019

shared: move lookup utils from kubevirt-plugin #1966

Merged

pass custom data to the list, detail and actions #1965

Merged

spadgett reviewed Jul 9, 2019

View reviewed changes

frontend/packages/metal3-plugin/src/components/host-menu-actions.tsx Outdated Show resolved Hide resolved

spadgett added this to the v4.2 milestone Jul 9, 2019

atiratree force-pushed the nodeDrain branch 3 times, most recently from 3752559 to e1df91c Compare July 9, 2019 19:00

atiratree changed the title ~~add node maintenance actions~~ metal3: add node maintenance actions Jul 9, 2019

atiratree force-pushed the nodeDrain branch from e1df91c to d11a190 Compare July 10, 2019 08:31

atiratree commented Jul 10, 2019

View reviewed changes

frontend/packages/metal3-plugin/src/components/host-detail.tsx Outdated Show resolved Hide resolved

openshift-ci-robot assigned jtomasek Jul 10, 2019

jtomasek suggested changes Jul 12, 2019

View reviewed changes

vojtechszocs suggested changes Jul 12, 2019

View reviewed changes

atiratree force-pushed the nodeDrain branch from d11a190 to f99fda4 Compare July 15, 2019 10:13

atiratree mentioned this pull request Jul 15, 2019

shared: add default value to getMachineNode selector #2029

Merged

atiratree force-pushed the nodeDrain branch from f99fda4 to 10d6940 Compare July 15, 2019 13:45

vojtechszocs suggested changes Jul 15, 2019

View reviewed changes

atiratree force-pushed the nodeDrain branch from 10d6940 to 6a0e352 Compare July 15, 2019 19:30

jtomasek mentioned this pull request Jul 16, 2019

[metal3] Introduces power actions for Baremetal hosts #2046

Merged

1 task

atiratree force-pushed the nodeDrain branch from 6a0e352 to 31cae0b Compare July 16, 2019 11:47

vojtechszocs reviewed Jul 16, 2019

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019

atiratree force-pushed the nodeDrain branch from 31cae0b to adc4c40 Compare July 16, 2019 17:59

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019

suomiy added 2 commits July 17, 2019 12:26

shared: move lookup utils from kubevirt-plugin

ae3845e

add node maintenance actions

e820815

- detect availability according to NodeMaintenance CRD - start maintenance dialog - enhance & refactor status - small refactoring - reverted the old flatten pattern to support status filtering and to prevent duplicate resource lookup

atiratree force-pushed the nodeDrain branch from adc4c40 to e820815 Compare July 17, 2019 10:39

jtomasek approved these changes Jul 17, 2019

View reviewed changes

vojtechszocs reviewed Jul 17, 2019

View reviewed changes

frontend/packages/metal3-plugin/src/models.ts

crd: true,

};

export const NodeMaintenanceModel: K8sKind = {

Copy link

Contributor

vojtechszocs Jul 17, 2019

Choose a reason for hiding this comment

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

👍

openshift-ci-robot assigned vojtechszocs Jul 17, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2019

openshift-merge-robot merged commit f3e76f3 into openshift:master Jul 17, 2019

metal3: add node maintenance actions #1967

metal3: add node maintenance actions #1967

Uh oh!

Conversation

atiratree commented Jul 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

atiratree commented Jul 10, 2019

Screenshots

Uh oh!

jtomasek commented Jul 10, 2019

Uh oh!

jtomasek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

atiratree commented Jul 15, 2019

Uh oh!

atiratree commented Jul 15, 2019

Uh oh!

atiratree commented Jul 15, 2019

Uh oh!

atiratree commented Jul 15, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atiratree commented Jul 17, 2019

Uh oh!

atiratree commented Jul 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vojtechszocs commented Jul 17, 2019

Uh oh!

openshift-ci-robot commented Jul 17, 2019

Uh oh!

vojtechszocs commented Jul 17, 2019

atiratree commented Jul 9, 2019 •

edited

Loading