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

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

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

cfergeau
Copy link
Contributor

@cfergeau cfergeau commented Oct 9, 2023

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

Fixes: Issue #N

Relates to: Issue #N, PR #N, ...

Solution/Idea

Describe in plain English what you solved and how. For instance, Added start command to CRC so the user can create a VM and set-up a single-node OpenShift cluster on it with one command. It requires blablabla...

Proposed changes

List main as well as consequential changes you introduced or had to introduce.

  1. Add start command.
  2. Add setup as prerequisite to start.
  3. ...

Testing

What is the bottom-line functionality that needs testing? Describe in pseudo-code or in English. Use verifiable statements that tie your changes to existing functionality.

  1. start succeeds first time after setup succeeded
  2. stdout contains ... if start succeeded
  3. stderr contains ... after start failed
  4. status returns ... if start succeeded
  5. status returns ... if start failed
  6. start fails after start succeeded or after status says CRC is Running
  7. ...

@praveenkumar
Copy link
Member

/lgtm
/approve

@openshift-ci
Copy link

openshift-ci bot commented Oct 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: praveenkumar

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:

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

@openshift-ci openshift-ci bot added the approved label Oct 9, 2023
@praveenkumar
Copy link
Member

/hold

It is failing for e2e with following error.

 go install -tags "containers_image_openpgp"  -ldflags="-X github.com/crc-org/crc/v2/pkg/crc/version.crcVersion=2.27.0 -X github.com/crc-org/crc/v2/pkg/crc/version.ocpVersion=4.13.12 -X github.com/crc-org/crc/v2/pkg/crc/version.okdVersion=4.13.0-0.okd-2023-09-03-082426 -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.12 -X github.com/crc-org/crc/v2/pkg/crc/version.commitSha=70fa0a "  ./cmd/crc
# gvisor.dev/gvisor/pkg/tcpip
vendor/gvisor.dev/gvisor/pkg/tcpip/tcpip.go:224:23: cannot convert a.addr[:4] (value of type []byte) to type [4]byte
vendor/gvisor.dev/gvisor/pkg/tcpip/tcpip.go:232:24: cannot convert a.addr[:16] (value of type []byte) to type [16]byte
note: module requires Go 1.20 

@cfergeau
Copy link
Contributor Author

/hold

It is failing for e2e with following error.

 go install -tags "containers_image_openpgp"  -ldflags="-X github.com/crc-org/crc/v2/pkg/crc/version.crcVersion=2.27.0 -X github.com/crc-org/crc/v2/pkg/crc/version.ocpVersion=4.13.12 -X github.com/crc-org/crc/v2/pkg/crc/version.okdVersion=4.13.0-0.okd-2023-09-03-082426 -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.12 -X github.com/crc-org/crc/v2/pkg/crc/version.commitSha=70fa0a "  ./cmd/crc
# gvisor.dev/gvisor/pkg/tcpip
vendor/gvisor.dev/gvisor/pkg/tcpip/tcpip.go:224:23: cannot convert a.addr[:4] (value of type []byte) to type [4]byte
vendor/gvisor.dev/gvisor/pkg/tcpip/tcpip.go:232:24: cannot convert a.addr[:16] (value of type []byte) to type [16]byte
note: module requires Go 1.20 

With this change, we need go 1.20 or 1.21 everywhere

@praveenkumar
Copy link
Member

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

@cfergeau
Copy link
Contributor Author

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

Seeing the e2e error, there must be one location which still has 1.19 or older.

@cfergeau
Copy link
Contributor Author

cfergeau commented Oct 10, 2023

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

Seeing the e2e error, there must be one location which still has 1.19 or older.

It's microshift e2e which is failing, and the logs show:

 Package golang-1.19.10-1.el9_2.x86_64 is already installed. 

@cfergeau
Copy link
Contributor Author

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

Seeing the e2e error, there must be one location which still has 1.19 or older.

It's microshift e2e which is failing, and the logs show:

 Package golang-1.19.10-1.el9_2.x86_64 is already installed. 

Must be coming from https://github.com/openshift/release/blob/2b5d87adbb1d3922afdd051c7a489d03057a8376/ci-operator/step-registry/code-ready/crc/microshift/test/code-ready-crc-microshift-test-commands.sh#L83-L86

@praveenkumar
Copy link
Member

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

Seeing the e2e error, there must be one location which still has 1.19 or older.

It's microshift e2e which is failing, and the logs show:

 Package golang-1.19.10-1.el9_2.x86_64 is already installed. 

Must be coming from https://github.com/openshift/release/blob/2b5d87adbb1d3922afdd051c7a489d03057a8376/ci-operator/step-registry/code-ready/crc/microshift/test/code-ready-crc-microshift-test-commands.sh#L83-L86

you mean during https://github.com/openshift/release/blob/2b5d87adbb1d3922afdd051c7a489d03057a8376/ci-operator/step-registry/code-ready/crc/microshift/test/code-ready-crc-microshift-test-commands.sh#L46C3-L46C28 part because the crc binary already built using container and copied to VM but we do build e2e binary.

@cfergeau
Copy link
Contributor Author

With this change, we need go 1.20 or 1.21 everywhere

@cfergeau we already have go-1.20 everywhere as of now, even the openshift CI side?

Seeing the e2e error, there must be one location which still has 1.19 or older.

It's microshift e2e which is failing, and the logs show:

 Package golang-1.19.10-1.el9_2.x86_64 is already installed. 

Must be coming from https://github.com/openshift/release/blob/2b5d87adbb1d3922afdd051c7a489d03057a8376/ci-operator/step-registry/code-ready/crc/microshift/test/code-ready-crc-microshift-test-commands.sh#L83-L86

you mean during https://github.com/openshift/release/blob/2b5d87adbb1d3922afdd051c7a489d03057a8376/ci-operator/step-registry/code-ready/crc/microshift/test/code-ready-crc-microshift-test-commands.sh#L46C3-L46C28 part because the crc binary already built using container and copied to VM but we do build e2e binary.

Yes, would be best to build everything on the same machine, and only copy pre-built binaries to gcloud.

@anjannath anjannath added the has-to-be-in-release This PR need to go in coming release. label Oct 16, 2023
@openshift-ci openshift-ci bot removed the lgtm label Oct 16, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 16, 2023

New changes are detected. LGTM label has been removed.

@anjannath anjannath removed the has-to-be-in-release This PR need to go in coming release. label Oct 16, 2023
@anjannath
Copy link
Member

anjannath commented Oct 16, 2023

(removed has-to-be-in-release as brew nodes are having go 1.20 which is able to build without the issue this PR was fixing which happens with go 1.21)

@cfergeau cfergeau force-pushed the gomod branch 2 times, most recently from 1b66c1f to 820710a Compare October 16, 2023 13:26
@cfergeau
Copy link
Contributor Author

I created openshift/release@master...cfergeau:release:prebuilt and updated this PR, but I don't know what is the best way forward for this openshift-release PR. Just file it?

Makefile Outdated
@@ -235,9 +235,6 @@ endif
ifndef CRC_BINARY
CRC_BINARY = --crc-binary=$(GOPATH)/bin
endif
ifndef VERSION_TO_TEST
VERSION_TO_TEST = --crc-version=$(CRC_VERSION)+$(COMMIT_SHA)
endif
e2e:
@go test --timeout=180m $(MODULEPATH)/test/e2e -tags "$(BUILDTAGS)" --ldflags="$(VERSION_VARIABLES)" -v $(PULL_SECRET_FILE) $(BUNDLE_LOCATION) $(CRC_BINARY) $(GODOG_OPTS) $(CLEANUP_HOME) $(VERSION_TO_TEST) $(INSTALLER_PATH) $(USER_PASSWORD)
Copy link
Member

Choose a reason for hiding this comment

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

should we remove $(VERSION_TO_TEST) as part of e2e target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there is a ifndef VERSION_TO_TEST check, the code assumes VERSION_TO_TEST can be overridden externally (eg VERSION_TO_TEST="--crc-version=0.1.2" make e2e).
If we keep it, it will expand to the empty string if unset, which is fine, and the previous behaviour will be unchanged if it's set.
Imo it's better to keep it.

@cfergeau
Copy link
Contributor Author

I'm going to split this, first the CI changes, once they are merged, we can merge the corresponding openshift/release changes, and then this gvisor-tap-vsock PR.

@cfergeau
Copy link
Contributor Author

I'm going to split this, first the CI changes, once they are merged, we can merge the corresponding openshift/release changes, and then this gvisor-tap-vsock PR.

Created #3876 with all the changes from this PR except for the gvisor-tap-vsock update.

@praveenkumar
Copy link
Member

openshift/release#44572 is created on openshift/release side since #3876 merged

@praveenkumar
Copy link
Member

/retest

@cfergeau
Copy link
Contributor Author

crc: /lib64/libc.so.6: version `GLIBC_2.33' not found (required by crc)
crc: /lib64/libc.so.6: version `GLIBC_2.32' not found (required by crc)
crc: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by crc)

CI fails because of this now :-/

@cfergeau
Copy link
Contributor Author

crc: /lib64/libc.so.6: version `GLIBC_2.33' not found (required by crc)
crc: /lib64/libc.so.6: version `GLIBC_2.32' not found (required by crc)
crc: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by crc)

CI fails because of this now :-/

I'll try to use registry.ci.openshift.org/openshift/release:rhel-8-release-golang-1.20-openshift-4.15 instead of registry.ci.openshift.org/openshift/release:rhel-9-release-golang-1.20-openshift-4.15 (RHEL8 based instead of RHEL9 based) and see if this helps.

There is no version of registry.access.redhat.com/ubi8/go-toolset with
golang 1.20:
```
$ podman build -f ./images/build-e2e/Dockerfile .
[1/2] STEP 1/5: FROM registry.access.redhat.com/ubi8/go-toolset:1.20 AS builder
Trying to pull registry.access.redhat.com/ubi8/go-toolset:1.20...
Error: creating build container: initializing source docker://registry.access.redhat.com/ubi8/go-toolset:1.20: reading manifest 1.20 in registry.access.redhat.com/ubi8/go-toolset: manifest unknown
```

This commit switches to
registry.ci.openshift.org/openshift/release:rhel-8-release-golang-1.20-openshift-4.15
which has golang 1.20 and is usable without authentication.
This stays on a rhel8 image as some of the test nodes are still using
rhel8, and building on a rhel9 node, and trying to run on a rhel8 node
is not possible (some libc linking issues at startup).

The builds will be noisy until
openshift-eng/art-tools#115 is merged and gets
into these images.
If needed, we can set GO_COMPLIANCE_INFO=0 in the images environment to
make the build more quiet.
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
```
@praveenkumar
Copy link
Member

/retest

1 similar comment
@praveenkumar
Copy link
Member

/retest

@openshift-ci
Copy link

openshift-ci bot commented Oct 26, 2023

@cfergeau: The following test 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 28da406 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.

@praveenkumar praveenkumar merged commit e2ac7b5 into crc-org:main Oct 26, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants