Skip to content

Drop engine api#834

Merged
openshift-bot merged 5 commits intoopenshift:masterfrom
soltysh:drop_engine-api
Nov 28, 2017
Merged

Drop engine api#834
openshift-bot merged 5 commits intoopenshift:masterfrom
soltysh:drop_engine-api

Conversation

@soltysh
Copy link
Contributor

@soltysh soltysh commented Nov 21, 2017

Fixes #833.

@openshift/sig-master fyi, this should in the end help to limit the pkg/builder vendored dirs as well

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 21, 2017
soltysh
soltysh commented Nov 21, 2017
View reviewed changes
pkg/docker/docker.go Outdated
}
// FIXME
exitCode := 0
// exitCode, err := d.client.ContainerWait(context.Background(), container.ID)
Copy link
Contributor Author

@soltysh soltysh Nov 21, 2017

Choose a reason for hiding this comment

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

This is the only bit that is left for me to finish it. It looks like they've changed it significantly and worst thing is that the current conditions (WaitConditionNotRunning, WaitConditionNextExit, WaitConditionRemoved) don't fit us. I was hoping the WiatConditionNextExit will, but if the container exits in between this will block until next exit, which is not quite right. I'll dig a bit more into it, and will see what's possible and what not.

Copy link
Contributor Author

@soltysh soltysh Nov 21, 2017

Choose a reason for hiding this comment

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

@bparees ^

@bparees
Copy link
Contributor

bparees commented Nov 21, 2017

lgtm pending resolution of the FIXME bits.

@bparees bparees mentioned this pull request Nov 22, 2017
Closed
@soltysh soltysh mentioned this pull request Nov 23, 2017
Merged
19 tasks
@soltysh
Copy link
Contributor Author

soltysh commented Nov 23, 2017

@bparees I've fixed the appropriate bits, but I've noticed we're not parsing the docker image spec properly, it was silently falling on reference.ParseNamed which requires canonical name, iow. registry/namespace/image, I changed that to reference.WithName similarly to what we did in origin during rebase. ptal

@soltysh soltysh changed the title WIP: Drop engine api Drop engine api Nov 23, 2017
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 23, 2017
@soltysh
Copy link
Contributor Author

soltysh commented Nov 23, 2017

/test
[test]

@soltysh
Copy link
Contributor Author

soltysh commented Nov 27, 2017

@bparees all green, ptal

@jim-minter
Copy link
Contributor

jim-minter commented Nov 27, 2017

with the caveat that I don't understand the ParseNormalizedNamed change, all this looks good to me. On this branch, hack/verify-godeps.sh indicates:

warning: differing godep github.com/opencontainers/runc/libcontainer/user: origin "4d6e6720a7c885c37b4cb083c0d372dda3425120" vs s2i "d223e2adae83f62d58448a799a5da05730228089"
warning: differing godep github.com/docker/go-units: origin "9e638d38cf6977a37a8ea0078f3ee75a7cdb2dd1" vs s2i "e30f1e79f3cd72542f2026ceec18d3bd67ab859c"
warning: differing godep github.com/Microsoft/go-winio: origin "78439966b38d69bf38227fbf57ac8a6fee70f69a" vs s2i "f533f7a102197536779ea3a8cb881d639e21ec5a"

I can repurpose #836 to deal with that if appropriate?

@soltysh
Copy link
Contributor Author

soltysh commented Nov 28, 2017

with the caveat that I don't understand the ParseNormalizedNamed change, all this looks good to me.

Docker changed their API, significantly. This works 😉

On this branch, hack/verify-godeps.sh indicates:

Uhm... I thought I've picked the exact same level of deps, I'll fix that right away.

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 28, 2017

Evaluated for source to image test up to de8ef59

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 28, 2017

Source To Image Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_s2i/664/) (Base Commit: 60a42d7) (PR Branch Commit: de8ef59)

@soltysh
Copy link
Contributor Author

soltysh commented Nov 28, 2017

All fixed and green, with 2 lgtm, I'm merging.

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 28, 2017 •
edited
Loading

Source To Image Merge Results: Waiting: You are in the build queue at position: 1

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 28, 2017

Evaluated for source to image merge up to de8ef59

@openshift-bot openshift-bot merged commit e3e0eed into openshift:master Nov 28, 2017
@soltysh soltysh deleted the drop_engine-api branch November 28, 2017 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@bparees bparees

Labels

size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@soltysh @bparees @jim-minter @openshift-bot @openshift-ci-robot