[WIP] Initial implementation of a spec generator package #4513
Conversation
The current Libpod pkg/spec has become a victim of the better part of three years of development that tied it extremely closely to the current Podman CLI. Defaults are spread across multiple places, there is no easy way to produce a CreateConfig that will actually produce a valid container, and the logic for generating configs has sprawled across at least three packages. This is an initial pass at a package that generates OCI specs that will supersede large parts of the current pkg/spec. The CreateConfig will still exist, but will effectively turn into a parsed CLI. This will be compiled down into the new SpecGenerator struct, which will generate the OCI spec and Libpod create options. The preferred integration point for plugging into Podman's Go API to create containers will be the new CreateConfig, as it's less tied to Podman's command line. CRI-O, for example, will likely tie in here. Signed-off-by: Matthew Heon <[email protected]>
openshift-ci-robot
added
do-not-merge/work-in-progress
|
This is an initial skeleton of what the new configuration will look like, with no implementation code as yet. @haircommander @baude PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon 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 |
openshift-ci-robot
added
the
approved
| // Default is private. | ||
| // Conflicts with ShmSize if not set to private. | ||
| // Mandatory. | ||
| IpcNS Namespace |
There was a problem hiding this comment.
I'm unsure with the location here. I get that Shm is handled similarly to other mounted volumes, but having namespace configuration (that pods can share, for instance) scattered about a bit confusing
| ContainerCgroupConfig | ||
| ContainerNetworkConfig |
There was a problem hiding this comment.
it's also similarly odd that cgroup and net namespaces get their own config. (cgroup has three values, but user has two).
There was a problem hiding this comment.
The original implementation had a single 300-odd like monolithic struct, but I tried to split it a bit to make things easier to parse. I don't know how successful this was.
There was a problem hiding this comment.
I love the general structure! the nits are more about how to split it up. (it's pretty much the same notes I had in #4265)
|
This pull request had no activity for 30 days. In the absence of activity or the "do-not-close" label, the pull request will be automatically closed within 7 days. |
|
☔ The latest upstream changes (presumably #5082) made this pull request unmergeable. Please resolve the merge conflicts. |
openshift-ci-robot
added
the
needs-rebase
|
@mheon: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Merged elsewhere by @baude so closing |
github-actions
bot
added
the
locked - please file new issue/PR
The current Libpod pkg/spec has become a victim of the better part of three years of development that tied it extremely closely to the current Podman CLI. Defaults are spread across multiple places, there is no easy way to produce a CreateConfig that will actually produce a valid container, and the logic for generating configs has sprawled across at least three packages.
This is an initial pass at a package that generates OCI specs that will supersede large parts of the current pkg/spec. The CreateConfig will still exist, but will effectively turn into a parsed CLI. This will be compiled down into the new SpecGenerator struct, which will generate the OCI spec and Libpod create options.
The preferred integration point for plugging into Podman's Go API to create containers will be the new CreateConfig, as it's less tied to Podman's command line. CRI-O, for example, will likely tie in here.