Update Golang to 1.12.1 #11330

kvch · 2019-03-20T09:02:13Z

Changes in 1.12: https://golang.org/doc/go1.12
Changes in minor release: https://github.com/golang/go/issues?q=milestone%3AGo1.12.1

Updated vendored packages:

github.com/tsg/gopacket/pcap
github.com/docker/docker/pkg/*
github.com/docker/go-connections

New vendored packages (as required by dependencies):

github.com/konsorten/go-windows-terminal-sequences

Removed vendored package:

github.com/Sirupsen/logrus: github.com/sirupsen/logrus is used and it leads to a case insensitive collision, required by docker dependencies

TODO

use documented Cgo types in github.com/tsg/gopacket/pcap Cgo changes
fix recovering from panic in logp

Possibly interesting PR to devs @elastic/apm-server

ruflin · 2019-03-20T09:14:50Z

Went through the changelog and also stumbled over TLS changes: https://golang.org/doc/go1.12 @ph I don't think it will have an affect but you should double check.

Overall +1 on going to 1.12.

kvch · 2019-03-20T09:22:05Z

@ruflin I would like to add support for TLS 1.3

kvch · 2019-03-21T09:46:45Z

metricbeat/Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.10.3


@ruflin Do you know why the version of Golang differed from other Beats versions? Is it ok to update it?
I haven't seen any related failures yet.

This is odd, all Beats should be in sync :-( I assume this had only an affect on testing and not packaging, meaning in 7.0 all Beats are packaged with the same Golang version.

Please update, I assume we missed it in the last Golang update.

It was downgraded here: https://github.com/elastic/beats/pull/9242/files#diff-69f23758930df5eda4eeb23437b8a2ccR1

It seems like an accidental change. Or at least I haven't seen any reasoning why it was done.

@kvch Thanks for investigation. @sayden I left a comment on the old PR, I wonder if any of these changes should have happened there.

It probably was a merge gone bad.

@sayden Can you please open a PR with reverting to the old Dockerfile?

ph · 2019-03-21T12:34:02Z

We might want to look at using this:

The compiler now accepts a -lang flag to set the Go language version to use. For example, -lang=go1.8 causes the compiler to emit an error if the program uses type aliases, which were added in Go 1.9. Language changes made before Go 1.12 are not consistently enforced.

ph · 2019-03-21T12:35:22Z

On Linux, the runtime now uses MADV_FREE to release unused memory. This is more efficient but may result in higher reported RSS. The kernel will reclaim the unused data when it is needed. To revert to the Go 1.11 behavior (MADV_DONTNEED), set the environment variable GODEBUG=madvdontneed=1.

Something to keep in mind on SDH

libbeat/logp/core_test.go

ph · 2019-03-21T12:52:35Z

Looking at the TLSv1.3, from my understanding you need to opt-in via an environment variables, not sure if can force it via a TLSConfig.

graphaelli · 2019-03-21T14:07:53Z

While it hasn't been kept up so far, should the various golang.org/x packages be kept in sync with this update too? These are currently vendored:

golang.org/x/net/bpf@release-branch.go1.9
golang.org/x/net/context@release-branch.go1.9
golang.org/x/net/context/ctxhttp@release-branch.go1.9
golang.org/x/net/http2@release-branch.go1.9
golang.org/x/net/http2/hpack@release-branch.go1.9
golang.org/x/net/icmp@release-branch.go1.9
golang.org/x/net/internal/iana@release-branch.go1.9
golang.org/x/net/ipv4@release-branch.go1.9
golang.org/x/net/ipv6@release-branch.go1.9
golang.org/x/net/lex/httplex@release-branch.go1.9
golang.org/x/net/proxy@release-branch.go1.9
golang.org/x/net/publicsuffix@release-branch.go1.9
golang.org/x/tools/cmd/goimports@release-branch.go1.10
golang.org/x/tools/go/ast/astutil@release-branch.go1.10
golang.org/x/tools/go/buildutil@release-branch.go1.9
golang.org/x/tools/go/loader@release-branch.go1.9
golang.org/x/tools/imports@release-branch.go1.10
golang.org/x/tools/internal/fastwalk@release-branch.go1.10
golang.org/x/tools/internal/gopathwalk@release-branch.go1.10

ph · 2019-03-21T14:17:19Z

@graphaelli correct, I think we were just updating them as needed or when a bug was detected (mostly following what ain't broke don't fix it..). I think we should clarify that strategy and I would be +1 to keep them in sync. Wdyt @ruflin?

kvch · 2019-03-21T15:56:40Z

@ph We could document that adding the environment variable lets you use TLSv3. But it's not so user friendly. Or we could add the feature regardless and release it officially when we update Go to 1.13. :D

@graphaelli @ph 👍 for keeping these packages in sync

ruflin · 2019-03-22T06:46:31Z

+1 to keep the libs in sync.

ruflin · 2019-03-22T06:47:24Z

vendor/github.com/tsg/gopacket/pcap/pcap.go

@@ -170,7 +170,7 @@ type InterfaceAddress struct {
 // BPF is a compiled filter program, useful for offline packet matching.
 type BPF struct {
 	orig string
-	bpf  _Ctype_struct_bpf_program // takes a finalizer, not overriden by outsiders
+	bpf  C.struct_bpf_program // takes a finalizer, not overriden by outsiders


Not really familiar with this change. @tsg could you have a look?

He merged the PR in his own repo. So he already accepted it. :)

ruflin · 2019-03-22T06:48:23Z

vendor/github.com/tsg/gopacket/pcap/pcap_tester.go

-
-// This binary tests that PCAP packet capture is working correctly by issuing
-// HTTP requests, then making sure we actually capture data off the wire.
-package main


Is the removal of this file related to 1.12.1?

It was removed by govendor when I ran fetch. I thought it was removed from the original repo... But now I see that it's still there. Let me look into why the tool got rid of this file.

The reason could be that its package main. My first gest was that it's a test file, but it's a script.

ruflin · 2019-03-25T08:21:56Z

CI Failure looks related.

CHANGELOG.next.asciidoc

jordansissel · 2019-04-17T04:08:15Z

Can this be backported into backport into 7.0/7.x? I only see it in master. (Context: I ran into this tonight)

Changes in 1.12: https://golang.org/doc/go1.12 Changes in minor release: https://github.com/golang/go/issues?q=milestone%3AGo1.12.1 Updated vendored packages: - `github.com/tsg/gopacket/pcap` - `github.com/docker/docker/pkg/*` - `github.com/docker/go-connections` New vendored packages (as required by dependencies): - `github.com/konsorten/go-windows-terminal-sequences` Removed vendored package: - `github.com/Sirupsen/logrus`: `github.com/sirupsen/logrus` is used and it leads to a case insensitive collision, required by docker dependencies

kvch force-pushed the update-golang-to-1.12.1 branch from ce055eb to 069b5f3 Compare March 21, 2019 09:00

kvch commented Mar 21, 2019

View reviewed changes

kvch added the review label Mar 21, 2019

kvch marked this pull request as ready for review March 21, 2019 09:50

kvch requested review from a team as code owners March 21, 2019 09:51

kvch requested review from andrewkroh, ruflin and ph and removed request for a team March 21, 2019 09:52

ph added the :Packaging label Mar 21, 2019

ph reviewed Mar 21, 2019

View reviewed changes

libbeat/logp/core_test.go Outdated Show resolved Hide resolved

kvch added the in progress Pull request is currently in progress. label Mar 21, 2019

kvch force-pushed the update-golang-to-1.12.1 branch 3 times, most recently from 1c6f729 to 8c451b8 Compare March 21, 2019 16:49

ruflin reviewed Mar 22, 2019

View reviewed changes

ruflin mentioned this pull request Mar 25, 2019

Xpack Foo module on mage build system #9242

Merged

kvch force-pushed the update-golang-to-1.12.1 branch from 232543b to 7c28ffb Compare April 1, 2019 11:38

kvch added 5 commits April 2, 2019 11:04

update version numbers in the appropiate places

474e351

vendor github.com/tsg/gopacket

d5d0a67

decrease the number of skipped Callers when recovering from panic

adb4561

add changelog entry

ca1598d

update docker dependencies to remove Sirupsen from vendor

89d0de2

kvch force-pushed the update-golang-to-1.12.1 branch from e9e2008 to 89d0de2 Compare April 2, 2019 09:13

urso reviewed Apr 2, 2019

View reviewed changes

CHANGELOG.next.asciidoc Show resolved Hide resolved

update changelog

adddf0c

kvch force-pushed the update-golang-to-1.12.1 branch from 9ddca5e to adddf0c Compare April 2, 2019 16:09

kvch removed the in progress Pull request is currently in progress. label Apr 9, 2019

urso approved these changes Apr 9, 2019

View reviewed changes

kvch merged commit 0c1d247 into elastic:master Apr 9, 2019

jsoriano mentioned this pull request Apr 15, 2019

Fix logrus dependency #10791

Closed

andrewkroh mentioned this pull request Apr 15, 2019

Fix logrus dependency #4642

Closed

jordansissel mentioned this pull request Apr 17, 2019

go mod not working with master #11850

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Golang to 1.12.1 #11330

Update Golang to 1.12.1 #11330

kvch commented Mar 20, 2019 •

edited

Loading

ruflin commented Mar 20, 2019

kvch commented Mar 20, 2019

kvch Mar 21, 2019

ruflin Mar 22, 2019

kvch Mar 22, 2019

ruflin Mar 25, 2019

andrewkroh Mar 25, 2019

ruflin Mar 27, 2019

ph commented Mar 21, 2019

ph commented Mar 21, 2019

ph commented Mar 21, 2019

graphaelli commented Mar 21, 2019

ph commented Mar 21, 2019

kvch commented Mar 21, 2019

ruflin commented Mar 22, 2019

ruflin Mar 22, 2019

kvch Mar 22, 2019

ruflin Mar 22, 2019

kvch Mar 22, 2019 •

edited

Loading

ruflin Mar 25, 2019

ruflin commented Mar 25, 2019

jordansissel commented Apr 17, 2019

Update Golang to 1.12.1 #11330

Update Golang to 1.12.1 #11330

Conversation

kvch commented Mar 20, 2019 • edited Loading

ruflin commented Mar 20, 2019

kvch commented Mar 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ph commented Mar 21, 2019

ph commented Mar 21, 2019

ph commented Mar 21, 2019

graphaelli commented Mar 21, 2019

ph commented Mar 21, 2019

kvch commented Mar 21, 2019

ruflin commented Mar 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvch Mar 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Mar 25, 2019

jordansissel commented Apr 17, 2019

kvch commented Mar 20, 2019 •

edited

Loading

kvch Mar 22, 2019 •

edited

Loading