Display error message when odo dev fails on podman and clean resources #6522
Conversation
✅ Deploy Preview for odo-docusaurus-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
pkg/podman/podman.go
Outdated
| // the last line is an empty new line; so we revert to the second last line for error | ||
| errLine := out[len(out)-2] | ||
| err = fmt.Errorf("%s: %s\n%s", err, string(exiterr.Stderr), errLine) |
There was a problem hiding this comment.
Why do we want to display only the last non-blank line, and not the entire content?
This seems very specific to a use-case where the error message would be in the last non-blank line.
There was a problem hiding this comment.
The last non-blank line points to the error. But, perhaps we can show the whole log in case of failure.
There was a problem hiding this comment.
On a second thought, the last line (err line) usually sums up the core issue, so showing just that should be fine. WDYT?
There was a problem hiding this comment.
I would prefer for the first versions to display the complete output, to have the maximum of information. If we consider it is too much, we could reduce it
pkg/dev/podmandev/cleanup.go
Outdated
| compName := odocontext.GetComponentName(ctx) | ||
| appName := odocontext.GetApplication(ctx) | ||
| name, err := util.NamespaceKubernetesObject(compName, appName) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| o.deployedPod = &corev1.Pod{} | ||
| o.deployedPod.SetName(name) |
There was a problem hiding this comment.
Is this related to this PR? What is the problem fixed by this change?
There was a problem hiding this comment.
In case of some failures, pod is still created, this change attempts at fixing this problem.
There was a problem hiding this comment.
I would prefer to fix the problem why deployedPod is null when a Pod is actually deployed
There was a problem hiding this comment.
Podman creates the pod even if something is incorrect with the manifest, for e.g. non-existent image.
I've modified the code to check if the pod exists before marking it as deployed.
valaparthvi
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
valaparthvi
force-pushed
the
podman-err-out
branch
from
3e28565
to
7fb4036
Compare
valaparthvi
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
openshift-merge-robot
added
the
needs-rebase
…s on error Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
valaparthvi
force-pushed
the
podman-err-out
branch
from
27545c5
to
5879bd0
Compare
openshift-merge-robot
removed
the
needs-rebase
openshift-ci
bot
added
the
lgtm
|
Kudos, SonarCloud Quality Gate passed!
|
|
/override windows-integration-test/Windows-test |
|
@feloy: Overrode contexts on behalf of feloy: windows-integration-test/Windows-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Parthvi Vala pvala@redhat.com
What type of PR is this:
/kind bug
What does this PR do / why we need it:
This PR ensures
odo devon podman exits with actual failure from podman and also cleans up pod if it was created.Which issue(s) this PR fixes:
Fixes #6487
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
odo init --devfile nodejs --starter nodejs-starter --name my-nodejs-appsed -i "s/registry.access.redhat.com\/ubi8\/nodejs-16:latest/adsffdfe/" devfile.yamlODO_EXPERIMENTAL_MODE=true odo dev --platform=podmanFor 6501
export PODMAN_CMD=echoODO_EXPERIMENTAL_MODE=true odo dev --platform=podman