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.

Sign up for GitHub

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

Jump to bottom

Upgrade: Bump to Go 1.10.4 #2321

Merged
merged 1 commit into from
Oct 23, 2018
Merged

Upgrade: Bump to Go 1.10.4 #2321

merged 1 commit into from
Oct 23, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Oct 18, 2018

Signed-off-by: Wei Fu [email protected]

Ⅰ. Describe what this PR did

  1. update .travis.yml
  2. make sure CI runs the client unit test
  3. fix golint error

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

no need

Ⅳ. Describe how to verify it

wait for CI

Ⅴ. Special notes for reviews

@codecov
Copy link

codecov bot commented Oct 18, 2018 •
edited
Loading

Codecov Report

Merging #2321 into master will increase coverage by 0.55%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2321      +/-   ##
==========================================
+ Coverage   67.45%   68.01%   +0.55%     
==========================================
  Files         218      265      +47     
  Lines       17678    18207     +529     
==========================================
+ Hits        11925    12383     +458     
- Misses       4351     4399      +48     
- Partials     1402     1425      +23
Flag Coverage Δ
#criv1alpha1test 31.58% <0%> (-0.72%) ⬇️
#criv1alpha2test 35.62% <0%> (-0.85%) ⬇️
#integrationtest 39.53% <11.11%> (-0.9%) ⬇️
#nodee2etest 32.9% <44.44%> (-0.67%) ⬇️
#unittest 25.31% <0%> (+1.89%) ⬆️
Impacted Files Coverage Δ
daemon/mgr/container_types.go 83.33% <ø> (ø) ⬆️
cri/v1alpha1/cri_utils.go 83.22% <0%> (ø) ⬆️
cri/v1alpha2/cri_utils.go 90.28% <100%> (ø) ⬆️
daemon/mgr/container_commit.go 55% <100%> (ø) ⬆️
cri/stream/httpstream/spdy/upgrade.go 54.28% <0%> (-5.72%) ⬇️
daemon/mgr/container_utils.go 83.43% <0%> (-0.62%) ⬇️
cri/v1alpha2/cri_wrapper.go 61.2% <0%> (ø) ⬆️
cri/v1alpha2/cri.go 67.91% <0%> (ø) ⬆️
pkg/term/term.go
pkg/serializer/serialize.go
... and 53 more

@fuweid fuweid changed the title upgrade: up to go1.10.4 version [WIP] upgrade: up to go1.10.4 version Oct 18, 2018
@fuweid fuweid changed the title [WIP] upgrade: up to go1.10.4 version upgrade: up to go1.10.4 version Oct 18, 2018
@Ace-Tang
Copy link
Contributor

Split un-related changes into other pr ? @fuweid

@Ace-Tang
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Oct 18, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Oct 18, 2018

Split un-related changes into other pr

@Ace-Tang all changed is related. The lint policy of new go version has been changed. So I need to modify the source.

@fuweid fuweid added this to the v1.1 milestone Oct 18, 2018
@HusterWan
Copy link
Contributor

LGTM

1 similar comment
@zhuangqh
Copy link
Contributor

LGTM

@allencloud
Copy link
Collaborator

allencloud commented Oct 18, 2018 •
edited
Loading

Record the cri-tools failure:

• Failure [3.657 seconds]
[k8s.io] Streaming
/home/travis/gopath/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:72
  runtime should support streaming interfaces
  /home/travis/gopath/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:55
    runtime should support attach [Conformance] [It]
    /home/travis/gopath/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:81
    The stdout of exec should be hello
    Expected
        <string>: hello
        
    to equal
        <string>: hello
        
    /home/travis/gopath/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:222

And re-run this.

@zhuangqh
Copy link
Contributor

please also upgrade the GO_VERSION variable in Vagrantfile to 1.10.4. @fuweid

@fuweid
Copy link
Contributor Author

fuweid commented Oct 19, 2018

@zhuangqh thank you!

@fuweid fuweid removed the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Oct 19, 2018
@fuweid fuweid changed the title upgrade: up to go1.10.4 version [WIP] upgrade: up to go1.10.4 version Oct 19, 2018
@fuweid
Copy link
Contributor Author

fuweid commented Oct 19, 2018

It seems there is some wrong in k8s.io/client/spdy implementations. still investigating..

@fuweid
Copy link
Contributor Author

fuweid commented Oct 20, 2018

root cause is here kubernetes-sigs/cri-tools#257

@fuweid fuweid changed the title [WIP] upgrade: up to go1.10.4 version [WIP] Bump to Go 1.10.4 Oct 20, 2018
@fuweid fuweid mentioned this pull request Oct 22, 2018
Closed
@fuweid
upgrade: up to go1.10.4 version
812b7b6 
1. update .travis.yml/.circleci-config.yml
2. make sure CI runs the client unit test
3. fix golint error

Signed-off-by: Wei Fu <[email protected]>
@fuweid fuweid changed the title [WIP] Bump to Go 1.10.4 Bump to Go 1.10.4 Oct 23, 2018
@fuweid fuweid changed the title Bump to Go 1.10.4 Upgrade: Bump to Go 1.10.4 Oct 23, 2018
@zhuangqh
Copy link
Contributor

LGTM

1 similar comment
@HusterWan
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Oct 23, 2018
@HusterWan HusterWan merged commit 555e253 into AliyunContainerService:master Oct 23, 2018
@fuweid fuweid deleted the upgrade_go_version_with_1-10-4 branch October 23, 2018 07:03
@pouchrobot pouchrobot mentioned this pull request Oct 27, 2018
Closed
@allencloud allencloud mentioned this pull request Nov 4, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhuangqh zhuangqh zhuangqh approved these changes

@allencloud allencloud Awaiting requested review from allencloud

@HusterWan HusterWan Awaiting requested review from HusterWan

@Ace-Tang Ace-Tang Awaiting requested review from Ace-Tang

Assignees
No one assigned
Labels
LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@fuweid @Ace-Tang @HusterWan @zhuangqh @allencloud @pouchrobot