Trust docker inspect exit code for runtime label detection #1976
Conversation
bmanan7
requested review from
jeremybeard,
karankhanchandani,
kushalmalani,
neel-astro and
pgvishnuram
as code owners
Pull Request Test Coverage Report for Build f28c061d-31b9-44a0-82df-9b4effda4befDetails
💛 - Coveralls |
karankhanchandani
left a comment
karankhanchandani
left a comment
There was a problem hiding this comment.
Thanks for the PR. LGTM. Need additional review from astro team as this will affect their functionality as well.
| if execWarn := strings.TrimSpace(stderr.String()); execWarn != "" { | ||
| logger.Debugf("container runtime stderr while inspecting %s: %s", imageName, execWarn) |
There was a problem hiding this comment.
Why are we ignoring this? It's still not clear to me 🤔. The image that is mentioned in the issue description astrocrpublic.azurecr.io/astronomer/astro-runtime:3.1-2 is invalid, and thus the get label fails at this point, right?
There was a problem hiding this comment.
Oops, I didn’t update the issue description correctly before - that's on me. The screenshot was still accurate. I've now updated the description; please take another look and let me know if it makes sense.
There was a problem hiding this comment.
Can we then specifically ignore when stderr has the string Emulate Docker CLI using podman, so that we still catch other issues?
Description
astro deployno longer aborts for valid runtime images.🎟 Issue(s)
Related #1975
🧪 Functional Testing
dockerto print the Podman compatibility banner (shim wraps the real Docker binary).astro deploy -n <deployment name>with a Dockerfile usingFROM astrocrpublic.azurecr.io/runtime:3.1-2; deployment now proceeds past the runtime check.astro deploywith a non-runtime base image (FROM python:3.11-slim); warning still fires and deploy cancels (confirming real failures remain intact).📸 Screenshots
📋 Checklist
make testbefore taking out of draftmake lintbefore taking out of draft