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

Bump kube dependency to 1.11.0, and update other deps #1230

Merged
merged 3 commits into from
Jul 30, 2018

Conversation

vdemeester
Copy link
Collaborator

  • Bump kubernetes dependencies to 1.11
  • Bump some dependencies to more recent versions (and tagged if available)

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@codecov-io
Copy link

codecov-io commented Jul 25, 2018

Codecov Report

Merging #1230 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1230      +/-   ##
==========================================
+ Coverage   54.23%   54.25%   +0.01%     
==========================================
  Files         268      268              
  Lines       17805    17799       -6     
==========================================
  Hits         9656     9656              
+ Misses       7542     7536       -6     
  Partials      607      607

vendor.conf Outdated
vbom.ml/util 928aaa586d7718c70f4090ddf83f2b34c16fdc8d
github.com/containerd/console cb7008ab3d8359b78c5f464cb7cf160107ad5925
github.com/tonistiigi/units 29de085e9400559bd68aea2e7bc21566e7b8281d
github.com/google/shlex 6f45313302b9c56850fc17f99e40caebce98c716
github.com/google/shlex 6f45313302b9c56850fc17f99e40caebce98c716
Copy link
Member

Choose a reason for hiding this comment

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

missing newline 😇

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😅

vendor.conf Outdated
github.com/containerd/containerd 08f7ee9828af1783dc98cc5cc1739e915697c667
github.com/containerd/continuity d8fb8589b0e8e85b8c8bbaa8840226d0dfeb7371
github.com/coreos/etcd v3.2.1
github.com/coreos/etcd v3.3.8
Copy link
Member

Choose a reason for hiding this comment

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

3.3.9 was released

Also; I've been punting on this one (SwarmKit is also on a very old version, but I think there were some complications there updating, but should check)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh 🤔

vendor.conf Outdated
github.com/google/gofuzz 24818f796faf91cd76ec7bddd72458fbced7a6c1
github.com/googleapis/gnostic 7c663266750e7d82587642f65e60bc4083f1f84e # v0.2.0
github.com/gorilla/context v1.1.1
github.com/gorilla/mux v1.6.2
gotest.tools v2.1.0
Copy link
Member

Choose a reason for hiding this comment

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

we should make this list alphabetical again 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I think I'm gonna do a commit that reorders them 😉

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Use the tag instead of the sha.

github.com/cpuguy83/go-md2man v1.0.8
github.com/davecgh/go-spew 346938d642f2ec3594ed81d874461961cd0faa76
github.com/davecgh/go-spew 346938d642f2ec3594ed81d874461961cd0faa76 # v1.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using the tag directly?

Copy link
Member

Choose a reason for hiding this comment

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

Ah! Yes, there's been many discussions about those. Ideally, there'd be a way to specify both tag and commit/sha;

reason to use a tag:

  • it's human-readable
  • it's a release done by the upstream dependency

reason to not use a tag:

  • we've had upstreams re-tagging (discovered when vendor check failed and no changes were made in the vendor.conf)
  • we've had cases where it turned out not to be a tag but a branch named v1.0 (but we thought it was a tag), then found out about that (again) when vendor check failed.
  • a commit-sha is unique, and can never change

So... how to get the best of both worlds?

  • use sha
  • leave comment what version it should match with?

It's tricky; because we also have to verify that the commit matches the tag 😞 possibly the git describe notation would be an option, e.g.;

v0.2.0-7-g1817cd4

  • 7 commits since v0.2.0
  • sha is 1817cd4

not sure if that actually verifies if they match though? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right, lot's of issues I wasn't aware of 😓 Then that's ok for me 😸

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

k8s.io/client-go kubernetes-1.11.0
k8s.io/kube-openapi d8ea2fe547a448256204cfc68dfee7b26c720acb
k8s.io/kubernetes v1.11.0
vbom.ml/util 256737ac55c46798123f754ab7d2c784e2c71783
Copy link
Member

Choose a reason for hiding this comment

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

Missing a \n

Copy link
Member

Choose a reason for hiding this comment

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

Was fixed

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit b4e5063 into docker:master Jul 30, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.09.0 milestone Jul 30, 2018
@vdemeester vdemeester deleted the update-k8s-and-other-deps branch July 30, 2018 15:49
@thaJeztah thaJeztah changed the title Update k8s and other deps Bump kube dependency to 1.11, and update other deps Aug 30, 2018
@thaJeztah thaJeztah changed the title Bump kube dependency to 1.11, and update other deps Bump kube dependency to 1.11.0, and update other deps Aug 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants