Skip to content

Parsing changes for podman support#303

Closed
Rory-Reid wants to merge 5 commits into
mariotoffia:masterfrom
Rory-Reid:parse-podman
Closed

Parsing changes for podman support#303
Rory-Reid wants to merge 5 commits into
mariotoffia:masterfrom
Rory-Reid:parse-podman

Conversation

@Rory-Reid
Copy link
Copy Markdown
Contributor

This is a summary of issues I have worked on in order to get closer to an initial pass for supporting podman (#300)

I've tried to do them as podman-agnostic as possible (more general defensive approaches to parsing slightly different formats of things) but it's a non-zero amount of complexity added now for this. I understand if that's not something you'd be interested in bringing into the library.

This includes a failing test for a result observed with podman
The only place this is used (.WaitForHealthy()) already performs a null-
safe check against the Status, comparing it to "Healthy" only. Changing
the property to nullable allows more defensive parsing if it is an empty
string for whatever reason, and is still interpreted as "Unhealthy"
where it matters.
This adds and uses a custom converter which will parse
Container.Config.Entrypoint from a json string array I.E.

```json
{
  "EntryPoint": [
    "my-entrypoint"
  ]
}
```

Or from a single string value, I.E:

```json
{
  "EntryPoint": "my-entrypoint"
}
```
In some cases the `--format` specification doesn't match the output
resulting in an error such as:

`Error: template: ls:1:43: executing "ls" at <.Scope>: can't evaluate field Scope in type network.ListPrintReports"`

If the full command is unsuccessful, this falls back to parsing just the
ID and Name, which prevents crashes at the cost of a less rich
experience.
This introduces a slightly more complex parsing method for timespans
in the ProcessRow class in order to deal with more diverse formats (such
as the format output by `podman top` - 1h2m3s vs 01:02:03).

This also adds a simple test to validate the parser's current behaviour
with docker, and a test to confirm it can parse podman output too
without crashing.
@Rory-Reid Rory-Reid mentioned this pull request Sep 29, 2023
@mariotoffia
Copy link
Copy Markdown
Owner

Hi @Rory-Reid and many thanks for this excellent PR!!

It seems to fail in two tests, I currently do not have the time to check why? Could you have a quick check and see if it is the current implementation affecting it?

Cheers,
Mario :)

@Rory-Reid
Copy link
Copy Markdown
Contributor Author

Rory-Reid commented Nov 5, 2023

Hi @mariotoffia and firstly thanks for looking at my PR! I appreciate it's not a small one!

Ah, yeah, this is the same two tests that are failing on the master branch as far as I can see - PullContainerBeforeRunningShallWork and ContainerHealthCheckShallWork.

Both of these passed locally for me so I assumed it was environmental and wasn't familiar with the CI setup but on rerun I can see they pass with an error in my logs because I don't have docker-machine ("Failed to find docker-machine client binary - please add it to your path").

Given they're currently failing on master I don't think they're related to my changes but I was purposefully leaving this PR as a draft until I could rebase onto a working build to prove that. I'll see if I can prove anything locally at least around this sometime soon.

@sonarqubecloud
Copy link
Copy Markdown

mariotoffia added a commit that referenced this pull request May 11, 2026
…reams (#334)

* ProcessRow: tolerate Podman top duration formats

Podman top emits durations like "0s", "12m34s", "1h2m3s" for the START
and TIME columns; the strict TimeSpan.Parse path threw on these. Add a
fallback chain through the podman shapes so cross-engine consumers
don't see a FormatException.

The helper signatures and the four-step parse cascade come from
@Rory-Reid's PR #303 — adopted essentially verbatim. Adds 13 unit
tests (parametrised theories) covering all three Podman shapes plus
the existing hh:mm:ss form and a garbage-input regression.

Co-authored-by: Rory Reid <Rory-Reid@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* CLI drivers: force UTF-8 on stdout/stderr

ProcessStartInfo defaults StandardOutputEncoding/StandardErrorEncoding
to the system code page, which mangles non-ASCII output on Windows
and on systems with a non-UTF-8 locale. The Docker API driver already
pins UTF-8 (DockerApiDriverBase.cs); the CLI drivers were missed.

Sets Encoding.UTF8 on both pipes in all three ProcessStartInfo paths
(capture, streaming, attach) for the Docker and Podman CLI drivers —
six identical two-line additions. Most plausible suspect behind
community report #329 (WaitForMessageInLogs not working with Podman).

Idea and original diff: @jellever's PR #331.

Co-authored-by: Jelle Vergeer <jellever@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Docker API driver: harden inspect parsing for Podman-compat responses

When the Docker API driver is pointed at podman-api-compat the inspect
response surfaces three quirks that the previous parser tripped on:

1. Entrypoint / Cmd may arrive as a bare string instead of an array.
   Swap GetStringArray to the existing GetStringOrArray extension for
   both fields — the extension already exists in JsonElementExtensions
   and is exercised by JsonElementExtensionsTests.

2. The State.Health block was never parsed at all by the Docker API
   driver, so callers always saw State.Health == null. Add a ParseHealth
   helper mirroring the Podman CLI parser (PodmanContainerParser.cs).

3. Inside State.Health, Status may be an empty string instead of being
   omitted; Enum.TryParse on "" returns false. Default to
   HealthState.Unknown when Status is missing, empty, or unrecognised.

Idea, JSON fixtures, and defensive empty-string handling: @Rory-Reid's
PR #303. Adds 7 unit tests covering Entrypoint-as-string,
Cmd-as-string, Health healthy/unhealthy/empty/missing, and Log entry
parsing.

Co-authored-by: Rory Reid <Rory-Reid@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Rory Reid <Rory-Reid@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Jelle Vergeer <jellever@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants