Skip to content

[release/0.11] Cherry-pick testing fixes from v0.10.0-rc.9 and update containerd version used #1881

Merged
helsaawy merged 5 commits intomicrosoft:release/0.11from
kiashok:updateCtrdTag
Sep 20, 2023
Merged

[release/0.11] Cherry-pick testing fixes from v0.10.0-rc.9 and update containerd version used #1881
helsaawy merged 5 commits intomicrosoft:release/0.11from
kiashok:updateCtrdTag

Conversation

@kiashok
Copy link
Contributor

@kiashok kiashok commented Sep 1, 2023 •
edited
Loading

This PR makes the following changes:

  1. Checkout appropriate containerd ref #1752 (not exact cherry-pick. Makes only ci.yml changes)
  2. skip failing test, use gotestsum #1820 (exact cherry-pick)
  3. [ci] Fix integration test failure #1799 (exact cherry-pick)
  4. Update containerd tag in hcsshim branch to v1.6.23 + update google.golang.org/grpc v1.50.1 to exactly match what containerd v1.6.23 is using. Confirmed by checking there are no unnecessary changes to protobuf/grpc etc in upstream containerd if these hcsshim changes are vendored in)
  5. Cleans up test that are skipped in CI:

kiashok and others added 4 commits September 18, 2023 12:58
@kiashok
update to latest containerd/1.6 tag v1.6.23
f7e3184 
Signed-off-by: Kirtana Ashok <[email protected]>
@helsaawy @kiashok
fix integration test failure (microsoft#1799)
98776c3 
Signed-off-by: Hamza El-Saawy <[email protected]>
(cherry picked from commit f5c5797)
Signed-off-by: Kirtana Ashok <[email protected]>
@kiashok
Checkout appropriate containerd ref
cc35b4b 
Signed-off-by: Kirtana Ashok <[email protected]>
@helsaawy @kiashok
skip failing test, use gotestsum (microsoft#1820)
bc58c27 
Signed-off-by: Hamza El-Saawy <[email protected]>
(cherry picked from commit 4ede1fd)
Signed-off-by: Kirtana Ashok <[email protected]>
@kiashok kiashok changed the title [release/0.11] Update containerd version used [release/0.11] Cherry-pick testing fixes from v0.10.0-rc.9 and update containerd version used Sep 18, 2023
@kiashok
Copy link
Contributor Author

kiashok commented Sep 18, 2023 •
edited
Loading

The only tests failing are the symlink failures which was automatically fixed when we moved hcsshim/main to containerd/1.7 . Should we skip this failing test for hcsshim/release/0.11 @helsaawy @katiewasnothere @anmaxvl ?
If its ok, I will update the PR with that commit as well.

Just to add - bc58c27 this commit that this PR has can be removed too as the TODO comment has made it to containerd v1.6.22 and updating containerd tag to v1.6.23 will cover this. @helsaawy is this ok to do?

@kiashok kiashok marked this pull request as ready for review September 18, 2023 20:05
@kiashok kiashok requested a review from a team as a code owner September 18, 2023 20:05
@kiashok kiashok mentioned this pull request Sep 18, 2023
Closed
@kiashok
Copy link
Contributor Author

kiashok commented Sep 18, 2023

cc @helsaawy @anmaxvl @katiewasnothere @kevpar

@helsaawy
Copy link
Contributor

helsaawy commented Sep 18, 2023

I skipped the symlink tests in #1891 as well (FOCUS="[^(TestContainerdRestart|TestContainerSymlinkVolumes)]" make cri-integration), so we might as well do that here too

@helsaawy
Copy link
Contributor

helsaawy commented Sep 18, 2023

Just to add - bc58c27 this commit that this PR has can be removed too as the TODO comment has made it to containerd v1.6.22 and updating containerd tag to v1.6.23 will cover this. @helsaawy is this ok to do?

I'd prefer that commit makes it in, since the gotestsum integration is pretty handy for looking at failures, but we can have another PR that upgrades to 1.6.23 and removes the skip?

@kiashok
Copy link
Contributor Author

kiashok commented Sep 18, 2023

Just to add - bc58c27 this commit that this PR has can be removed too as the TODO comment has made it to containerd v1.6.22 and updating containerd tag to v1.6.23 will cover this. @helsaawy is this ok to do?

I'd prefer that commit makes it in, since the gotestsum integration is pretty handy for looking at failures, but we can have another PR that upgrades to 1.6.23 and removes the skip?

Sure I can do that. The last commit has work to upgrade the containerd tag to v1.6.23 - do you want me to break it out to a separate PR? There are some failures due to go version if the version is not upgraded. Is it ok if I just add the change to remove TestConvert and add change to skip TestSymLink as an additional commit to this PR so that we have all the tests passing?

thaJeztah
thaJeztah approved these changes Sep 19, 2023
View reviewed changes
Copy link
Contributor

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

changes LGTM

Looks like the TestContainerSymlinkVolumes failures were known (failing both on Windows 2022 and Windows 2019)

I wish these hcsshim errors were more informative 😂 (looking at you, The parameter is incorrect.: unknown 😂)

Details 
--- FAIL: TestContainerSymlinkVolumes (22.97s)
    --- FAIL: TestContainerSymlinkVolumes/file_in_symlinked_folder (7.66s)
    --- FAIL: TestContainerSymlinkVolumes/symlinked_file (7.66s)
    --- FAIL: TestContainerSymlinkVolumes/symlinkedFileInSymlinkedFolder (7.65s)

=== RUN   TestContainerSymlinkVolumes
=== RUN   TestContainerSymlinkVolumes/file_in_symlinked_folder
    container_volume_test.go:95: Create test sandbox with log directory
    common.go:120: Image "k8s.gcr.io/e2e-test-images/busybox:1.29-2" already exists, not pulling.
    container_volume_test.go:107: Create a container with a symlink volume mount
    container_volume_test.go:119: Start the container
E0918 23:22:12.723157    7180 remote_runtime.go:270] StartContainer "033cb6329f8068757914ceaf206147741be390a679d35fd41cab07d7bdcd5651" from runtime service failed: rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 033cb6329f8068757914ceaf206147741be390a679d35fd41cab07d7bdcd5651: The parameter is incorrect.: unknown
    container_volume_test.go:120: 
        	Error Trace:	D:\a\hcsshim\hcsshim\src\github.com\containerd\containerd\container_volume_test.go:120
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 033cb6329f8068757914ceaf206147741be390a679d35fd41cab07d7bdcd5651: The parameter is incorrect.: unknown
        	Test:       	TestContainerSymlinkVolumes/file_in_symlinked_folder
=== RUN   TestContainerSymlinkVolumes/symlinked_file
    container_volume_test.go:95: Create test sandbox with log directory
    common.go:120: Image "k8s.gcr.io/e2e-test-images/busybox:1.29-2" already exists, not pulling.
    container_volume_test.go:107: Create a container with a symlink volume mount
    container_volume_test.go:119: Start the container
E0918 23:22:20.383430    7180 remote_runtime.go:270] StartContainer "23b45f287a3f09641e1d098d9686403a1ae3eb3fed81f884370b02acd2e307bd" from runtime service failed: rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 23b45f287a3f09641e1d098d9686403a1ae3eb3fed81f884370b02acd2e307bd: The parameter is incorrect.: unknown
    container_volume_test.go:120: 
        	Error Trace:	D:\a\hcsshim\hcsshim\src\github.com\containerd\containerd\container_volume_test.go:120
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 23b45f287a3f09641e1d098d9686403a1ae3eb3fed81f884370b02acd2e307bd: The parameter is incorrect.: unknown
        	Test:       	TestContainerSymlinkVolumes/symlinked_file
=== RUN   TestContainerSymlinkVolumes/symlinkedFileInSymlinkedFolder
    container_volume_test.go:95: Create test sandbox with log directory
    common.go:120: Image "k8s.gcr.io/e2e-test-images/busybox:1.29-2" already exists, not pulling.
    container_volume_test.go:107: Create a container with a symlink volume mount
    container_volume_test.go:119: Start the container
E0918 23:22:28.032554    7180 remote_runtime.go:270] StartContainer "21c8586a7ce8ab162cf37c644a6c22da3ac7987035e3f1213a5a5f25a828dcb8" from runtime service failed: rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 21c8586a7ce8ab162cf37c644a6c22da3ac7987035e3f1213a5a5f25a828dcb8: The parameter is incorrect.: unknown
    container_volume_test.go:120: 
        	Error Trace:	D:\a\hcsshim\hcsshim\src\github.com\containerd\containerd\container_volume_test.go:120
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unknown desc = failed to create containerd task: failed to create shim task: hcs::CreateComputeSystem 21c8586a7ce8ab162cf37c644a6c22da3ac7987035e3f1213a5a5f25a828dcb8: The parameter is incorrect.: unknown
        	Test:       	TestContainerSymlinkVolumes/symlinkedFileInSymlinkedFolder

@thaJeztah thaJeztah mentioned this pull request Sep 19, 2023
Closed
@kiashok
Cleanup tests that are skipped in CI:
3865825 
Removes TestConvert() from being skipped as corresponding fix made to v1.6.22
Skip TestContainerSymlinkVolumes() tests as it seems to pass only with containerd/1.7

Signed-off-by: Kirtana Ashok <[email protected]>
@kiashok
Copy link
Contributor Author

kiashok commented Sep 20, 2023

All the tests are now passing

@helsaawy helsaawy merged commit 9c3272e into microsoft:release/0.11 Sep 20, 2023
kiashok added a commit that referenced this pull request Sep 20, 2023
@kiashok
Revert "[release/0.11] Cherry-pick testing fixes from v0.10.0-rc.9 an…
917dd5a 
…d update containerd version used (#1881)"

This reverts commit 9c3272e.
@kiashok kiashok mentioned this pull request Sep 20, 2023
Merged
kiashok added a commit that referenced this pull request Sep 20, 2023
@kiashok
Revert "[release/0.11] Cherry-pick testing fixes from v0.10.0-rc.9 an…
cba2f73 
…d update containerd version used (#1881)" (#1911)

This reverts commit 9c3272e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anmaxvl anmaxvl anmaxvl approved these changes

+1 more reviewer

@thaJeztah thaJeztah thaJeztah approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kiashok @helsaawy @thaJeztah @anmaxvl