Add podman kube apply command

[umohnani8]

Add the abilitiy to deploy the generated kube yaml to a
kubernetes cluster with the podman kube apply command.
Add support to directly apply containers, pods, or volumes
by passing in their names or ids to the command.
Use the kubernetes API endpoints and http requests to connect
to the cluster and deploy the various kubernetes object kinds.

Signed-off-by: Urvashi Mohnani [email protected]

Does this PR introduce a user-facing change?

Add ability to deploy the generated yaml to a k8s cluster with `podman kube apply`

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: umohnani8

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:

• OWNERS [umohnani8]

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

[umohnani8]

@rhatdan @vrothberg @mheon PTAL

Added the "no new tests needed" as there is no good way to test this, we will need spin up a cluster every time for this.

There is no major change in binary size and we are not vendoring in anything new, just using the endpoints with http requests.

[umohnani8]

Any idea why the binary size for podman-remote is going up so much?

[rhatdan]

Could
ktypes "github.com/containers/podman/v4/pkg/k8s.io/api/core/v1"
Be causing the podman-remote to suck in a huge amount of code?

[mheon]

I fear that's it. We did a lot to disentangle Podman and K8S earlier, but this is probably going to drag a lot of those deps back in.

[umohnani8]

Okay, I moved most of the code to pkg/domain/infra/abi/generate.go. I think that should stop the deps from being pulled in.

[vrothberg]

I think this functionality deserves a dedicated command. It's similar to play kube --down. Flags are for new functionality make it very difficult to extend the functionality. Maybe we want to add flags for authentication etc.

In symmetry to kubectl we could add a kube apply command. WDYT?

[rhatdan]

I am fine with a new command now that we have the podman kube command. I don't fully understand how this is supposed to work.

[vrothberg]

I am fine with a new command now that we have the podman kube command. I don't fully understand how this is supposed to work.

I imagine that we can generate a YAML via kube generate (soon an alias for generate kube) which can then be run in a cluster via kube apply.

[umohnani8]

So the initial plan was podman generate kube would deploy the yaml generated automatically when some flags are set. Now we want to pass in the generated yaml to a new command called podman kube apply so the user would have to run 2 commands instead of 1.

I am fine with doing a separate kube command for this, but do we want to leave the flags for generate kube so users can still generate and deploy with one command?

[rhatdan]

When you say Deploy are you talking about to OpenSHift or to a Kubernetes cluster?

[umohnani8]

You can deploy to either as long as you have a kubeconfig. We are using the k8s API endpoints underneath.

[rhatdan]

I think this should definitely be a separate command then.

podman generate kube CONTAINER new.Yaml
podman kupe deply new.Yaml.

[mheon]

I would be OK with a --deploy flag to generate kube to not require two commands, but I concur that a dedicated command is a good idea too

[vrothberg]

Sorry for the wall of text. I found it worth elaborating on this design decision to keep the doors for future features open.

I am fine with doing a separate kube command for this, but do we want to leave the flags for generate kube so users can still generate and deploy with one command?

I am against a --deploy flag for generate kube as we are losing separation of concerns. It quickly gets complex when mixing two (somehow) separate features into one. The list of flags/options for kubectl apply is long. For instance, how would credentials and other optional parameters be passed? We probably had to namespace the flags (e.g., --deploy-user / --deploy-password) which screams for using a separate command as the namespace.

Maybe users want to login to a cluster only once, so we may need a kube login command in the future. Separating these concerns keeps things much easier to extend (and maintain) long term; a hypothetical bug in generate kube would not necessarily impact kube apply.

So the initial plan was podman generate kube would deploy the yaml generated automatically when some flags are set.

I wouldn't like to directly deploy a workload to a cluster without having inspected or even tested it (as it would with generate --deploy). If I use generate kube, I'd probably test it locally via kube play before deploying it to a cluster (that is potentially linked to higher costs).

Now we want to pass in the generated yaml to a new command called podman kube apply so the user would have to run 2 commands instead of 1.

But we do not reduce the feature to one use case only that can otherwise be piped, for instance, podman generate kube $myPOd | podman kube apply -.

What about deploying existing files? I had to run play | generate --deploy. The YAML I'd deploy would look different from the initial one which would drop my confidence level significantly as I did not test it. play would also alter local state which should not be required for remote deploying work.

Another potentital use case, could be a git-ops kind of workflow. I generate the YAML once, drop it to my repository and the fleet will take the YAML as is to deploy it.

Regarding testing: We need some testing somewhere. The feature is absolutely amazing, so it would be extremely painful to silently regress. Could we use something like kind and deploy to this local "cluster"?

[umohnani8]

This is ready for review.

Will be adding tests with #15826. Let's not block this on tests.

[rhatdan]

LGTM
@vrothberg @mheon @giuseppe PTAL
@containers/podman-maintainers PTAL

[vrothberg]

I am strictly against adding new functionality without tests. If others think differently, feel free to merge.

[umohnani8]

Added system tests for this.

Updated the command to accept container/pod id/name as well. So we can directly apply a container or pod to the cluster without having to generate a yaml file and saving it first. It calls kube generate and the applies the yaml file generated from that to the cluster.

[Luap99]

Some comments about the stuff I know, I cannot comment about the actual k8s logic.

[umohnani8]

I am a bit confused with what is going on with the shell completion issue here https://cirrus-ci.com/task/5611626027024384?logs=main#L6948, any idea what I am missing that the test is unhappy? @Luap99 @mheon

nvm, I think I see the issue

[umohnani8]

Tests are green, @containers/podman-maintainers PTAL

[Luap99]

I am a bit confused with what is going on with the shell completion issue here https://cirrus-ci.com/task/5611626027024384?logs=main#L6948, any idea what I am missing that the test is unhappy? @Luap99 @mheon

nvm, I think I see the issue

Sorry the test is very hard to understand. It only checks shell completion vs usage line, it is hard to know which of the two is wrong. I am open to suggestions if you think we can/should improve the error message.

[Luap99]

The cli bits LGTM but I cannot comment on the k8s api calls.

[rhatdan]

/lgtm
Good work @umohnani8 Now we need to get people playing with this and see if it works with multiple different kube environments.

[vrothberg]

Apologies for the review after the merge; I didn't find time after yesterday's meeting. Just one comment on exposing it on the REST API. I think we can skip the REST API and call the function directly in podman-remote.

[vrothberg]

This doesn't look right. We usually don't allow client to control reading/writing files on the host for security reasons. They're usually send over the wire in a tarball.

Do we need to expose this on the REST API at all? Couldn't we have podman and podman-remote call the same function?

We could move pkg/domain/infra/abi/apply.go into an internal package pkg/domain/infra/internal/apply.go that the infra/abi and infra/tunnel can call.

[Luap99]

Do we need to expose this on the REST API at all? Couldn't we have podman and podman-remote call the same function?

That is a good question. I guess it comes down to: Do we want to API callers use it, e.g. should podman-py or podman-desktop have this functionality?

We usually don't allow client to control reading/writing files on the host for security reasons.

I don't see how this is an argument? podman-remote run -v allows full access to the file system anyway.

[vrothberg]

Do we want to API callers use it, e.g. should podman-py or podman-desktop have this functionality?

I don't see a compelling reason. I see podman kube apply more for something during development but would recommend kubectl for production use.

I don't see how this is an argument? podman-remote run -v allows full access to the file system anyway.

That's an unfortunate circumstance but shouldn't justify more unfortunate circumstances. A probably more convincing reason is that users will expect podman-remote --file foo to be evaluated on the client side not on the relative path of the server on a remote machine.

[Luap99]

I don't see how this is an argument? podman-remote run -v allows full access to the file system anyway.

That's an unfortunate circumstance but shouldn't justify more unfortunate circumstances. A probably more convincing reason is that users will expect podman-remote --file foo to be evaluated on the client side not on the relative path of the server on a remote machine.

Sure I agree there and it would make it more consistent with the other commands. Just saying we should not argue with security. The train is long gone, access to the podman socket allows arbitrary code execution, this is by design of how podman run works.

[vrothberg]

You are correct to put it into context but I will always argue with security. One potentially dangerous feature should not justify making others less secure. It's possible to deny the use of "dangerous" parameters, for instance. The more there are, the harder it gets.