Skip to content

Conversation

@austinvazquez
Copy link
Member

@austinvazquez austinvazquez commented Feb 27, 2024

Issue

N/A

The release/1.6 branch is building with slightly older Go version. This change is bring the Go support more inline with mainline.

Description

Follow-up to #9877. This change backports:

  1. 0f043ae - seccomp, apparmor: add go:noinline
  2. c883410 - upgrade mingw on Windows 2019 GitHub runners
  3. 21640c5 - uninstall mingw before upgrade
  4. 32bd8ef - move inline PS scripts into files
  5. 76d68b0 - container_stat_test.go: avoid checking snapshot size
  6. 82d6c2f - revert container_stats_test.go change which caused Windows CRI integration test failure
  7. 871b6b6 - use testify
  8. a5d9587 - update to go1.21.6, go1.20.13
  9. 488b563 - composite Go action to simplify Go installation across all CI workflows.
  10. 87aa9e8 - updates default Go to Go v1.21.6 and adds Go v1.22.0 to matrix.

Testing

PR runs workflows

@k8s-ci-robot
Copy link

Hi @austinvazquez. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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/test-infra repository.

@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch from 7d4ed6f to 4868bd3 Compare February 27, 2024 16:43
@austinvazquez
Copy link
Member Author

Seeing some integration test failures which is not clear to me. Let me pull out the compiler updates to see if issue persists.

@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch from 4868bd3 to a66b58e Compare February 27, 2024 22:48
@austinvazquez
Copy link
Member Author

There seems to be an issue running the integration tests with the new toolchain on the older platforms.

@akhilerm
Copy link
Member

FYI, there is a go.mod in integration/client. You may want to update that files too.

@akhilerm
Copy link
Member

akhilerm commented Mar 1, 2024

There seems to be an issue running the integration tests with the new toolchain on the older platforms.

@austinvazquez You can cherry-pick 0f043ae to fix the tests

@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch from 94bf584 to 410c66b Compare March 1, 2024 21:13
@austinvazquez
Copy link
Member Author

@austinvazquez You can cherry-pick 0f043ae to fix the tests

Wow @akhilerm great find. I don't think I would have got there without help. The issue sounds complex. Cherry-picked and pushed with latest. Will publish PR if CI looks good.

@akhilerm
Copy link
Member

akhilerm commented Mar 5, 2024

For the failure of tests on windows-2019, you may want to refer to these commits and make the changes acordingly.

@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch 3 times, most recently from c0795f5 to 71b12e0 Compare March 5, 2024 15:21
@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch 2 times, most recently from ba25513 to 6cb5a78 Compare March 5, 2024 15:26
@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch 3 times, most recently from b24addc to 987b464 Compare March 5, 2024 16:18
@austinvazquez
Copy link
Member Author

austinvazquez commented Mar 5, 2024

Fix one failing test two more take its place. 😅

Windows integration tests seem to be in a good place now after a few more backports. Updated the description with the backports needed so far.

Linux integration tests are now failing for:

=== Failed
=== FAIL: sys TestSetNegativeOomScoreAdjustmentWhenUnprivilegedHasNoEffect (0.00s)
    oom_linux_test.go:58: 
        	Error Trace:	/home/runner/work/containerd/containerd/sys/oom_linux_test.go:58
        	Error:      	Not equal: 
        	            	expected: -123
        	            	actual  : 500
        	Test:       	TestSetNegativeOomScoreAdjustmentWhenUnprivilegedHasNoEffect

Good news is all the test are consistently failing with this one error, so there seems to be light at the end of the tunnel. Going to keep chasing it down more.

@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch 3 times, most recently from 19ccda1 to 1f54d6d Compare March 6, 2024 17:19
@austinvazquez
Copy link
Member Author

austinvazquez commented Mar 6, 2024

Hey folks, kicking and screaming this one. 😅

It looks like others are also seeing the OOM integration test failures on release/1.6 and even now showing in mainline. See #9939

@austinvazquez austinvazquez marked this pull request as ready for review March 6, 2024 17:45
@akhilerm
Copy link
Member

akhilerm commented Mar 7, 2024

Can you rebase the PR with release/1.6. That should make the CI go green. We can try to get this merged before #9934.

AkihiroSuda and others added 9 commits March 7, 2024 20:12
Tests in pkg/cri/[sb]server/container_create_linux_test.go depends on go:noinline
since Go 1.21.

e.g.,
> ```
> === FAIL: pkg/cri/sbserver TestGenerateSeccompSecurityProfileSpecOpts/should_set_default_seccomp_when_seccomp_is_runtime/default (0.00s)
>     container_create_linux_test.go:1013:
>         	Error Trace:	/home/runner/work/containerd/containerd/pkg/cri/sbserver/container_create_linux_test.go:1013
>         	Error:      	Not equal:
>         	            	expected: 0x263d880
>         	            	actual  : 0x263cbc0
>         	Test:       	TestGenerateSeccompSecurityProfileSpecOpts/should_set_default_seccomp_when_seccomp_is_runtime/default
> ```

See comments in PR 8957.

Thanks to Wei Fu for analyzing this.

Co-authored-by: Wei Fu <fuweid89@gmail.com>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
(cherry picked from commit 0f043ae)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
The default version of MinGW and GCC on the GitHub-hosted Windows 2019
runners compile fine but lead to linker errors during runtime.

Signed-off-by: Nashwan Azhari <nazhari@cloudbasesolutions.com>
(cherry picked from commit c883410)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
(cherry picked from commit 21640c5)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
(cherry picked from commit 32bd8ef)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
On Linux, the snapshot size differs depending on the backing filesystem.
See issue 7909.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
(cherry picked from commit 76d68b0)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
…ation test failure

PR containerd#7892 which supposed to fix issue on Linux introduced random failure
on Windows, this commit is to revert that change for Windows platform

Signed-off-by: Tony Fang <nenghui.fang@gmail.com>
(cherry picked from commit 82d6c2f)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
(cherry picked from commit 871b6b6)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
go1.21.6 (released 2024-01-09) includes fixes to the compiler, the runtime, and
the crypto/tls, maps, and runtime/pprof packages. See the Go 1.21.6 milestone on
our issue tracker for details:

- https://github.com/golang/go/issues?q=milestone%3AGo1.21.6+label%3ACherryPickApproved
- full diff: golang/go@go1.21.5...go1.21.6

go1.20.13 (released 2024-01-09) includes fixes to the runtime and the crypto/tls
package. See the Go 1.20.13 milestone on our issue tracker for details:

- https://github.com/golang/go/issues?q=milestone%3AGo1.20.13+label%3ACherryPickApproved
- full diff: golang/go@go1.20.12...go1.20.13

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a5d9587)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
(cherry picked from commit 488b563)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch from 1f54d6d to b6e4952 Compare March 7, 2024 20:13
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>
(cherry picked from commit 87aa9e8)
Signed-off-by: Austin Vazquez <macedonv@amazon.com>
@austinvazquez austinvazquez force-pushed the release-1.6-backport-easy-go-install-and-update-go branch from b6e4952 to f6475ea Compare March 7, 2024 20:41
@dmcgowan dmcgowan merged commit e826d58 into containerd:release/1.6 Mar 7, 2024
@austinvazquez austinvazquez deleted the release-1.6-backport-easy-go-install-and-update-go branch March 7, 2024 23:21
Copy link
Member

Choose a reason for hiding this comment

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

The GO_VERSION env needs to be removed from this file.

Copy link
Member Author

@austinvazquez austinvazquez Mar 7, 2024

Choose a reason for hiding this comment

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

Dang, yep that is a miss, let me get a change out to resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.