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

Ensure remote shells generated by SCVMM are closed when finished #14591

Merged
merged 2 commits into from
Apr 3, 2017
Merged

Ensure remote shells generated by SCVMM are closed when finished #14591

merged 2 commits into from
Apr 3, 2017

Conversation

djberg96
Copy link
Contributor

The WinRM shells being created for SCVMM are not being closed properly. It seems we did not completely update the code for WinRM 2.x completely when we upgraded. The result is that sometimes the server will refuse the connection until some of the existing shells eventually time out. This fixes that by ensuring a shell is closed once finished.

It also takes advantage of the stdout and stderr methods, and removes an unnecessary variable assignment.

Addresses https://bugzilla.redhat.com/show_bug.cgi?id=1436180

Replaces #13637

shell.close
end
end
end
Copy link
Member

Choose a reason for hiding this comment

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

@djberg96 can you place both "with_winrm_shell" definitions contiguous in the source file? Since you're adding both of them I can see no reason to have them at opposite ends of the file. Otherwise LTGM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jerryk55 Ok, I reorganized the code as requested.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@miq-bot
Copy link
Member

miq-bot commented Apr 3, 2017

Checked commits https://github.com/djberg96/manageiq/compare/54ec2614becbde33fb6893e445dd55caae0b06fb~...6643953454105765f1074225ec75873a0e3a0619 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 👍

@jerryk55
Copy link
Member

jerryk55 commented Apr 3, 2017

@djberg96 latest changes good. 👍

@djberg96
Copy link
Contributor Author

djberg96 commented Apr 3, 2017

@miq-bot add_label fine/yes

@miq-bot
Copy link
Member

miq-bot commented Apr 3, 2017

@djberg96 unrecognized command 'fine', ignoring...

Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

@djberg96
Copy link
Contributor Author

djberg96 commented Apr 3, 2017

@miq-bot add_label darga/no

@djberg96
Copy link
Contributor Author

djberg96 commented Apr 3, 2017

@miq-bot add_label euwe/no

@blomquisg blomquisg merged commit 5f36311 into ManageIQ:master Apr 3, 2017
@blomquisg blomquisg added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 3, 2017
simaishi pushed a commit that referenced this pull request Apr 3, 2017
Ensure remote shells generated by SCVMM are closed when finished
(cherry picked from commit 5f36311)

https://bugzilla.redhat.com/show_bug.cgi?id=1438593
@simaishi
Copy link
Contributor

simaishi commented Apr 3, 2017

Fine backport details:

$ git log -1
commit 5c92c904846f4b13dd58f670f82453f51e86aca8
Author: Greg Blomquist <blomquisg@gmail.com>
Date:   Mon Apr 3 14:17:20 2017 -0400

    Merge pull request #14591 from djberg96/powershell4
    
    Ensure remote shells generated by SCVMM are closed when finished
    (cherry picked from commit 5f36311ecbc827d7665d5e377ab7eb78b5d766f3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1438593

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants