Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Godep: Add godep for rkt. #7410

Merged
merged 1 commit into from Apr 28, 2015
Merged

Godep: Add godep for rkt. #7410

merged 1 commit into from Apr 28, 2015

Conversation

yifan-gu
Copy link
Contributor

rkt: v0.5.4
appc spec: v0.5.1

rkt: v0.5.4
appc spec: v0.5.1
@yifan-gu
Copy link
Contributor Author

@dchen1107 @vmarmol @bakins

@dchen1107 dchen1107 self-assigned this Apr 28, 2015
@dchen1107
Copy link
Member

LGTM on this Godep and will merge it once it is green.

@yifan-gu
Copy link
Contributor Author

Thank you!

@dchen1107 dchen1107 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 28, 2015
@pmorie
Copy link
Member

pmorie commented Apr 28, 2015

Kicked shippable, looked like a flake.

@yifan-gu
Copy link
Contributor Author

@pmorie Thank you!

@pmorie
Copy link
Member

pmorie commented Apr 28, 2015

Yet another shippable flake, kicked again 👣

@vmarmol
Copy link
Contributor

vmarmol commented Apr 28, 2015

Travis is green, LGTM. Merging!

vmarmol added a commit that referenced this pull request Apr 28, 2015
@vmarmol vmarmol merged commit 17e6123 into kubernetes:master Apr 28, 2015
@erictune
Copy link
Member

So, for docker, we vendor client libraries, but for rkt, we vendor the entire rkt implementation? And we plan to compile all of rkt into Kubelet?

Just thinking through what this means.

  • Upgrading kubelet upgrades rkt support as well.
  • how do we start kubelet in a container without a separate rkt binary? With --run_once?
  • how is the rkt version reported, compared to how docker version is reported?

@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

@erictune this (this PR's title) is a bit misleading, we still use the rkt binary for most operations. We are importing the client libraries and APIs around it. Today, it is a bit of an ugly hybrid. With time I think we will move fully to a client.

@pmorie
Copy link
Member

pmorie commented Apr 30, 2015

@vmarmol, this threw me too and was why I was asking if we were embedding
rocket the other day.

On Thu, Apr 30, 2015 at 4:42 PM, Victor Marmol notifications@github.com
wrote:

@erictune https://github.com/erictune this is a bit misleading, we
still use the rkt binary for most operations. We are importing the client
libraries and APIs around it. Today, it is a bit of an ugly hybrid. With
time I think we will move fully to a client.


Reply to this email directly or view it on GitHub
#7410 (comment)
.

@erictune
Copy link
Member

erictune commented May 1, 2015

Is this just vendoring the client? That is a lot of files. Or is it
vendoring all of rkt? Is there a way to vendor just the client?

On Thu, Apr 30, 2015 at 1:44 PM, Paul Morie notifications@github.com
wrote:

@vmarmol, this threw me too and was why I was asking if we were embedding
rocket the other day.

On Thu, Apr 30, 2015 at 4:42 PM, Victor Marmol notifications@github.com
wrote:

@erictune https://github.com/erictune this is a bit misleading, we
still use the rkt binary for most operations. We are importing the client
libraries and APIs around it. Today, it is a bit of an ugly hybrid. With
time I think we will move fully to a client.


Reply to this email directly or view it on GitHub
<
#7410 (comment)

.


Reply to this email directly or view it on GitHub
#7410 (comment)
.

@yifan-gu
Copy link
Contributor Author

yifan-gu commented May 1, 2015

@erictune That's because now we exposed the internal storage type of rkt to get the image manifest. I think we will fix it when rkt binary provides a way to retrieve the image manifest. All the rkt package should be removed. Only appc/spec is necessary.

@erictune
Copy link
Member

erictune commented May 1, 2015

Okay, as long as you have a plan :-)

On Fri, May 1, 2015 at 9:16 AM, Yifan Gu notifications@github.com wrote:

@erictune https://github.com/erictune That's because now we exposed the
internal storage type of rkt to get the image manifest. I think we will fix
it when rkt binary provides a way to retrieve the image manifest. All the
rkt package should be removed. Only appc/spec is necessary.


Reply to this email directly or view it on GitHub
#7410 (comment)
.

@yifan-gu yifan-gu deleted the rkt_godep branch May 7, 2015 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants