update to go 1.20 #3843

anjannath · 2023-09-26T08:24:24Z

No description provided.

anjannath · 2023-09-26T08:25:05Z

/hold

cfergeau · 2023-09-26T08:50:41Z

Failure is

GOARCH=amd64 GOOS=windows go build -tags "containers_image_openpgp" -ldflags="-X github.com/crc-org/crc/v2/pkg/crc/version.crcVersion=2.26.0 -X github.com/crc-org/crc/v2/pkg/crc/version.ocpVersion=4.13.9 -X github.com/crc-org/crc/v2/pkg/crc/version.okdVersion=4.13.0-0.okd-2023-06-04-080300 -X github.com/crc-org/crc/v2/pkg/crc/version.podmanVersion=4.4.4 -X github.com/crc-org/crc/v2/pkg/crc/version.microshiftVersion=4.13.9 -X github.com/crc-org/crc/v2/pkg/crc/version.commitSha=46e313 " -o out/windows-amd64/crc.exe  ./cmd/crc
# github.com/crc-org/crc/v2/cmd/crc
C:\hostedtoolcache\windows\go\1.19.13\x64\pkg\tool\windows_amd64\link.exe: running gcc failed: exit status 1
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RUNNER~1\AppData\Local\Temp\go-link-867149281\000007.o: in function `_cgo_preinit_init':
\\_\_\runtime\cgo/gcc_libinit_windows.c:40: undefined reference to `__imp___iob_func'
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RUNNER~1\AppData\Local\Temp\go-link-867149281\000007.o: in function `x_cgo_notify_runtime_init_done':
\\_\_\runtime\cgo/gcc_libinit_windows.c:105: undefined reference to `__imp___iob_func'
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RUNNER~1\AppData\Local\Temp\go-link-867149281\000007.o: in function `_cgo_beginthread':
\\_\_\runtime\cgo/gcc_libinit_windows.c:149: undefined reference to `__imp___iob_func'
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\Users\RUNNER~1\AppData\Local\Temp\go-link-867149281\000008.o: in function `x_cgo_thread_start':
\\_\_\runtime\cgo/gcc_util.c:18: undefined reference to `__imp___iob_func'
collect2.exe: error: ld returned 1 exit status

mingw32-make: *** [Makefile:105: out/windows-amd64/crc.exe] Error 1
Error: Process completed with exit code 1.

anjannath · 2023-09-26T11:45:24Z

/unhold

cfergeau · 2023-09-26T11:55:26Z

If I understand the commit log correctly, mingw is installed by github on the Windows images they offer? We are also using the official actions/setup-go action, so it's very unexpected there is such a breakage.
One thing I noticed is that we use actions/setup-go@v3 which hasn't been updated in a year https://github.com/actions/setup-go/tags
Did you try using actions/setup-go@v4?

cfergeau · 2023-09-26T13:45:48Z

Does not help, but I'd also drop the explicit go: 1.19 requirements, at least for testing.

cfergeau · 2023-09-26T15:44:37Z

The latest push passed gh actions checks.

anjannath · 2023-09-27T02:11:50Z

yeah, if explicit go version is not mentioned, then it works, it seems actions/setup-go just uses the go version already installed (1.20) in the runner (the runners comes preinstalled with a certain version: https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md#language-and-runtime)

actions/setup-go is not really taking care of the C compiler, it just installs the golang package, I think we have to downgrade mingw like before if we want to keep a pinned version of golang, which is what we want, right?

cfergeau · 2023-09-27T07:44:15Z

yeah, if explicit go version is not mentioned, then it works, it seems actions/setup-go just uses the go version already installed (1.20) in the runner

My guess is that 1.20 and 1.21 would work if explicitly mentioned, and stable / oldstable would also work if they map to 1.21 and 1.20

I think we have to downgrade mingw like before if we want to keep a pinned version of golang, which is what we want, right?

In general, yes, we want to make sure crc compiles with the go version we'll be using to build our releases. Regarding the present issue :

golang 1.19 is no longer supported upstream since 1.21 was released this summer
RHEL (which we use for our release builds) has golang 1.20, at least in RHEL 8.8

This seems like a good time to switch to a newer go version :)

pkg/crc/cluster/podman.go

.github/workflows/macos-installer.yml

rand.Seed has been deprecated from go 1.20 it is replaced with rand.NewSource to create a locally scoped random generator

anjannath · 2023-09-28T17:57:23Z

Need to update the go version on openshift ci as well

cfergeau · 2023-09-29T07:35:01Z

Need to update the go version on openshift ci as well

If you feel it's taking too much time, let's just go with the mingw downgrade to fix CI, then you can work at your own pace on the 1.20 update.

praveenkumar · 2023-09-29T11:56:34Z

Need to update the go version on openshift ci as well

created openshift/release#43837

anjannath · 2023-09-30T05:58:33Z

Need to update the go version on openshift ci as well

If you feel it's taking too much time, let's just go with the mingw downgrade to fix CI, then you can work at your own pace on the 1.20 update.

No its fine, i believe once the openshift CI PR is merged, we'll have green CI here as well, and since the choco release job will anyway have to be triggered from the release commit, downgrading will not fix the choco release job for publishing the choco package purpose

praveenkumar · 2023-10-03T06:00:46Z

/retest

openshift-merge-robot · 2023-10-03T12:45:24Z

@anjannath: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/images	`66fb6e0`	link	true	`/test images`
ci/prow/e2e-microshift-crc	`7547612`	link	true	`/test e2e-microshift-crc`
ci/prow/integration-crc	`7547612`	link	true	`/test integration-crc`
ci/prow/e2e-crc	`7547612`	link	true	`/test e2e-crc`

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/test-infra repository. I understand the commands that are listed here.

anjannath · 2023-10-04T11:40:31Z

/test e2e-crc

cfergeau · 2023-10-04T13:26:09Z

pkg/crc/machine/bundle/metadata_test.go

+	// when its a windows path with drive letter
+	if volume := filepath.VolumeName(absPath); volume != "" {
+		return fmt.Sprintf("file://%s/%s", volume[:1], absPath[3:])
+	}


file:///D:/a/crc/crc/pkg/crc/machine/bundle.. is apparently acceptable, see golang/go#6027 , so maybe calling https://pkg.go.dev/path/filepath#ToSlash would be enough to get a functional URI?

It didn't work, even after passing it to filepath.ToSlash the resulting URI is D:\D:\a\crc\crc\pkg\crc\machine\bundle\testdata\sha256sum_correct_4.13.0.txt.sig

filepath.Abs already returns D:\a\crc\crc\pkg\crc\machine\bundle\testdata\sha256sum_correct_4.13.0.txt.sig and when we use the URI file:/// the additional / is again converted to the root volume so we have double D:\D:\

job run: https://github.com/crc-org/crc/actions/runs/6414338773/job/17414568644?pr=3843

This passes on my test Windows VM with:

diff --git a/pkg/crc/machine/bundle/metadata_test.go b/pkg/crc/machine/bundle/metadata_test.go index cff984a98..61134f9b0 100644 --- a/pkg/crc/machine/bundle/metadata_test.go +++ b/pkg/crc/machine/bundle/metadata_test.go @@ -209,5 +209,5 @@ func TestVerifiedHash(t *testing.T) { func testDataURI(t *testing.T, sha256sum string) string { absPath, err := filepath.Abs(filepath.Join("testdata", sha256sum)) require.NoError(t, err) - return fmt.Sprintf("file:///%s", absPath) + return fmt.Sprintf("file://%s", filepath.ToSlash(absPath)) }

(file:/// -> file:// + use of filepath.ToSlash()).
On linux, the test also passes with these changes.

ahh, i think i might have used file:/// instead of file://

cfergeau · 2023-10-04T13:30:57Z

Looks good to me!

this uses filepath.ToSlash function to convert root of the filepath to either slash or windows drive letter it fixes the following unit test failure on windows: ``` === RUN TestVerifiedHash metadata_test.go:214: Error Trace: D:/a/crc/crc/pkg/crc/machine/bundle/metadata_test.go:214 D:/a/crc/crc/pkg/crc/machine/bundle/metadata_test.go:193 Error: Received unexpected error: parse "file://D:\\a\\crc\\crc\\pkg\\crc\\machine\\bundle\\testdata\\sha256sum_correct_4.13.0.txt.sig": invalid port ":\\a\\crc\\crc\\pkg\\crc\\machine\\bundle\\testdata\\sha256sum_correct_4.13.0.txt.sig" after host Test: TestVerifiedHash ```

cfergeau

ACK if CI "passes" for the tests impacted by these changes.

openshift-ci · 2023-10-05T10:46:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cfergeau]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2023-10-05T12:01:07Z

@anjannath: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-crc	`b1ab34f`	link	true	`/test e2e-crc`
ci/prow/integration-crc	`b1ab34f`	link	true	`/test integration-crc`

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/test-infra repository. I understand the commands that are listed here.

This brings go 1.20/1.21 support, but drops go 1.19 support. After crc-org#3843 this should be fine. This fixes this build error with go 1.21: ``` package github.com/crc-org/crc/v2/cmd/crc imports github.com/crc-org/crc/v2/cmd/crc/cmd imports github.com/containers/gvisor-tap-vsock/pkg/virtualnetwork imports github.com/containers/gvisor-tap-vsock/pkg/services/dhcp imports github.com/containers/gvisor-tap-vsock/pkg/tap imports gvisor.dev/gvisor/pkg/tcpip/stack imports gvisor.dev/gvisor/pkg/sync/locking imports gvisor.dev/gvisor/pkg/gohacks: build constraints exclude all Go files in /Users/yevhen/work/redhat/crc/vendor/gvisor.dev/gvisor/pkg/gohacks make: *** [install] Error 1 ```

This brings go 1.20/1.21 support, but drops go 1.19 support. After #3843 this should be fine. This fixes this build error with go 1.21: ``` package github.com/crc-org/crc/v2/cmd/crc imports github.com/crc-org/crc/v2/cmd/crc/cmd imports github.com/containers/gvisor-tap-vsock/pkg/virtualnetwork imports github.com/containers/gvisor-tap-vsock/pkg/services/dhcp imports github.com/containers/gvisor-tap-vsock/pkg/tap imports gvisor.dev/gvisor/pkg/tcpip/stack imports gvisor.dev/gvisor/pkg/sync/locking imports gvisor.dev/gvisor/pkg/gohacks: build constraints exclude all Go files in /Users/yevhen/work/redhat/crc/vendor/gvisor.dev/gvisor/pkg/gohacks make: *** [install] Error 1 ```

openshift-ci bot requested review from gbraad and praveenkumar September 26, 2023 08:24

openshift-ci bot added the do-not-merge/hold label Sep 26, 2023

anjannath force-pushed the gh-win branch from 24eefba to 6733174 Compare September 26, 2023 11:44

anjannath changed the title ~~[test][ghci] downgrade mingw to workaround go build failure~~ downgrade mingw to workaround go build failure Sep 26, 2023

openshift-ci bot removed the do-not-merge/hold label Sep 26, 2023

anjannath force-pushed the gh-win branch from 072fa2b to efb0bb2 Compare September 26, 2023 14:29

anjannath force-pushed the gh-win branch from efb0bb2 to cbd546c Compare September 28, 2023 14:34

anjannath changed the title ~~downgrade mingw to workaround go build failure~~ update to go 1.20 Sep 28, 2023

cfergeau reviewed Sep 28, 2023

View reviewed changes

pkg/crc/cluster/podman.go Show resolved Hide resolved

praveenkumar reviewed Sep 28, 2023

View reviewed changes

.github/workflows/macos-installer.yml Outdated Show resolved Hide resolved

anjannath force-pushed the gh-win branch from 235bf28 to ba109ed Compare September 28, 2023 15:52

remove use of rand.seed

885a833

rand.Seed has been deprecated from go 1.20 it is replaced with rand.NewSource to create a locally scoped random generator

anjannath force-pushed the gh-win branch from ba109ed to 0c8ea19 Compare September 28, 2023 16:16

anjannath force-pushed the gh-win branch 3 times, most recently from 99e2441 to 7547612 Compare October 3, 2023 12:23

anjannath force-pushed the gh-win branch 3 times, most recently from 9c0b8a6 to 473a41c Compare October 4, 2023 10:59

cfergeau reviewed Oct 4, 2023

View reviewed changes

anjannath added 2 commits October 5, 2023 08:56

update to go 1.20

7810b5b

update to actions/setup-go@v

d241e3f

anjannath force-pushed the gh-win branch 3 times, most recently from 0f82789 to 56cc93d Compare October 5, 2023 08:24

anjannath force-pushed the gh-win branch from 56cc93d to b1ab34f Compare October 5, 2023 10:31

cfergeau approved these changes Oct 5, 2023

View reviewed changes

openshift-ci bot assigned cfergeau Oct 5, 2023

openshift-ci bot added the lgtm label Oct 5, 2023

openshift-ci bot added the approved label Oct 5, 2023

anjannath merged commit fedda02 into crc-org:main Oct 6, 2023
13 checks passed

cfergeau mentioned this pull request Oct 9, 2023

go.mod: Update gvisor-tap-vsock to latest release #3861

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update to go 1.20 #3843

update to go 1.20 #3843

anjannath commented Sep 26, 2023 •

edited

Loading

anjannath commented Sep 26, 2023

cfergeau commented Sep 26, 2023

anjannath commented Sep 26, 2023

cfergeau commented Sep 26, 2023

cfergeau commented Sep 26, 2023

cfergeau commented Sep 26, 2023

anjannath commented Sep 27, 2023

cfergeau commented Sep 27, 2023

anjannath commented Sep 28, 2023

cfergeau commented Sep 29, 2023

praveenkumar commented Sep 29, 2023

anjannath commented Sep 30, 2023 •

edited

Loading

praveenkumar commented Oct 3, 2023

openshift-merge-robot commented Oct 3, 2023

anjannath commented Oct 4, 2023

cfergeau Oct 4, 2023

anjannath Oct 5, 2023 •

edited

Loading

cfergeau Oct 5, 2023

anjannath Oct 5, 2023

cfergeau commented Oct 4, 2023

cfergeau left a comment

openshift-ci bot commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

update to go 1.20 #3843

update to go 1.20 #3843

Conversation

anjannath commented Sep 26, 2023 • edited Loading

anjannath commented Sep 26, 2023

cfergeau commented Sep 26, 2023

anjannath commented Sep 26, 2023

cfergeau commented Sep 26, 2023

cfergeau commented Sep 26, 2023

cfergeau commented Sep 26, 2023

anjannath commented Sep 27, 2023

cfergeau commented Sep 27, 2023

anjannath commented Sep 28, 2023

cfergeau commented Sep 29, 2023

praveenkumar commented Sep 29, 2023

anjannath commented Sep 30, 2023 • edited Loading

praveenkumar commented Oct 3, 2023

openshift-merge-robot commented Oct 3, 2023

anjannath commented Oct 4, 2023

cfergeau Oct 4, 2023

Choose a reason for hiding this comment

anjannath Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

cfergeau Oct 5, 2023

Choose a reason for hiding this comment

anjannath Oct 5, 2023

Choose a reason for hiding this comment

cfergeau commented Oct 4, 2023

cfergeau left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Oct 5, 2023

openshift-ci bot commented Oct 5, 2023

anjannath commented Sep 26, 2023 •

edited

Loading

anjannath commented Sep 30, 2023 •

edited

Loading

anjannath Oct 5, 2023 •

edited

Loading