Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Fixed Hyper-V VM remaining in invalid state #4012

Merged
merged 1 commit into from
Mar 21, 2019
Merged

Conversation

Wiezzel
Copy link

@Wiezzel Wiezzel commented Mar 15, 2019

If creating the machine failed during virtual disk creation then the VM would remain in a an invalid state (actually not the VM but some config files used by docker-machine). This in turn would make it impossible to re-create the VM.
The error had been unsuccessfully mitigated against by adding a remove command in HyperVHypervisor._failed_to_create() method. This was not effective because removing the VM sometimes fails if it done shortly after the unsuccessful creation.
To make the cleanup more robust a retry loop was added. It required changing the logic of DockerMachineHypervisor.vms() because docker-machine remove -q doesn't print out VMs in invalid states.

If creating the machine failed during virtual disk creation then the VM
would remain in a an invalid state (actually not the VM but some config
files used by docker-machine). This in turn would make it impossible to
re-create the VM.
The error was unsuccessfully mitigated against by adding a remove
command in HyperVHypervisor._failed_to_create() method. This was not
effective because removing the VM sometimes fails if it done shortly
after the unsuccessful creation.
To make the cleanup more robust a retry loop was added. It required
changing the logic of DockerMachineHypervisor.vms() because `docker-
machine remove -q` doesn't print out VMs in invalid states.
Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

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

Nice find on the -q!

Sad the retry is needed but good its more robust now

@codecov
Copy link

codecov bot commented Mar 15, 2019

Codecov Report

Merging #4012 into b0.19 will increase coverage by 0.06%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##            b0.19    #4012      +/-   ##
==========================================
+ Coverage   89.18%   89.25%   +0.06%     
==========================================
  Files         205      205              
  Lines       18414    18422       +8     
==========================================
+ Hits        16423    16443      +20     
+ Misses       1991     1979      -12

f'machine "{name}" failed')
time.sleep(0.1)

else:
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this always trigger on the last iteration?

Copy link
Author

Choose a reason for hiding this comment

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

That was the idea. If all the iterations passed then we assume that removing the VM failed. If it succeeded then break would be triggered and else clause wouldn't be executed.

@shadeofblue shadeofblue merged commit c025899 into b0.19 Mar 21, 2019
@shadeofblue shadeofblue deleted the vm_remove_retry branch March 21, 2019 11:55
@ghost ghost removed the in progress label Mar 21, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants