-
Notifications
You must be signed in to change notification settings - Fork 180
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
Added ip_power control for BareMetal platform #3550
base: main
Are you sure you want to change the base?
Conversation
Please rebase on latest main to avoid conflict. |
|
||
def on(self, port: int) -> None: | ||
request_on = f"{self._request_cmd}{port}=1" | ||
requests.get(request_on) |
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.
Follow the internal example to check the status, make sure it's reliable.
3e96684
to
43ecb77
Compare
8b12262
to
f5a0fe6
Compare
# This is the 1st commit message: Added ip_power control for BareMetal platform # The commit message microsoft#2 will be skipped: # Added tftp deployment on BareMetal platform (microsoft#3422) # # Co-authored-by: paull <[email protected]>
Co-authored-by: paull <[email protected]>
If you merge from main, please use |
Co-authored-by: paull <[email protected]>
Co-authored-by: paull <[email protected]>
9b8b473
to
ea63b9a
Compare
Use |
|
||
def off(self, port: str) -> None: | ||
request_off = f"{self._request_cmd}{port}=0" | ||
self._set_ip_power(request_off) |
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.
How long to wait, to make sure the turn off stable?
ea63b9a
to
a0ba5ba
Compare
a0ba5ba
to
b50a704
Compare
) | ||
|
||
def _set_ip_power(self, power_cmd: str) -> None: | ||
try: |
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.
If there is no specific reason, don't overwrite the existing exception. The raw call trace is lost by doing this. Refer to below pattern.
response = requests.get(power_cmd, timeout=REQUEST_TIMEOUT)
response.raise_for_status()
self._log.debug(f"Command {power_cmd} done in set_ip_power")
state: features.StopState = features.StopState.Shutdown, | ||
) -> None: | ||
request_off = f"{self._request_cmd}=0" | ||
self._set_ip_power(request_off) |
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.
As mentioned before, please check how long is safe to make sure the power off is totally done.
return Ip9285StartStop | ||
else: | ||
raise NotImplementedError( | ||
f"start_stop type {self.runbook.start_stop.type} " f"is not supported." |
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.
f"start_stop type {self.runbook.start_stop.type} " f"is not supported." | |
f"start_stop type {self.runbook.start_stop.type} is not supported." |
def deploy(self, environment: Environment) -> Any: | ||
# connect to serial console | ||
for node in environment.nodes.list(): | ||
# start serial console to save all log | ||
_ = node.features[features.SerialConsole] | ||
_ = node.features[features.StartStop] |
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.
It doesn't need to follow above SerialConsole line. The SerialConsole must collect log from beginning, so I wrote previous line. The StartStop feature doesn't need to do the same.
No description provided.