V2 podman system service by jwhonce · Pull Request #5634 · containers/podman

jwhonce · 2020-03-27T01:49:04Z

Added support for system service
Enabled linting on the varlinkapi source, needed to support V2
service command
Add hack/golangci-lint.sh to support linting with multiple sets of build tags

Signed-off-by: Jhon Honce jhonce@redhat.com

openshift-ci-robot · 2020-03-27T01:49:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwhonce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jwhonce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rh-atomic-bot · 2020-03-27T12:55:31Z

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

mheon · 2020-03-27T13:42:48Z

Are we doing Varlink in V2? Can we get away with keeping varlink support on a 1.0 branch?

baude · 2020-03-27T14:10:28Z

Are we doing Varlink in V2? Can we get away with keeping varlink support on a 1.0 branch?

unfortunately, we are going to have to for some amount of time. i would love to talk about how to fence it off with a build tag so we could begin deprecating in fedora.

rhatdan · 2020-03-27T14:38:27Z

Var link will need to be supported for the length of RHEL8, I think or at least until RHEL8.6

rhatdan · 2020-03-27T14:39:19Z

We can begin deprecating it in Fedora 33.

vrothberg

Tested locally. Overall LGTM. One thing I noticed is that the service silently returns when the socket is already present. The previous behavior was to error out. I believe we should only error out iff the socket is in use.

vrothberg

There is some behavioral change. v1 throws an error if the socket already exists, v2 exits without an error.

rhatdan · 2020-04-03T18:51:11Z

/lgtm
/hold

rhatdan · 2020-04-07T23:54:51Z

@jwhonce Needs rebase.

* Added support for system service * Enabled linting on the varlinkapi source, needed to support V2 service command * Added support for PODMAN_SOCKET Skip linting deprecated code Rather than introduce bugs by correcting deprecated code, linting the code is being skipped. Code that is being ported into V2 is being checked. Signed-off-by: Jhon Honce <jhonce@redhat.com>

cevich · 2020-04-08T14:37:32Z

+[[ $1 == run ]] && shift
+
+for i in tunnel abi; do
+  echo Build Tags: ${BUILD_TAGS[$i]}


Nit: It's good practice to always quote string arguments to echo.

jwhonce · 2020-04-08T15:33:58Z

/hold cancel

baude · 2020-04-09T12:53:37Z

/lgtm

jwhonce requested a review from baude March 27, 2020 01:49

jwhonce self-assigned this Mar 27, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 27, 2020

openshift-ci-robot requested review from mheon and vrothberg March 27, 2020 01:49

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 27, 2020

jwhonce force-pushed the wip/service branch from 8237d49 to 257dfc7 Compare March 27, 2020 15:46

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2020

jwhonce force-pushed the wip/service branch from 257dfc7 to 5cb74cd Compare April 1, 2020 22:02

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 1, 2020

jwhonce changed the title ~~[WIP] V2 podman system service~~ V2 podman system service Apr 1, 2020

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 Apr 1, 2020

jwhonce force-pushed the wip/service branch 4 times, most recently from 1dcf215 to a37592e Compare April 2, 2020 23:01

jwhonce requested a review from cevich April 2, 2020 23:07

jwhonce force-pushed the wip/service branch from a37592e to 2a0df2b Compare April 2, 2020 23:08

vrothberg reviewed Apr 3, 2020

View reviewed changes

Comment thread hack/golangci-lint.sh Outdated

vrothberg reviewed Apr 3, 2020

View reviewed changes

Comment thread pkg/domain/entities/system.go Outdated

Comment thread pkg/domain/entities/system.go Outdated

jwhonce force-pushed the wip/service branch 2 times, most recently from 3268fed to 798f561 Compare April 3, 2020 17:35

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 3, 2020

openshift-ci-robot assigned rhatdan Apr 3, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2020

jwhonce force-pushed the wip/service branch from 798f561 to e93a342 Compare April 3, 2020 21:05

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 3, 2020

jwhonce force-pushed the wip/service branch from e93a342 to 4668764 Compare April 3, 2020 21:20

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 5, 2020

jwhonce force-pushed the wip/service branch from 4668764 to fe52546 Compare April 7, 2020 16:50

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 7, 2020

jwhonce force-pushed the wip/service branch from fe52546 to ced79b7 Compare April 8, 2020 01:47

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2020

jwhonce force-pushed the wip/service branch from ced79b7 to e0847f5 Compare April 8, 2020 02:22

cevich reviewed Apr 8, 2020

View reviewed changes

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 8, 2020

openshift-ci-robot assigned baude Apr 9, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2020

openshift-merge-robot merged commit 555b30e into containers:master Apr 9, 2020

rh-atomic-bot mentioned this pull request Apr 9, 2020

podmanv2 events #5770

Merged

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

Conversation

jwhonce commented Mar 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 27, 2020

Uh oh!

rh-atomic-bot commented Mar 27, 2020

Uh oh!

mheon commented Mar 27, 2020

Uh oh!

baude commented Mar 27, 2020

Uh oh!

rhatdan commented Mar 27, 2020

Uh oh!

rhatdan commented Mar 27, 2020

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rhatdan commented Apr 3, 2020

Uh oh!

rhatdan commented Apr 7, 2020

Uh oh!

cevich Apr 8, 2020

Choose a reason for hiding this comment

Uh oh!

jwhonce commented Apr 8, 2020

Uh oh!

baude commented Apr 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jwhonce commented Mar 27, 2020 •

edited

Loading