Skip to content

Conversation

@selansen
Copy link
Contributor

This is to address issue moby/moby#35757.

In sandbox creation we disable IPV6 config. But this causes problem in live-restore case
where all IPV6 configs are wiped out on running container. Hence extra check has been added
take care of this issue.

Signed-off-by: selansen elango.siva@docker.com

@codecov-io
Copy link

codecov-io commented Dec 21, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@0eff162). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2043   +/-   ##
=========================================
  Coverage          ?   40.12%           
=========================================
  Files             ?      138           
  Lines             ?    22132           
  Branches          ?        0           
=========================================
  Hits              ?     8881           
  Misses            ?    11955           
  Partials          ?     1296
Impacted Files Coverage Δ
osl/namespace_linux.go 35.31% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eff162...76bfcb0. Read the comment docs.

@mavenugo
Copy link
Contributor

LGTM

Can you pls write a test case for this ?

@fcrisciani
Copy link

LGTM too, waiting for the test

// In live-restore mode, IPV6 entries are getting cleaned up due to below code
// We should retain IPV6 configrations in live-restore mode when Docker Daemon
// comesback. It should work as it is on other cases
if !isRestore {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please avoid the double indentation of the logic below by instead

if !isRestore && !n.isDefault {

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

ping @selansen ^^

@selansen
Copy link
Contributor Author

selansen commented Dec 27, 2017 via email

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "master" git@github.com:selansen/libnetwork.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354563768
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

In sandbox creation we disable IPV6 config. But this causes problem in live-restore case
where all IPV6 configs are wiped out on running container. Hence extra check has been added
take care of this issue.

Signed-off-by: selansen <elango.siva@docker.com>
@selansen
Copy link
Contributor Author

Incorporated all comments and added new unit test case . Pls merge the commit into upstream.

@fcrisciani
Copy link

LGTM

@abhi abhi changed the title Enable IPV6 config on Sandbaox creation on live-restore case Enable IPV6 config on Sandbox creation on live-restore case Jan 2, 2018
Copy link
Contributor

@abhi abhi left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants