Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get errors from winrm commands #17788

Merged
merged 4 commits into from
Apr 5, 2018
Merged

get errors from winrm commands #17788

merged 4 commits into from
Apr 5, 2018

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Apr 5, 2018

While this may not completely solve the open issues around winrm failures, updating winrmcp and getting the remote errors may help troubleshoot the issues.

The main goal here was to return possible errors from winrm commands.
The command error field in not exported, and only returned from the client Run* methods. Replace our runCommand with the winrm equivalent Client.Run.

Update the winrmcp package.

Update the log format of both communicators.

"versionExact": "master"
},
{
"path": "github.com/packer-community/winrmcp...",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path looks suspicious... was this a govendor fetch typo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is, thanks, I though I caught that before committing :/

@@ -536,7 +540,7 @@ func scpUploadFile(dst string, src io.Reader, w io.Writer, r *bufio.Reader, size
defer os.Remove(tf.Name())
defer tf.Close()

log.Println("Copying input data into temporary file so we can read the length")
log.Println("[DBEUG] Copying input data into temporary file so we can read the length")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[DBEUG]

Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one more typo on my second read through here but otherwise this looks good to me!

Ensure correct formatting and add a log level to all output.
The error from a remote command is not exported, and only exposed via
the Run method. Otherwise the Run method works exactly like the
runCommand function being removed.
Match the tested behavior, and that of the ssh implementation, where the
communicator automatically connects when starting a command.

Remove unused import from legacy dependency handling.
@jbardin
Copy link
Member Author

jbardin commented Apr 5, 2018

Thanks! pushed a fix for that last typo

@jbardin jbardin merged commit 79d0a5c into master Apr 5, 2018
@jbardin jbardin deleted the jbardin/communicators branch April 5, 2018 17:05
@ghost
Copy link

ghost commented Apr 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants