Stick with go <1.19 #352

zeeke · 2022-08-17T09:44:41Z

Using a dynamic version like '>=1.18.2' may lead to unexpected
build results, with new go tools in place.

This PR should fix errors like this, as they occur due to go 1.19

Run actions/setup-go@v3
Setup go version spec >=1.18.2
Found in cache @ /opt/hostedtoolcache/go/1.19.0/x64
Added go to the path
Successfully set up Go version >=1.1[8](https://github.com/k8snetworkplumbingwg/sriov-network-operator/runs/7839168685?check_suite_focus=true#step:2:9).2
go version go1.1[9](https://github.com/k8snetworkplumbingwg/sriov-network-operator/runs/7839168685?check_suite_focus=true#step:2:10) linux/amd64

github-actions · 2022-08-17T09:44:57Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

SchSeba · 2022-08-17T09:47:50Z

Thanks!

/lgtm

coveralls · 2022-08-17T09:52:24Z

Pull Request Test Coverage Report for Build 2882586772

0 of 0 changed or added relevant lines in 0 files are covered.
7 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.01%) to 15.814%

Files with Coverage Reduction	New Missed Lines	%
api/v1/helper.go	3	34.8%
controllers/sriovibnetwork_controller.go	4	65.35%

Totals
Change from base Build 2841240107:	-0.01%
Covered Lines:	1156
Relevant Lines:	7310

💛 - Coveralls

.github/workflows/go.yml

github-actions · 2022-08-18T08:49:17Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

zeeke · 2022-08-18T08:50:44Z

Retitling as we set go version <1.19

github-actions · 2022-08-18T09:04:37Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

SchSeba · 2022-08-18T09:23:34Z

Thanks!

/lgtm

adrianchiris · 2022-08-18T11:39:13Z

Before i LGTM. are we sure the >1.19 will run latest go 1.18 and will not by some weird circumstances run older version of go ?

e0ne

Thanks for addressing my comments. CI use go 1.18.5 now https://github.com/k8snetworkplumbingwg/sriov-network-operator/runs/7895516472?check_suite_focus=true#step:2:12

SchSeba · 2022-08-18T12:05:54Z

I will always run latest version that is < 1.19

zeeke · 2022-08-18T12:09:50Z

Before i LGTM. are we sure the >1.19 will run latest go 1.18 and will not by some weird circumstances run older version of go ?

Good point. I tried understanding how actions/setup-go actually works:

https://github.com/actions/setup-go/blob/main/src/installer.ts#L58
- https://github.com/actions/toolkit/blob/master/packages/tool-cache/src/tool-cache.ts#L473
  - https://github.com/actions/toolkit/blob/master/packages/tool-cache/src/tool-cache.ts#L671

And it seems to look in a local, repository-shared, cache. So it seems to be possible to get a go1.17 build in some rare case.

Options:
a. stick with 1.8.2 (Pro: We know for sure which version we use in CI, Cons: manual bump of go version)
b. use '>=1.8.2 && < 1.9.0'
c. keep '<1.9.0' and see how often an old go version occurs

(personally, I'd go for a)

SchSeba · 2022-08-18T12:15:18Z

I will prefer to go with b it should not be any change between Z stream of go 1.18

There was a specific issue in 1.18.1 but in general we should not care what Z stream it use

adrianchiris · 2022-08-18T12:18:19Z

i would go with b as well

github-actions · 2022-08-18T12:44:07Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

Go 1.19 has different rules in `gofmt` and may lead to broken CI builds. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

github-actions · 2022-08-18T12:44:43Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

zeeke · 2022-08-18T12:45:19Z

Ok, going for b

go-version: '>=1.18.2 <1.19'

github-actions · 2022-08-18T12:56:08Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

github-actions · 2022-08-18T12:56:52Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

SchSeba

LGTM

Thanks @zeeke and sorry for the ping-pong :)

zeeke mentioned this pull request Aug 17, 2022

Set mod=readonly when installing binaries #351

Merged

github-actions bot added the lgtm label Aug 17, 2022

zeeke mentioned this pull request Aug 17, 2022

Sync master 16 08 22 openshift/sriov-network-operator#686

Merged

e0ne requested changes Aug 17, 2022

View reviewed changes

.github/workflows/go.yml Outdated Show resolved Hide resolved

zeeke force-pushed the go1.18 branch from 36bf9ba to 4fd265b Compare August 18, 2022 08:49

zeeke changed the title ~~Stick with go v1.18.2~~ Stick with go <1.19 Aug 18, 2022

zeeke force-pushed the go1.18 branch from 4fd265b to 64db3b7 Compare August 18, 2022 09:04

SchSeba approved these changes Aug 18, 2022

View reviewed changes

zeeke requested a review from e0ne August 18, 2022 09:41

e0ne approved these changes Aug 18, 2022

View reviewed changes

zeeke force-pushed the go1.18 branch from 64db3b7 to 6d43ac6 Compare August 18, 2022 12:43

Stick with go <1.19

d26b20c

Go 1.19 has different rules in `gofmt` and may lead to broken CI builds. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

zeeke force-pushed the go1.18 branch from 6d43ac6 to d26b20c Compare August 18, 2022 12:44

Increase golang-ci run timeout to 10m

c619386

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

zeeke force-pushed the go1.18 branch from 8d441cc to c619386 Compare August 18, 2022 12:56

SchSeba approved these changes Aug 18, 2022

View reviewed changes

adrianchiris approved these changes Aug 18, 2022

View reviewed changes

adrianchiris merged commit c2149c1 into k8snetworkplumbingwg:master Aug 18, 2022

zeeke mentioned this pull request Mar 13, 2024

Publish container images k8snetworkplumbingwg/sriov-network-metrics-exporter#33

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stick with go <1.19 #352

Stick with go <1.19 #352

zeeke commented Aug 17, 2022

github-actions bot commented Aug 17, 2022

SchSeba commented Aug 17, 2022

coveralls commented Aug 17, 2022 •

edited

Loading

github-actions bot commented Aug 18, 2022

zeeke commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

SchSeba commented Aug 18, 2022

adrianchiris commented Aug 18, 2022

e0ne left a comment

SchSeba commented Aug 18, 2022

zeeke commented Aug 18, 2022

SchSeba commented Aug 18, 2022

adrianchiris commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

zeeke commented Aug 18, 2022 •

edited

Loading

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

SchSeba left a comment

Stick with go <1.19 #352

Stick with go <1.19 #352

Conversation

zeeke commented Aug 17, 2022

github-actions bot commented Aug 17, 2022

SchSeba commented Aug 17, 2022

coveralls commented Aug 17, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2882586772

💛 - Coveralls

github-actions bot commented Aug 18, 2022

zeeke commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

SchSeba commented Aug 18, 2022

adrianchiris commented Aug 18, 2022

e0ne left a comment

Choose a reason for hiding this comment

SchSeba commented Aug 18, 2022

zeeke commented Aug 18, 2022

SchSeba commented Aug 18, 2022

adrianchiris commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

zeeke commented Aug 18, 2022 • edited Loading

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

SchSeba left a comment

Choose a reason for hiding this comment

coveralls commented Aug 17, 2022 •

edited

Loading

zeeke commented Aug 18, 2022 •

edited

Loading