containers / podlet

Generate Podman Quadlet files from a Podman command, compose file, or existing object
https://crates.io/crates/podlet
Mozilla Public License 2.0
318 stars 10 forks source link

podlet compose --pod appears to potentially have logic/design issues with unsupported options #66

Closed NoX1De closed 3 months ago

NoX1De commented 3 months ago

My goal with trying out podlet was to convert a docker compose stack into a Kubernetes yaml file and also have a .kube generated, so based on the help section of podlet I ran the following command:

$ podlet compose --pod ./docker-compose.yml
Error:
   0: could not convert service `test` into k8s container spec
   1: pods do not support per container `restart` options, try setting the pod option instead

Location:
   src/cli/k8s.rs:201

This error does not seem logical for a few reasons...first, I am already using the --pod option, second when I REMOVE the --pod option podlet actually runs and outputs quadlet text for my docker-compose.yml which was a bit baffling. Third, is this the correct approach that is implemented here by podlet?

I'm assuming the error message stating "try setting the pod option instead" is telling the user to use the --pod option of podlet compose. So that assumed here...why is it not checking to see if --pod is already being used? Additionally, the point of conversion here is, well, conversion...so since pods do not support certain per container options such as restart etc. why wouldn't podlet just automatically (or after prompting the user for a decision) simply remove or (if an equivalent exists) substitute those options for a compatible comparable equivalent as part of the conversion to then output a useful a Kubernetes YAML file etc.?

https://github.com/k9withabone/podlet/blob/c1a057f266c8de1728474133f8856bf46c06c87e/src/cli/k8s.rs#L188-L206

k9withabone commented 3 months ago

I definitely agree that the error message could be worded better. What I meant with it is to set the option on the pod in the generated Kubernetes YAML, after you have removed / commented out the option from your compose file and run podlet again. For example, in this case, you would remove the restart field from your service, run podlet again, and set the restartPolicy in the pod spec.

The reason the restart policy and the other options in the quoted code return an error instead of just ignoring it or setting the k8s pod option is that either of those would make Kubernetes behave differently from compose, violating the principle of least surprise. Also, multiple containers could have conflicting options or the user may not want an option applying to the whole pod instead of just one container. Unfortunately, Kubernetes doesn't support any granularity for these options in its pod spec.

I like the idea of prompting the user, but I think it might be too difficult / complex to implement nicely. There are a number of people who use podlet in non-interactive contexts as well. I personally don't think it's too much to ask the user to make minor edits to output files themselves since they will probably want to edit it anyway. It also ensures, to a degree, that they understand the behavior differences from compose. Podlet is meant as a starting point for the conversion to quadlets / Kubernetes YAML, not necessarily the end in itself.

NoX1De commented 3 months ago

I tried to comment out all of the instances of restart in my docker-compose.yml for this stack and podlet seemed to still produce the same error? I'm confused as to why this is, perhaps it's not recognizing that those lines are commented out?

$ cat docker-compose-convert.yml | grep 'restart'
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
#    restart: unless-stopped
$ podlet compose --pod ./docker-compose-convert.yml
Error:
   0: could not convert service `test` into k8s container spec
   1: pods do not support per container `restart` options, try setting the pod option instead

Location:
   src/cli/k8s.rs:201

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
k9withabone commented 3 months ago

I think I know what is happening. When you run podlet compose --pod ./docker-compose-convert.yml, podlet is using ./docker-compose-convert.yml as the name of the pod, not as the compose file. You must have a compose.yaml, compose.yml, docker-compose.yaml, or docker-compose.yml file in your current directory. Podlet is using that file as the input. The podlet compose --pod option requires a name for the pod as an argument. From podlet compose --help:

Generate podman quadlet files from a compose file

Creates a `.container` file for each service, a `.volume` file for each volume, and a `.network` file for each network.

The `--file` option must be a directory if used.

Some compose options are not supported, such as `build`.

When podlet encounters an unsupported option, an error will be returned. Modify the compose file to resolve the error.

Usage: podlet compose [OPTIONS] [COMPOSE_FILE]

Arguments:
  [COMPOSE_FILE]
          The compose file to convert

          If `-` or not provided and stdin is not a terminal, the compose file will be read from stdin.

          If not provided, and stdin is a terminal, podlet will look for (in order) `compose.yaml`, `compose.yml`, `docker-compose.yaml`,
          and `docker-compose.yml`, in the current working directory.

Options:
      --pod <POD>
          Create a Kubernetes YAML file for a pod instead of separate containers

          A `.kube` file using the generated Kubernetes YAML file will also be created.

  -h, --help
          Print help (see a summary with '-h')
k9withabone commented 3 months ago

@NoX1De did you resolve the issue?

k9withabone commented 3 months ago

I'm going to close this. I'll try to add a better error message in the next release.