Fixed cockpit process starting #20077
Merged
Conversation
This file contains 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
jrafanie
reviewed
Apr 16, 2020
| @@ -151,7 +151,7 @@ def cockpit_ws_run | |||
| Bundler.with_clean_env do | |||
| stdin, stdout, stderr, wait_thr = Open3.popen3(env, *cockpit_ws.command(BINDING_ADDRESS), :unsetenv_others => true) | |||
| end | |||
| stdin.close | |||
| stdin&.close | |||
There was a problem hiding this comment.
I believe the intention was to have stdin.close in the scope of the Open3 command and was accidentally moved to a different scope, outside the Bundler.with_clean_env scope, in commit: 63b44ed
I believe checking for stdin here is not right. We should either not close stdin at all or move the stdin.close in the with_clean_env block.
cc @carbonin
There was a problem hiding this comment.
move the stdin.close in the with_clean_env block.
☝️ This I think.
yrudman
force-pushed
the
check-for-nilif-stdin-not-nil-in-cockpit-worker
branch
from
a94aa2c
to
e00b25d
Compare
yrudman
changed the title
|
it looks like |
jrafanie
reviewed
Apr 20, 2020
yrudman
force-pushed
the
check-for-nilif-stdin-not-nil-in-cockpit-worker
branch
3 times, most recently
from
593cdea
to
5295103
Compare
fixed possible nil.close, close stderr and stdout when process terminated added missing default value in MiqCockpitWsWorker::Runner#wait_on_cockpit_ws(pid) method, added logging what command used to start cockpit process and enviroment settings fixed typo when accessig command to start cockpit return pid or raise error if cockpit process not started Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1824355
yrudman
force-pushed
the
check-for-nilif-stdin-not-nil-in-cockpit-worker
branch
from
5295103
to
6e67f1f
Compare
yrudman
changed the title
jrafanie
reviewed
Apr 20, 2020
|
Checked commits yrudman/manageiq@7bb0735~...ddec1e3 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
simaishi
pushed a commit
that referenced
this pull request
Apr 23, 2020
…in-cockpit-worker Fixed cockpit process starting (cherry picked from commit 87de914) https://bugzilla.redhat.com/show_bug.cgi?id=1824355
|
Jansa backport details: |
simaishi
pushed a commit
that referenced
this pull request
May 7, 2020
…in-cockpit-worker Fixed cockpit process starting (cherry picked from commit 87de914) https://bugzilla.redhat.com/show_bug.cgi?id=1824355
|
Ivanchuk backport details: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue: Cockpit process not starting
There were multiple issues with each preventing successful starting for cockpit process.
After:
Cockpit process successfully started on reproducer environment:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1824355
@miq-bot add-label bug, core, ivanchuk/yes