Skip to content

Remove some redundant code and cleanup validation#38910

Merged
thaJeztah merged 5 commits intomoby:masterfrom
thaJeztah:remove_hot_pluggable_check
Mar 21, 2019
Merged

Remove some redundant code and cleanup validation#38910
thaJeztah merged 5 commits intomoby:masterfrom
thaJeztah:remove_hot_pluggable_check

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 20, 2019

See individual commits for details

@thaJeztah thaJeztah force-pushed the remove_hot_pluggable_check branch 2 times, most recently from 21a71a6 to 3f4fae1 Compare March 20, 2019 09:50
@thaJeztah thaJeztah changed the title Remove some redundant code and cleanup validation [WIP] Remove some redundant code and cleanup validation Mar 20, 2019
@thaJeztah
Remove redundant isNetworkHotPluggable() function
0169ad3 
All platforms now have hot-pluggable networks, so this
check was no longer needed.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Move EnableServiceDiscoveryOnDefaultNetwork to container-operations
20dde01 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Simplify hasUserDefinedIPAddress, and centralize validation
ebe0174 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
connectToNetwork: use locally scoped err
bcb4a33 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Simplify verifyNetworkingConfig()
c7105e3 
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the remove_hot_pluggable_check branch from 3f4fae1 to c7105e3 Compare March 20, 2019 17:47
@thaJeztah thaJeztah changed the title [WIP] Remove some redundant code and cleanup validation Remove some redundant code and cleanup validation Mar 20, 2019
@codecov
Copy link

codecov bot commented Mar 20, 2019 •
edited
Loading

Codecov Report

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

@@            Coverage Diff            @@
##             master   #38910   +/-   ##
=========================================
  Coverage          ?   36.89%           
=========================================
  Files             ?      614           
  Lines             ?    45372           
  Branches          ?        0           
=========================================
  Hits              ?    16741           
  Misses            ?    26338           
  Partials          ?     2293

vdemeester
vdemeester approved these changes Mar 20, 2019
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM if green 🐯

yongtang
yongtang approved these changes Mar 21, 2019
View reviewed changes
Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

@vdemeester
Copy link
Member

vdemeester commented Mar 21, 2019 •
edited
Loading 

06:59:17 === RUN   TestCheckpoint
06:59:18 --- FAIL: TestCheckpoint (1.59s)
06:59:18     checkpoint_test.go:40: Error (criu/util.c:816): exited, status=3
06:59:18         Warn  (criu/net.c:2840): Unable to get socket network namespace
06:59:18         Warn  (criu/net.c:2840): Unable to get tun network namespace
06:59:18         Warn  (criu/sk-unix.c:229): unix: Unable to open a socket file: Bad address
06:59:18         Warn  (criu/net.c:2840): Unable to get socket network namespace
06:59:18         Warn  (criu/kerndat.c:881): Can't keep kdat cache on non-tempfs
06:59:18         Looks good.
06:59:18     checkpoint_test.go:51: Start a container
06:59:18     checkpoint_test.go:69: ++ type -P true
06:59:18         ++ type -P ip6tables-restore
06:59:18         + mount --bind /bin/true /sbin/ip6tables-restore
06:59:18         ++ type -P true
06:59:18         ++ type -P ip6tables-save
06:59:18         + mount --bind /bin/true /sbin/ip6tables-save
06:59:18     checkpoint_test.go:81: Do a checkpoint and leave the container running
06:59:18     checkpoint_test.go:24: Exec: [touch /tmp/test-file]
06:59:18     checkpoint_test.go:28: 
06:59:18     checkpoint_test.go:115: Do a checkpoint and stop the container
06:59:18     checkpoint_test.go:144: Restore the container
06:59:18     checkpoint_test.go:146: assertion failed: error is not nil: Error response from daemon: failed to upload checkpoint to containerd: commit failed: content sha256:d577e54a7a0fb20548164606d7b6249d03027e5821101abe80fe1fca7f8c7cb8: already exists
06:59:18     checkpoint_test.go:77: ++ type -P ip6tables-restore
06:59:18         + umount -c -i -l /sbin/ip6tables-restore
06:59:18         ++ type -P ip6tables-save
06:59:18         + umount -c -i -l /sbin/ip6tables-save

Is it a known flaky test ?

@thaJeztah
Copy link
Member Author

thaJeztah commented Mar 21, 2019

I think that test was added recently; looks like this is the failure;

Error response from daemon: failed to upload checkpoint to containerd: commit failed: content sha256:d577e54a7a0fb20548164606d7b6249d03027e5821101abe80fe1fca7f8c7cb8: already exists

@thaJeztah thaJeztah merged commit c522dd4 into moby:master Mar 21, 2019
@thaJeztah thaJeztah deleted the remove_hot_pluggable_check branch March 21, 2019 12:27
@thaJeztah thaJeztah mentioned this pull request Mar 28, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@vdemeester vdemeester vdemeester approved these changes

@yongtang yongtang yongtang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/daemon Core Engine status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @vdemeester @yongtang @GordonTheTurtle