Skip to content

Baremetal API v1: Node Power State#1479

Merged
jtopjian merged 1 commit intogophercloud:masterfrom
stbenjam:node-power
Feb 27, 2019
Merged

Baremetal API v1: Node Power State#1479
jtopjian merged 1 commit intogophercloud:masterfrom
stbenjam:node-power

Conversation

@stbenjam
Copy link
Copy Markdown
Contributor

For #1429

API's for changing a Node's power state.

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 25, 2019

Coverage Status

Coverage increased (+0.0009%) to 76.43% when pulling e5c8988 on stbenjam:node-power into 423572c on gophercloud:master.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Feb 25, 2019

Build succeeded.

@stbenjam
Copy link
Copy Markdown
Contributor Author

This should be ready for a review

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@stbenjam just one change with the tag. This will also need rebased with master to resolve the conflict.

@stbenjam
Copy link
Copy Markdown
Contributor Author

Thanks, updated!

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Feb 26, 2019

Build succeeded.

Copy link
Copy Markdown
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@stbenjam Just one last question, otherwise I can merge this if you'd like.

// PowerStateOpts for a request to change a node's power state.
type PowerStateOpts struct {
Target TargetPowerState `json:"target" required:"true"`
Timeout int `json:"timeout,omitempty"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One more question: will there ever be a need to specify a timeout of 0? If so, then this will need to be changed to *int.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah good question - it cannot be 0, so I think this is good to go!

https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/node.py#L505-L506

@jtopjian jtopjian merged commit cfa8434 into gophercloud:master Feb 27, 2019
@stbenjam stbenjam deleted the node-power branch February 27, 2019 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants