podman rmi: refactor logic #5863
Conversation
openshift-ci-robot
added
the
approved
|
I did not test the remote client yet but feel free to review. |
|
@vrothberg gating isn't hip |
|
Quick meeting record: we agreed to generalize the consolidation/code-share issue in the PR and make pkg/api/hanlders use the infra/abi code. |
|
These are examples of where the CLI instantiates the runtime engines. PodmanConfig might be too generous of input, we may need to narrow the data being passed in. |
vrothberg
force-pushed
the
v2-fix-rmi
branch
2 times, most recently
from
33118de to
e0db52e
Compare
|
☔ The latest upstream changes (presumably #5872) made this pull request unmergeable. Please resolve the merge conflicts. |
| func (ic *ContainerEngine) VarlinkService(_ context.Context, opts entities.ServiceOptions) error { | ||
| var varlinkInterfaces = []*iopodman.VarlinkInterface{ | ||
| iopodmanAPI.New(opts.Command, ic.Libpod), |
There was a problem hiding this comment.
| func (ic *ContainerEngine) VarlinkService(_ context.Context, opts entities.ServiceOptions) error { | |
| var varlinkInterfaces = []*iopodman.VarlinkInterface{ | |
| iopodmanAPI.New(opts.Command, ic.Libpod), | |
| func (ic *ContainerEngine) VarlinkService(_ context.Context, opts entities.ServiceOptions) error { | |
| var varlinkInterfaces = []*iopodman.VarlinkInterface{ | |
| iopodmanAPI.New(opts.Command, ic.Libpod), |
Should this move into cmd/podman as well?
There was a problem hiding this comment.
Yes, this would be another nice candidate. For now, this PR focuses on podman rmi.
vrothberg
force-pushed
the
v2-fix-rmi
branch
3 times, most recently
from
b4a2e3c to
ba8be97
Compare
|
swagger failures are fixed now. @jwhonce @baude, the issue I faced is that swagger went south because pkg/api now imports infra/abi. infra/abi had the build header set to ABISupport which in turn confused swagger because it couldn't parse it anymore. I just removed the headers as the imports are guarded in infra. There's now way to point swagger to some build tags and go doesn't support setting them via environment variables either. Should be good now 👍 |
|
ok LGTM |
|
☔ The latest upstream changes (presumably #5899) made this pull request unmergeable. Please resolve the merge conflicts. |
While this commit was initially meant to fix containers#5847, it has turned into a bigger refactoring which I did not manage to break into smaller pieces: * Fix containers#5847 by refactoring the image-removal logic. * Make the api handler for image-removal use the ABI code. This way, both (i.e., ABI and Tunnel) end up using the same code. Achieving this code share required to move some code around to prevent circular dependencies. * Everything in pkg/api (excluding pkg/api/types) must now only be accessed from code using `ABISupport`. * Avoid imports from entities on handlers to prevent circular dependencies. * Move `podman system service` logic into `cmd` to prevent circular dependencies - it depends on pkg/api. * Also remove the build header from infra/abi files. It will otherwise confuse swagger and other tools; errors we cannot fix as go doesn't expose a build-tag env variable. Fixes: containers#5847 Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
github-actions
bot
added
the
locked - please file new issue/PR
While this commit was initially meant to fix #5847, it has turned into a
bigger refactoring which I did not manage to break into smaller pieces:
Fix podmanV2 rmi issues #5847 by refactoring the image-removal logic.
Make the api handler for image-removal use the ABI code. This way,
both (i.e., ABI and Tunnel) end up using the same code. Achieving
this code share required to move some code around to prevent circular
dependencies.
Everything in pkg/api (excluding pkg/api/types) must now only be
accessed from code using
ABISupport.Avoid imports from entities on handlers to prevent circular
dependencies.
Move
podman system servicelogic intocmdto prevent circulardependencies - it depends on pkg/api.
Fixes: #5847
Signed-off-by: Valentin Rothberg [email protected]