Add --log-file argument and log test result details

[KyleFromNVIDIA]

Add a --log-file argument for container-canary to write detailed logs, so that developers can see exactly how and why a test failed.

[jameslamb]

I'd like to revive this, but I don't have write access here.

@KyleFromNVIDIA could you update this to latest main and add a PR description?

And @jacobtomlinson after that could you help us out with a review?

[jacobtomlinson]

Thanks for the patience on the review here.

I'm keen to dig into the motivation here, particularly around logging to a file. I expect canary will be used mostly in CI, so I would want output to be logged to stdout where it can be captured by the CI runner rather than disappearing into a file.

If I understand correctly the goal here is to provide more verbose messages around failures. Why not use the existing console output to supply this information?

[jacobtomlinson]

I'm curious about the motivation here. This feels orthogonal to the rest of this PR.

It's my understanding that Kubeflow only expects a service on port 8888, not any particular path on that web server. You could equally run VSCode Server or some other application and expect it to work.

[KyleFromNVIDIA]

This PR has some bug fixes and tests too. One of the bugs was that it wasn't checking the status code when doing httpGet. I fixed that, but as a result this test started failing (I don't remember what the return code was). GETting a different path got a successful status code.

[jacobtomlinson]

Why not use stdout? That gives more flexibility for redirection.

[KyleFromNVIDIA]

I believe stdout is already consumed by the user-readable output. Writing to a separate log file allows us to get separate machine-readable output, and CI can always cat the file if we want the developer to see it (that's what I plan on doing).

[jacobtomlinson]

@jameslamb I've added you to the codeowners team here. @KyleFromNVIDIA I was unable to add you as you're not a member of the @NVIDIA org, we probably need to figure out how to add you so that you can be added to the codeowners.

[KyleFromNVIDIA]

I was unable to add you as you're not a member of the @NVIDIA org, we probably need to figure out how to add you so that you can be added to the codeowners.

I've fixed this, you can go ahead and add me.

[KyleFromNVIDIA]

@jameslamb @jacobtomlinson Is there anything else left to address here?

[jacobtomlinson]

I'm still not sold on the design here I'm afraid. Writing detailed information to a logfile feels unusual in the context of this tool.

A core goal of container canary is to fit in comfortably in the cloud tooling ecosystem and behave in ways that match the expectations of users of cloud-native tooling like Kubernetes and Docker.

Given that canary is a tool that is intended to be used primarily in CI or remote environments it's much more common to always write to stdout/stderr with a configurable level of verbosity. A large motivation for this is that we assume the environment is ephemeral and the filesystem is lost after the job completes, but the standard streams are captured by an external logging service.

Adding a flag to write to a log file, then adding a cat command feels like a long winded way to just bump up the verbosity with a flag.

If there is strong motivation to merge this as it is I'd love to see some examples of other tools that follow this pattern of writing a more verbose stream to a separate file on disk. Especially tools which we expect to be commonly used alongside canary.