Initial varlink implementation #627

baude · 2018-04-16T20:07:08Z

mheon · 2018-04-18T15:05:55Z

Makefile

This really ought to be separate from install.varlink - generate.varlink target, or just put it in make, localintegration, localunit as needed

mheon · 2018-04-18T15:07:57Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

Don't want this. We never implemented podman update so no reason to have a Varlink target for a nonexistant API

Per our IRC conversation, it will stay not implemented and culled later.

mheon · 2018-04-18T15:08:20Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

See above - no podman command, so no need for an API

Per our IRC conversation, it will stay not implemented and culled later.

mheon · 2018-04-18T15:08:50Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

How is this different from RemoveContainer? We have no distinction in libpod between deleting and removing

Fixed. It should be DeleteStoppedContainers

mheon · 2018-04-18T15:09:27Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

Does this pull? If not, what does pull images, I don't see a pull target?

Good find @mheon . It does do a pull and that API has no outright Pull target. I can understand why, given most everything will pull if it isnt local. That said, I would have zero objection to adding a Pull target. WDYT?

I think it's probably a good idea. If nothing else, it's a lot more clear what it will do vs CreateImage, which sounds like it could potentially be related to build? API docs will help this, but familiar names are always good

mheon · 2018-04-18T15:10:06Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

Missing CommitContainer()

It is under the image side of things ...

mheon · 2018-04-18T15:10:36Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

This should be baked into attach. We don't do separate TTY resize.

Per our IRC conversation, it will stay not implemented and culled later.

mheon · 2018-04-18T15:11:14Z

cmd/podman/ioprojectatomicpodman/io.projectatomic.podman.varlink

I think we should move this over to the Container commands and call it Commit

mheon · 2018-04-18T15:14:12Z

cmd/podman/varlink.go

Does this return an error? We probably should not ignore it if so

nice catch, fixed

mheon · 2018-04-18T15:15:24Z

docs/podman-varlink.1.md

This paragraph makes no sense... Did you reuse podman stop but s/stop/varlink?

lol, this is a copy/paste leftover. fixed

mheon · 2018-04-18T15:16:04Z

libpod/info.go

Why is kernel Version caps now?

that is a result of a refactor for version. ill revert it.

baude · 2018-04-18T15:16:05Z

(8) Add podman varlink subcommand

mheon · 2018-04-18T15:16:31Z

libpod/version.go

Rename to Version

Actually, rename to GetVersion, and rename the struct it returns to Version

mheon · 2018-04-18T15:17:23Z

libpod/version.go

Do we have to include this here? Are other structs going to need it?

yes, all returned structs

This means we're going to be throwing them in the DB. I'm not sure I like that.

mheon · 2018-04-18T15:20:23Z

Can you throw the generated files in .gitignore so we don't commit them accidentally?

rh-atomic-bot · 2018-04-20T14:56:14Z

☔ The latest upstream changes (presumably 7580b1c) made this pull request unmergeable. Please resolve the merge conflicts.

Signed-off-by: baude <bbaude@redhat.com>

baude · 2018-04-20T18:00:31Z

bot, retest this please

baude · 2018-04-20T21:15:14Z

bot, retest this please

mheon · 2018-04-23T14:29:04Z

LGTM. Good first step.

mheon · 2018-04-23T14:29:17Z

It helps when I hit the right button

mheon · 2018-04-23T14:29:41Z

@rh-atomic-bot r+

rh-atomic-bot · 2018-04-23T14:29:42Z

📌 Commit 816d582 has been approved by mheon

rh-atomic-bot · 2018-04-23T14:29:48Z

⌛ Testing commit 816d582 with merge 8493dba...

rh-atomic-bot · 2018-04-23T14:51:30Z

☀️ Test successful - status-papr
Approved by: mheon
Pushing 8493dba to master...

Following the vndr docs [1]: $ go get -u github.com/LK4D4/vndr $ vndr golang.org/x/text $ git add -A vendor/golang.org/x/text The targeted 'git add' was because we seem to have versioned some test files (e.g. vendor/github.com/varlink/go/varlink/varlink_test.go in 8493dba (Initial varlink implementation, 2018-03-26, containers#627). I don't know why, possibly an old vndr version? But either way, I'm punting that particular issue to a separate branch. [1]: https://github.com/LK4D4/vndr/blob/1fc68ee0c852556a9ed53cbde16247033f104111/README.md Signed-off-by: W. Trevor King <wking@tremily.us>

Following the vndr docs [1]: $ go get -u github.com/LK4D4/vndr $ vndr golang.org/x/text $ git add -A vendor/golang.org/x/text The targeted 'git add' was because we seem to have versioned some test files (e.g. vendor/github.com/varlink/go/varlink/varlink_test.go in 8493dba (Initial varlink implementation, 2018-03-26, #627). I don't know why, possibly an old vndr version? But either way, I'm punting that particular issue to a separate branch. [1]: https://github.com/LK4D4/vndr/blob/1fc68ee0c852556a9ed53cbde16247033f104111/README.md Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #686 Approved by: mheon

This line landed in 8493dba (Initial varlink implementation, 2018-03-26, containers#627), but this Makefile has never consumed that variable. Signed-off-by: W. Trevor King <wking@tremily.us>

This line landed in 8493dba (Initial varlink implementation, 2018-03-26, #627), but this Makefile has never consumed that variable. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #818 Approved by: mheon

baude force-pushed the varlink branch 7 times, most recently from 5f03ba4 to ea7555f Compare April 18, 2018 15:02

baude changed the title ~~[WIP] Initial varlink implementation~~ Initial varlink implementation Apr 18, 2018

mheon reviewed Apr 18, 2018

View reviewed changes

baude force-pushed the varlink branch 6 times, most recently from 294a203 to 63d5f33 Compare April 19, 2018 13:46

Initial varlink implementation

816d582

Signed-off-by: baude <bbaude@redhat.com>

baude force-pushed the varlink branch from 63d5f33 to 816d582 Compare April 20, 2018 15:10

mheon closed this Apr 23, 2018

mheon reopened this Apr 23, 2018

rh-atomic-bot closed this in 8493dba Apr 23, 2018

TomasTomecek mentioned this pull request Apr 26, 2018

implement libpod/podman as a new backend user-cont/conu#200

Closed

baude deleted the varlink branch December 22, 2019 19:15

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023

Initial varlink implementation #627

Initial varlink implementation #627

Uh oh!

Conversation

baude commented Apr 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baude commented Apr 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mheon commented Apr 18, 2018

Uh oh!

rh-atomic-bot commented Apr 20, 2018

Uh oh!

baude commented Apr 20, 2018

Uh oh!

baude commented Apr 20, 2018

Uh oh!

mheon commented Apr 23, 2018

Uh oh!

mheon commented Apr 23, 2018

Uh oh!