Clearify error message if user tries to uninstall a nonkube site.#2283
Open
lynnemorrison wants to merge 1 commit intoskupperproject:mainfrom
Open
Clearify error message if user tries to uninstall a nonkube site.#2283lynnemorrison wants to merge 1 commit intoskupperproject:mainfrom
lynnemorrison wants to merge 1 commit intoskupperproject:mainfrom
Conversation
lynnemorrison
requested review from
c-kruse,
fgiorgetti and
nluaces
as code owners
lynnemorrison
force-pushed
the
v2_uninstall
branch
3 times, most recently
from
d7874ce to
93b018b
Compare
fgiorgetti
reviewed
Oct 10, 2025
Member
fgiorgetti
left a comment
fgiorgetti
left a comment
There was a problem hiding this comment.
The message is much clearer now.
But I recommend validating that the error is a *net.OpError, before advising
user to run the skupper system install command, as if something else happend,
the recommendation can be incorrect.
Thoughts?
lynnemorrison
force-pushed
the
v2_uninstall
branch
from
93b018b to
32e53aa
Compare
lynnemorrison
force-pushed
the
v2_uninstall
branch
from
32e53aa to
97a829f
Compare
fgiorgetti
reviewed
Oct 16, 2025
| if err != nil { | ||
| return fmt.Errorf("failed to create container client: %v", err) | ||
| errMsg := fmt.Errorf("failed to create container client: %v", err) | ||
| netErr := &net.OpError{ |
Member
There was a problem hiding this comment.
I think it is safer to just wrap the error by switching %v by %w, with no need to force returning a net.OpError. This way, the validation you're using with errors.As still works fine if the actual error is not a net.OpError.
lynnemorrison
force-pushed
the
v2_uninstall
branch
from
97a829f to
ee6382e
Compare
nluaces
approved these changes
Nov 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If system-controller or podman socket endpoint is not installed and user issued a "skupper system uninstall" command
the error message is confusing. This fix simplifies the error message and provides help on what user should do.
fixes #2281