Refactor image related functions to use docker engine-api #23800

resouer · 2016-04-03T15:52:00Z

ref #23563

cc @Random-Liu will add more work after initial review.

k8s-bot · 2016-04-03T16:24:50Z

GCE e2e build/test passed for commit 6091a1a6f88a66bf5e12e3909c456791fbeeaf49.

k8s-bot · 2016-04-03T17:09:16Z

GCE e2e build/test passed for commit 3f2000a3e3ccb7f7397e73bd93f09a48ee9822b4.

Random-Liu · 2016-04-04T07:28:59Z

@resouer Thanks a lot! Really appreciate your help~ Will review today. :)

Random-Liu · 2016-04-05T21:29:46Z

pkg/kubelet/dockertools/kube_docker_types.go

+)
+
+// APIImage is the container information returned by ContainerList
+type APIImage dockertypes.Image


I decided not to define our own types to make the interface cleaner. (See #23699)
Maybe just use dockertypes.Image instead. :)

Random-Liu · 2016-04-05T21:39:11Z

Some type naming nits, aside from that LGTM.

My plan is to finish most work this week. If you have time, you could work on InspectImage and RemoveImage as well. I'll work on other functions first, and come back to these two functions if you don't have time to do that. :)

Random-Liu · 2016-04-06T08:49:44Z

@resouer Just FYI, I've finished all but image related functions #23907. Have you started working on that? If not, I'll move on. :)

resouer · 2016-04-06T09:11:53Z

@Random-Liu I am doing image related functions and will update here

Random-Liu · 2016-04-06T18:33:01Z

@resouer Could you leave the PullImage to me? Because there are some secret related things, I need to discuss with people. :)

resouer · 2016-04-07T01:58:53Z

@Random-Liu Sure, I will remove that commit.

k8s-bot · 2016-04-07T03:45:06Z

GCE e2e build/test passed for commit e37da602b7083582e6dda628d409dff65bb93054.

Random-Liu · 2016-04-07T05:07:06Z

@resouer
If you have finished that commit, just sent it here. I meant that if you hadn't work on it, left it to me. Since you already did, just use yours~

k8s-bot · 2016-04-07T06:19:54Z

GCE e2e build/test passed for commit 22e8b5f3cf22da07020c0fa694940722493266f5.

Random-Liu · 2016-04-07T07:37:12Z

pkg/kubelet/dockertools/docker.go

-	opts := docker.PullImageOptions{
-		Repository: repoToPull,
-		Tag:        tag,
+	// opts := docker.PullImageOptions{


Remove the commented code.

k8s-bot · 2016-04-18T16:03:41Z

GCE e2e build/test passed for commit 6faff2221f90929f229a5c6b0ea0fa2155cf7107.

Random-Liu · 2016-04-18T16:54:41Z

pkg/kubelet/dockertools/kube_docker_client.go

-		RegistryAuth: base64Auth,
-	}, nil)
+	opts.RegistryAuth = base64Auth
+	resp, err := d.client.ImagePull(getDefaultContext(), opts, nil)


Something wrong when rebasing, see https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockertools/kube_docker_client.go#L204-L218 and #24101.

@resouer Could you please fix this? After this we could get this PR merged as soon as possible. :)

MHBauer · 2016-04-22T17:32:35Z

pkg/kubelet/dockertools/docker.go

-	RemoveImage(image string) error
+	InspectImage(image string) (*dockertypes.ImageInspect, error)
+	ListImages(opts dockertypes.ImageListOptions) ([]dockertypes.Image, error)
+	PullImage(image string, auth dockertypes.AuthConfig, opts dockertypes.ImagePullOptions) error


Same question as by others below.

Why are we adding an image argument when the opts contain the image information.

@MHBauer We do this, because:

Engine-api has moved image out of ImagePullOptions Make required arguments required. docker/engine-api#162, it will be easier to bump up the engine-api version later.

This is more intuitive Make required arguments required - optional arguments should be optional docker/engine-api#137. :)

Refactor image remove

k8s-bot · 2016-04-23T15:12:02Z

GCE e2e build/test passed for commit a393947.

Random-Liu · 2016-04-23T18:49:15Z

@resouer Thanks a lot! LGTM.

k8s-bot · 2016-04-23T19:38:03Z

GCE e2e build/test passed for commit a393947.

k8s-bot · 2016-04-23T20:43:49Z

GCE e2e build/test failed for commit a393947.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

Random-Liu · 2016-04-24T00:57:08Z

@k8s-bot test this issue #24265.

k8s-bot · 2016-04-24T01:32:45Z

GCE e2e build/test passed for commit a393947.

k8s-github-robot · 2016-04-24T02:21:59Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-04-24T03:01:38Z

GCE e2e build/test passed for commit a393947.

k8s-github-robot · 2016-04-24T03:01:41Z

Automatic merge from submit-queue

UPSTREAM: 82697: Fix TestBlockMapperMapDeviceNotSupportAttach informer sync race Origin-commit: 4dd18181ce98cd4d866bb7acdfe3c49961119384

googlebot added the cla: yes label Apr 3, 2016

k8s-github-robot assigned dchen1107 Apr 3, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Apr 3, 2016

resouer force-pushed the image-refactor branch from 6091a1a to 3f2000a Compare April 3, 2016 16:39

Random-Liu assigned Random-Liu and unassigned dchen1107 Apr 4, 2016

Random-Liu reviewed Apr 5, 2016
View reviewed changes

Random-Liu added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Apr 6, 2016

Random-Liu added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 7, 2016

resouer force-pushed the image-refactor branch from 3f2000a to e37da60 Compare April 7, 2016 03:07

resouer changed the title ~~Refactor list image to use docker engine-api~~ Refactor image related functions to use docker engine-api Apr 7, 2016

resouer mentioned this pull request Apr 7, 2016

Kubelet: Switch to Docker engine-api and add timeout for all docker calls #23563

Closed

22 tasks

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 7, 2016

Random-Liu reviewed Apr 7, 2016
View reviewed changes

Random-Liu mentioned this pull request Apr 18, 2016

Scalability flake: Not all pods in namespace 'kube-system' running and ready within 10m0s #24265

Closed

Random-Liu reviewed Apr 18, 2016
View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2016

MHBauer reviewed Apr 22, 2016
View reviewed changes

resouer added 3 commits April 23, 2016 16:37

Refactor list image to use new api

7ecb44f

Refactor InspectImage method

3918eee

Refactor PullImage RemoveImage methods

a393947

Refactor image remove

resouer force-pushed the image-refactor branch from 6faff22 to a393947 Compare April 23, 2016 14:36

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 23, 2016

Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 23, 2016

k8s-github-robot merged commit 4f9e872 into kubernetes:master Apr 24, 2016

resouer deleted the image-refactor branch April 24, 2016 03:52

resouer mentioned this pull request Jun 30, 2016

Minor fix of nits docker/engine-api#294

Merged

miminar mentioned this pull request Jul 10, 2017

Drop dockerclient in favor of Docker's engine API openshift/origin#15113

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor image related functions to use docker engine-api #23800

Refactor image related functions to use docker engine-api #23800

resouer commented Apr 3, 2016 •

edited

Loading

k8s-bot commented Apr 3, 2016

k8s-bot commented Apr 3, 2016

Random-Liu commented Apr 4, 2016

Random-Liu Apr 5, 2016

Random-Liu commented Apr 5, 2016

Random-Liu commented Apr 6, 2016

resouer commented Apr 6, 2016

Random-Liu commented Apr 6, 2016

resouer commented Apr 7, 2016

k8s-bot commented Apr 7, 2016

Random-Liu commented Apr 7, 2016

k8s-bot commented Apr 7, 2016

Random-Liu Apr 7, 2016

k8s-bot commented Apr 18, 2016

Random-Liu Apr 18, 2016 •

edited

Loading

Random-Liu Apr 19, 2016 •

edited

Loading

MHBauer Apr 22, 2016

Random-Liu Apr 22, 2016

MHBauer Apr 22, 2016

k8s-bot commented Apr 23, 2016

Random-Liu commented Apr 23, 2016

k8s-bot commented Apr 23, 2016

k8s-bot commented Apr 23, 2016

Random-Liu commented Apr 24, 2016

k8s-bot commented Apr 24, 2016

k8s-github-robot commented Apr 24, 2016

k8s-bot commented Apr 24, 2016

k8s-github-robot commented Apr 24, 2016

Refactor image related functions to use docker engine-api #23800

Refactor image related functions to use docker engine-api #23800

Conversation

resouer commented Apr 3, 2016 • edited Loading

k8s-bot commented Apr 3, 2016

k8s-bot commented Apr 3, 2016

Random-Liu commented Apr 4, 2016

Random-Liu Apr 5, 2016

Choose a reason for hiding this comment

Random-Liu commented Apr 5, 2016

Random-Liu commented Apr 6, 2016

resouer commented Apr 6, 2016

Random-Liu commented Apr 6, 2016

resouer commented Apr 7, 2016

k8s-bot commented Apr 7, 2016

Random-Liu commented Apr 7, 2016

k8s-bot commented Apr 7, 2016

Random-Liu Apr 7, 2016

Choose a reason for hiding this comment

k8s-bot commented Apr 18, 2016

Random-Liu Apr 18, 2016 • edited Loading

Choose a reason for hiding this comment

Random-Liu Apr 19, 2016 • edited Loading

Choose a reason for hiding this comment

MHBauer Apr 22, 2016

Choose a reason for hiding this comment

Random-Liu Apr 22, 2016

Choose a reason for hiding this comment

MHBauer Apr 22, 2016

Choose a reason for hiding this comment

k8s-bot commented Apr 23, 2016

Random-Liu commented Apr 23, 2016

k8s-bot commented Apr 23, 2016

k8s-bot commented Apr 23, 2016

Random-Liu commented Apr 24, 2016

k8s-bot commented Apr 24, 2016

k8s-github-robot commented Apr 24, 2016

k8s-bot commented Apr 24, 2016

k8s-github-robot commented Apr 24, 2016

resouer commented Apr 3, 2016 •

edited

Loading

Random-Liu Apr 18, 2016 •

edited

Loading

Random-Liu Apr 19, 2016 •

edited

Loading