don't try to get IP of VM when its not running #4521
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rohanKanojia The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pkg/crc/machine/status.go
Outdated
| @@ -35,6 +35,10 @@ func (client *client) Status() (*types.ClusterStatusResult, error) { | |||
| return nil, errors.Wrap(err, "Cannot get machine state") | |||
| } | |||
|
|
|||
| if vmStatus != state.Running { | |||
| return createClusterStatusResult(vmStatus, vm.bundle.GetBundleType(), vm.bundle.GetVersion(), "", 0, 0, 0, 0, 0, 0, nil) | |||
There was a problem hiding this comment.
The code is quite cryptic, so an inline comment might be helpful to clearly indicate what happens. description is not enough top determine what the cluster result is.
There was a problem hiding this comment.
network-mode is not even an option on windows and mac, so probably we can merge #4514 and set linux as network-mode user too.
Then we can rework this to detect linux, and network-mode to system. That would clarify why quick early with a madeup cluster is needed as network-mode system needs vm running to work
There was a problem hiding this comment.
The code is quite cryptic, so an inline comment might be helpful to clearly indicate what happens. description is not enough top determine what the cluster result is.
added a inline comment in and also updated the commit description, as earlier it was wrong, the IP is actually needed to get the openshift cluster status
There was a problem hiding this comment.
how about following, this way it doesn't look cryptic and for both network mode it follows same path to get the IP since ram/disksize ..etc doesn't return the error but zero values.
[...]
ip := ""
if vmStatus == state.Running {
ip, err = vm.IP()
if err != nil {
return nil, errors.Wrap(err, "Error getting ip")
}
}
[...]There was a problem hiding this comment.
Thanks, updated the PR!
for crc status command to get the status of the OpenShift cluster we need the IP address of the VM but while using network-mode 'system' and a Stopped VM this fails with: Error getting ip: host is not running this checks if the VM is in Running state before trying to get the VM IP we do not the see the same error for 'user' network-mode as in that case the VM ip is always 127.0.0.1
|
@anjannath: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
for crc status command to get some VM information like disk-usage, pv count, etc. we need to SSH into the VM but this is only possible when the VM is running
in case the state of the VM is not Running it will now return early without trying to fetch those additional information which results in the following error:
Fixes: #4512
Type of change
test, version modification, documentation, etc.)
Proposed changes
Return early from
client.Status()when VM state is notRunningTesting
crc statussucceeds aftercrc stopon linuxContribution Checklist