Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

versions: Bump golang from 1.8.3 to 1.9.7#643

Merged
sboeuf merged 1 commit intokata-containers:masterfrom
jodh-intel:golang-1.8.3-to-1.9.7
Sep 14, 2018
Merged

versions: Bump golang from 1.8.3 to 1.9.7#643
sboeuf merged 1 commit intokata-containers:masterfrom
jodh-intel:golang-1.8.3-to-1.9.7

Conversation

@jodh-intel
Copy link

golang version 1.8.3 is old and the runtime cannot even be built with
it now it seems.

Since it is no longer considered a stable version [1], move to the
oldest official stable version (version 1.9.7).

Fixes #642.

[1] - https://golang.org/dl/

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

@jodh-intel
Copy link
Author

/cc @chavafg.

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm

@chavafg @jodh-intel - do we need to tweak our Travis setup - I think we have a 1.8 job there?

@jodh-intel
Copy link
Author

grep isn't finding that in https://github.com/kata-containers/ci, but does memory serve that Travis installs the packaged version of go (which might be 1.8.x) and we then install the correct version using https://github.com/kata-containers/tests/blob/master/.ci/install_go.sh @chavafg?

@grahamwhaley
Copy link
Contributor

Ah, it may be in travis.yml in other repos that means I've seen it on the Travis reports:
https://github.com/kata-containers/proxy/blob/master/.travis.yml#L21
and in agent at least. Could be others.

@grahamwhaley
Copy link
Contributor

This repo appears to Travis only 1.10, and that is reflected for instance in the relevant link for this PR
https://travis-ci.org/kata-containers/runtime/builds/420115962?utm_source=github_status&utm_medium=notification

I guess the question is, are we setting 1.9.x as the base for all the repos? I suspect the answer will be yes.

@jodh-intel
Copy link
Author

ok great, so no further changes required for this repo though :)

Copy link

@devimc devimc left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented Aug 24, 2018

Codecov Report

Merging #643 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #643   +/-   ##
=======================================
  Coverage   65.35%   65.35%           
=======================================
  Files          85       85           
  Lines        9879     9879           
=======================================
  Hits         6456     6456           
  Misses       2766     2766           
  Partials      657      657

@opendev-zuul
Copy link

opendev-zuul bot commented Aug 24, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@bergwolf
Copy link
Member

@jodh-intel Thanks for raising this. Is this version yaml used to build kata packages we ship? If so, I would suggest that we use the latest stable go compiler instead. Newer compilers have a lot of improvements that can benefit kata users.

For old go compilers, we can do a simply sanity check by building with them. travis-ci can be a good place to play such compile only tests.

@jodh-intel
Copy link
Author

Hi @bergwolf - see kata-containers/packaging#147 regarding the version of golang we use for package builds.

I do wonder if we want to re-assess whether we need to specify two go versions in versions.yaml any more.

@jcvenegas, @chavafg - wdyt? Is languages.golang.meta.newest-version in versions.yaml still useful or could we just drop it and only have languages.golang.version?

@bergwolf
Copy link
Member

@jodh-intel Thanks for the pointers. I think we can:

  1. build with different go versions
  2. test and ship with the latest stable go version

I don't remember we have any dependency on golang versions yet. If we do, we can declare a minimal go version to support (like 1.9.7).

WDYT?

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 165709 KB
Proxy: 4006 KB
Shim: 8890 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003572 KB

@chavafg
Copy link
Contributor

chavafg commented Aug 24, 2018

lgtm

I see that tests, proxy shim... repositories have different go versions in their .travis.yml. We should also change them to the same here.

For jenkins we use the .ci/install_go.sh, which reads languages.golang.meta.newest-version.

I agree that we can stick to only one version.

@caoruidong
Copy link
Member

caoruidong commented Aug 25, 2018

"not ok 2 Running within memory constraints"
This test fails on many PRs, like kata-containers/agent#331.
But we just bump golang version. No idea about why.

@WeiZhang555
Copy link
Member

  1. build with different go versions
  2. test and ship with the latest stable go version

I agree with @bergwolf , this looks reasonable

@jodh-intel
Copy link
Author

Hi @bergwolf,

  • build with different go versions

I'm not quite clear what you mean here - do you mean we should build each component with a range of go versions (like we do in some of the .travis.yml files)? Or do you mean we should all the CI's to build with any version of go they like (as long as it's => minimum supported version and <= latest stable version)?

  • test and ship with the latest stable go version

Ah - so presumably you mean we should run all CI + packaging using the same version (latest stable) - sounds good to me.

I don't remember we have any dependency on golang versions yet.

I had thought there was a reason we standardised on 1.8.3. Maybe clearcontainers/runtime@c5569a5 but @sboeuf may remember more :)

If we do, we can declare a minimal go version to support (like 1.9.7).

I might be misunderstanding, but that's the intention of this PR - to update the minimum version we will use for CI + packaging.

@WeiZhang555
Copy link
Member

Hi @jodh-intel , today I debugged a bit and find that it may need little change to fix golang 1.8.3 build, so I raised another PR #662 to address this, I think it's better to fix the build issue though 1.8.3 is not stable anymore. What do you think?

This PR is still valid despite of #662 so we can still merge it.

golang version 1.8.3 is old and the runtime cannot even be built with
it now it seems.

Since it is no longer considered a stable version [1], move to the
oldest official stable version (version 1.9.7).

Fixes kata-containers#642.

[1] - https://golang.org/dl/

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the golang-1.8.3-to-1.9.7 branch from 2cc20d5 to d814bc5 Compare August 30, 2018 08:40
@opendev-zuul
Copy link

opendev-zuul bot commented Aug 30, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 169077 KB
Proxy: 4006 KB
Shim: 8996 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003564 KB

@WeiZhang555
Copy link
Member

@jodh-intel How about update golang to latest stable 1.11?

@jodh-intel
Copy link
Author

Sounds like a good idea. But I'm still not clear on the best approach here. Your recent #662 suggests folk do still care about older versions of go, but should we leave such PRs up to those in the community that do care, or just constantly follow the "tip" (1.11)?

Maybe we should change the golang entry in the versions database to something like:

diff --git a/versions.yaml b/versions.yaml
index 145e8aa..76c0740 100644
--- a/versions.yaml
+++ b/versions.yaml
@@ -183,9 +183,9 @@ languages:
   golang:
     description: "Google's 'go' language"
     notes: "'version' is the default minimum version used by this project."
-    version: "1.8.3"
+    version: "1.11"
     meta:
-      newest-version: "1.10"
+      oldest-version: "1.8.3"

But if we do that, who / what will ensure that we haven't broken support for oldest-version? I don't think our current CI has sufficient capacity to test with >1 golang versions but maybe that situation will change if zuul ever happens (kata-containers/ci#42).

Related:

/cc @grahamwhaley, @chavafg, @jcvenegas, @egernst, @bergwolf.

@sboeuf
Copy link

sboeuf commented Sep 14, 2018

This conversation has to continue and as @jodh-intel mentioned, maybe testing several version of Golang might be a good idea if we have enough CI resources.
But for the moment (cc @jodh-intel @WeiZhang555), I think it's still better to move to 1.9.7 given the fact this is the oldest latest stable, and that 1.8.3 is too old.

@sboeuf sboeuf merged commit e620470 into kata-containers:master Sep 14, 2018
@egernst egernst removed the review label Sep 14, 2018
ganeshmaharaj pushed a commit to ganeshmaharaj/kata-runtime that referenced this pull request Jul 17, 2019
Fixes: #123412345
Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
ganeshmaharaj pushed a commit to ganeshmaharaj/kata-runtime that referenced this pull request Jul 17, 2019
Fixes: #123412345
Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
ganeshmaharaj pushed a commit to ganeshmaharaj/kata-runtime that referenced this pull request Jul 17, 2019
Fixes: #123412345
Depends-on: github.com/kata-containers/packaging#643
Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>
egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
…bridVsock

protocols/client: support hybrid vsocks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants