Skip to content
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

Fix the rename command on Windows #1993

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

dardelean
Copy link

The rename command did not work on Windows due to the hostst.Update() function that works only for Linux. The etchosts mechanism on Windows does nothing atm.

Signed-off-by: Dan Ardelean [email protected]

base.Cmd("ps", "-a").AssertOutContains(testContainerName + "_new")
base.Cmd("rename", testContainerName, testContainerName+"_new").AssertFail()
base.Cmd("rename", testContainerName+"_new", testContainerName+"_new").AssertFail()
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this test pass on Linux too?

Copy link
Author

Choose a reason for hiding this comment

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

Changed the test.

@AkihiroSuda AkihiroSuda added the platform/Windows/Non-WSL2 Microsoft Windows (non-WSL2) label Feb 9, 2023
@dardelean dardelean force-pushed the feature/hyperv-containers branch from 5eb3dd2 to 3d5b02b Compare February 10, 2023 08:16
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@dardelean
Copy link
Author

Ping.

@AkihiroSuda AkihiroSuda added this to the v1.2.1 milestone Feb 15, 2023
@AkihiroSuda
Copy link
Member

CI is failing

=== RUN TestRenameHyperVContainer
container_rename_windows_test.go:44: assertion failed: res.ExitCode is not exitCode: time="2023-02-15T13:29:15Z" level=warning msg="default network named "nat" does not have an internal nerdctl ID or nerdctl-managed config file, it was most likely NOT created by nerdctl"
time="2023-02-15T13:29:15Z" level=fatal msg="failed to create shim task: error in preparing config doc: failed to get host processor information: failed to retrieve processor and processor topology information: hcs::GetServiceProperties: Catastrophic failure: unknown"
--- FAIL: TestRenameHyperVContainer (0.44s)

@dardelean dardelean force-pushed the feature/hyperv-containers branch from 3d5b02b to b81ee79 Compare February 16, 2023 11:44
@dcantah
Copy link
Member

dcantah commented Feb 16, 2023

Oh interesting, is the test added here the first hyperv test we have? "failed to retrieve processor and processor topology information: hcs::GetServiceProperties: Catastrophic failure: unknown" This funnily enough, and I had fun finding this in the source, gets thrown if virtualization (or in this case nested virt) isn't on.

@dardelean dardelean force-pushed the feature/hyperv-containers branch from b81ee79 to abb1da2 Compare February 16, 2023 11:59
@dardelean
Copy link
Author

Fixed, added a check to see if HyperV is enabled, if not skip the test.

@AkihiroSuda AkihiroSuda merged commit f4f475f into containerd:main Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/Windows/Non-WSL2 Microsoft Windows (non-WSL2)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants