Podman API takes a SpecGenerator rather than CreateOptions, why?

[cdoern]

Discussed in #12820

^{Originally posted by cdoern January 11, 2022}
@containers/podman-maintainers I have been grappling with this for a while when implementing different pod create options...

The last domino in redesigning the infra container for easier implementation of pod creation options seems to be podman-remote as well as the API itself.

Currently in cmd/podman/pods/create.go, the infra container's options are formed from the command line and turned into a spec generator in that file. If the user is executing the podman pod create command via podman-remote, all of this work is basically thrown out.

This is because pkg/api/handlers/libpod/pods.go PodCreate takes a PodSpecGenerator as the request input, decoding directly into one of these structs. This causes the process of creating the pod to be duplicated, filling out the specgen again, and manually inputting the infra information since we do not have flags to place the create options.

This also causes unecessary fields to be added to the PodSpecGenerator that are really container SpecGenerator fields that need to be carried over for the remote API...

A good example I am working on now is SysCtls:

1. cmd/podman/pods/create.go populates the create option for sysct of type []stringl and gives it to the infra container's spec via FillOutSpecGen, validating it as a []string map.

Step 1 is all we need for a regular podman and podman-remote approach technically, steps 2 and 3 are only done because of the requirements of the API

2. the original ContainerCreateOption of type []string is put back into the PodCreateOptions and then re validated and put into the PodSpecgen as a []string map for the remote path.

3. pkg/api/handlers/libpod/pods.go then receives the PodSpecgen, ignoring infra's if it is already filled out, and recreates infra's SpecGenerator manually and by marshaling/unmarshaling things that happen to match

This is a redundant process for every case except for the direct API usage.

I would like to have a conversation about either:

1. using create options rather than a Specgen for the API input

2. making a less roundabout way of populating a ContainerCreateOptions struct so infra is formed properly. Maybe there is a way to somehow map a PodSpecgen back to ContainerCreateOptions?

Looking at cmd/podman and pkg/api shows some commonality that should somehow be joined, and aren't the create options more "user friendly" instead of exposing the Specgen to the API users?

I am converting this to an issue as it is exemplified with issues like #14233. The main issue here is that the work done for the infra container redesign basically is undone by the tunnel path since InfraContainerSpec within PodSpecGen cannot be marshalled because if it was, it would expose too many options to user of remote pod create. I think we should convert the API to using create options across the map rather than a specgen.

[cdoern]

@rhatdan @jwhonce @mheon PTAL at this, I created a discussion a while back and got no traction, the issue keeps popping up, let me know what you think!

[rhatdan]

I am fine with this, but I will leave it up to @mheon for approval.

[baude]

The API must be specgen. I cannot remember all the details, but that was a lesson learned. Even if we agreed, you would have to wait for Podman 5 to do this. Maybe @mheon or @jwhonce will remember or have more to say.

[cdoern]

@baude I definitely see why its specgen from the podman-remote side of things: since we go through cmd/podman first and then get directed to tunnel's ___Create() functions, going back to createopts would be weird. but looking at it from a CLI standpoint the API (when directly accessing the socket) takes much different params than native podman. Maybe there is some midpoint we can reach where a redirect happens on the interior (a remotePod/Ctr specgen struct that has different options than the native specgen). Else, the more pod create options we add ... the more bloated cmd/podman will become, each time infra gets funneled an option that is "infra-specific" I need to create a matching entity in the pod spec/config that only gets utilized in the remote path.

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

@cdoern what is going on with this issue?

[cdoern]

@rhatdan I am working on getting this implemented. It is a 5.0 feature so we have time, but it involves a lot of redesign so I thought I would get started.

[vrothberg]

If possible, lets try our best to not break existing APIs. If users use the libpod API directly, they should not break on each (~yearly) major version bump.

[cdoern]

@vrothberg I am trying to make it so that specifying infra_container_spec is not necessary. If I can do that, it is a non breaking change. We did discuss at cabal though that we were ok with going ahead with a breaking change if necessary

[vrothberg]

@vrothberg I am trying to make it so that specifying infra_container_spec is not necessary. If I can do that, it is a non breaking change.

That would be great, thanks!

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[rhatdan]

@cdoern @vrothberg Please update what is going on here?

[cdoern]

I am still working on this, slated for a 5.0 timeline so I did not want to get too far ahead too quickly because this will be a major structural change @rhatdan

[vrothberg]

@rhatdan @baude @Luap99

I do think we'll find time to work on it.

[mheon]

Retagging 6.0