Skip to content

Conversation

@shaloulcy
Copy link
Contributor

@shaloulcy shaloulcy commented Jun 20, 2018

It seems a bug of libnetwork. When libnetwork restores the sandbox, it forgets to initialize the epPriority map. So the Join panic. The latest alibaba/libnetwork has fixed the bug. We will vendor the latest libnetwork and add some test cases

https://github.com/alibaba/libnetwork/commit/655f08f8be32ff847d0554ef0128a3de80665cd6

Signed-off-by: Eric Li [email protected]

Ⅰ. Describe what this PR did

It seems a bug of libnetwork. When libnetwork restores the sandbox, it forgets to initialize the epPriority map. So the Join panic. The latest alibaba/libnetwork has fixed the bug. We will vendor the latest libnetwork and add some test cases

Ⅱ. Does this pull request fix one issue?

fixes #1550

Ⅲ. Describe how you did it

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Jun 20, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1556      +/-   ##
==========================================
- Coverage   41.58%   41.57%   -0.02%     
==========================================
  Files         267      267              
  Lines       17339    17339              
==========================================
- Hits         7211     7209       -2     
- Misses       9241     9243       +2     
  Partials      887      887
Impacted Files Coverage Δ
apis/server/utils.go 59.52% <0%> (-4.77%) ⬇️

@allencloud
Copy link
Collaborator

Do we need to add integration test to cover issue #1550 ? @shaloulcy

@pouchrobot pouchrobot added size/L and removed size/XS labels Jun 20, 2018
@shaloulcy shaloulcy force-pushed the connect_panic branch 2 times, most recently from b0347d8 to ca4f8c8 Compare June 20, 2018 07:37
@shaloulcy
Copy link
Contributor Author

@allencloud I have added integration test. PTAL

// TODO: parse endpoint's links

// set priority option
joinOptions = append(joinOptions, libnetwork.JoinOptionPriority(nil, endpoint.Priority))
Copy link
Collaborator

@rudyfly rudyfly Jun 20, 2018

Choose a reason for hiding this comment

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

I'm afraid I will refuse about this, we have used this feature, can you fix this bug in our libnetwork(https://github.com/alibaba/libnetwork) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rudyfly I have update the codes, PTAL

It seems a bug of libnetwork. When libnetwork restores the sandbox,
it forgets to initialize the epPriority map. So the Join panic. The
latest alibaba/libnetwork has fixed the bug. We will vendor the latest
libnetwork and add some test cases

alibaba/libnetwork@655f08f

Signed-off-by: Eric Li <[email protected]>
@pouchrobot pouchrobot added size/M and removed size/L labels Jun 21, 2018
@pouchrobot pouchrobot added size/L and removed size/M labels Jun 21, 2018
@shaloulcy shaloulcy changed the title bugfix: delete the JoinOptionPriority for connect panic bugfix: vendor newest libnetwork for connect panic Jun 21, 2018
@shaloulcy shaloulcy changed the title bugfix: vendor newest libnetwork for connect panic bugfix: vendor latest libnetwork for connect panic Jun 21, 2018
@rudyfly
Copy link
Collaborator

rudyfly commented Jun 21, 2018

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jun 21, 2018
@allencloud allencloud merged commit a9777c7 into AliyunContainerService:master Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

areas/network areas/test kind/bug This is bug report for project kind/panic LGTM one maintainer or community participant agrees to merge the pull reuqest. priority/P0 size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] connect network panic

5 participants