Update Go to 1.17 #2609
Merged
Update Go to 1.17 #2609
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2609 +/- ##
==========================================
+ Coverage 60.75% 60.78% +0.03%
==========================================
Files 285 285
Lines 23045 23045
==========================================
+ Hits 14000 14009 +9
+ Misses 7575 7570 -5
+ Partials 1470 1466 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
antoninbas
force-pushed
the
update-go-to-1.17
branch
from
8142870
to
75cb68f
Compare
|
/test-all |
antoninbas
force-pushed
the
update-go-to-1.17
branch
from
75cb68f
to
b3065de
Compare
antoninbas
force-pushed
the
update-go-to-1.17
branch
from
b3065de
to
31efeb8
Compare
Go 1.17 has just been released, which means that Go 1.15 will not be actively maintained any more. One notable change in Go 1.17 is that "pruned module graphs" are used: https://golang.org/doc/go1.17#go-command. In Go 1.17, for the go command to correctly resolve transitive imports using the pruned module graph, the go.mod file for each module needs to include more detail about the transitive dependencies relevant to that module. This leads to a second require directive for modules that provide a transitively-imported package. In previous versions, the go.mod file typically only included explicit requirements for directly-imported packages. The following command was used to update the go.mod (as suggested): go-1.17 mod tidy -go=1.16 && go-1.17 mod tidy -go=1.17 I checked that the project could still be built using older Go versions: GO=go-1.15 make bin GO=go-1.16 make bin GO=go-1.17 make bin Another notable change is in the implementation of ParseIP and ParseCIDR from the net package. These functions now reject IPv4 addresses which contain decimal components with leading zeros. K8s is taking some extra steps as they upgrade to Go 1.17 to ensure backwards-compatibility of API resources which include IP fields and for which the contents of the field are validated using the net stdlib functions. They are defining their own versions of these functions, using code from the old version of the standard library. In our case, we have decided not to follow in K8s steps and we are sticking to the Go 1.17 standard library. Our analysis has concluded that there is no reason to preserve past behavior for network policies which use such IPv4 addresses. Note that these policies will still be "valid" resources that users can delete / update. But the Agent will reject the internal version of the policies distributed by the Controller. As part of this change, I also tried to have the current Go version be defined in a "central" location to facilitate future updates. Unfortunately this doesn't apply to Github actions yet, for which the Go version is still hardcoded in multiple places. Fixes antrea-io#2606 Signed-off-by: Antonin Bas <abas@vmware.com>
antoninbas
force-pushed
the
update-go-to-1.17
branch
from
31efeb8
to
2f10111
Compare
|
@tnqn I rebased and need a new approval |
|
/test-all |
tnqn
approved these changes
Aug 24, 2021
abdallahyas
added a commit
to abdallahyas/antrea
that referenced
this pull request
Aug 24, 2021
Following antrea-io#2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI.
abdallahyas
added a commit
to abdallahyas/antrea
that referenced
this pull request
Aug 25, 2021
Following antrea-io#2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI.
abdallahyas
added a commit
to abdallahyas/antrea
that referenced
this pull request
Aug 30, 2021
Following antrea-io#2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI. Signed-off-by: abdallahyas <abdallahyas@mellanox.com>
antoninbas
pushed a commit
that referenced
this pull request
Aug 30, 2021
Following #2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI. Signed-off-by: abdallahyas <abdallahyas@mellanox.com>
antoninbas
pushed a commit
to antoninbas/antrea
that referenced
this pull request
Aug 30, 2021
Following antrea-io#2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI. Signed-off-by: abdallahyas <abdallahyas@mellanox.com>
antoninbas
added a commit
that referenced
this pull request
Aug 30, 2021
Following #2609 `docker build` for the ubuntu image now requires the `GO_VERSION` to be passed, which broke the Mellanox CI. This patch changes the Mellanox CI so that it would use `make build-ubuntu` instead of using the `docker build` command so that all changes to Makefile would not break CI. Signed-off-by: abdallahyas <abdallahyas@mellanox.com> Co-authored-by: abdallahyas <abdallahyas@mellanox.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Go 1.17 has just been released, which means that Go 1.15 will not be
actively maintained any more.
One notable change in Go 1.17 is that "pruned module graphs" are used:
https://golang.org/doc/go1.17#go-command. In Go 1.17, for the go command
to correctly resolve transitive imports using the pruned module graph,
the go.mod file for each module needs to include more detail about the
transitive dependencies relevant to that module. This leads to a second
require directive for modules that provide a transitively-imported
package. In previous versions, the go.mod file typically only included
explicit requirements for directly-imported packages.
The following command was used to update the go.mod (as suggested):
go-1.17 mod tidy -go=1.16 && go-1.17 mod tidy -go=1.17
I checked that the project could still be built using older Go versions:
GO=go-1.15 make bin
GO=go-1.16 make bin
GO=go-1.17 make bin
Another notable change is in the implementation of ParseIP and ParseCIDR
from the net package. These functions now reject IPv4 addresses which
contain decimal components with leading zeros. K8s is taking some extra
steps as they upgrade to Go 1.17 to ensure backwards-compatibility of
API resources which include IP fields and for which the contents of the
field are validated using the net stdlib functions. They are defining
their own versions of these functions, using code from the old version
of the standard library. In our case, we have decided not to follow in
K8s steps and we are sticking to the Go 1.17 standard library. Our
analysis has concluded that there is no reason to preserve past behavior
for network policies which use such IPv4 addresses. Note that these
policies will still be "valid" resources that users can delete /
update. But the Agent will reject the internal version of the policies
distributed by the Controller.
As part of this change, I also tried to have the current Go version be
defined in a "central" location to facilitate future updates.
Unfortunately this doesn't apply to Github actions yet, for which the Go
version is still hardcoded in multiple places.
Fixes #2606
Signed-off-by: Antonin Bas abas@vmware.com