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.

Sign up for GitHub

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

Jump to bottom

Improve starting ipmi_sim program #9378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve starting ipmi_sim program #9378

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions testsuite/features/step_definitions/command_steps.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,12 @@
raise ScriptError, 'File injection failed' unless success
end
server.run('chmod +x /etc/ipmi/fake_ipmi_host.sh', verbose: true, check_errors: true)
server.run('ipmi_sim -n < /dev/null > /dev/null &', verbose: true, check_errors: true)
server.run('nohup ipmi_sim -n > /var/log/ipmi_sim.log 2>&1 &', verbose: true, check_errors: true)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe before starting a new one, we can have an assert/pre-condition where we assure there is no other process already running.
If we have it, we might want to skip the start with a warning message, or we might want to just fail the step.

end

When(/^the server stops mocking an IPMI host$/) do
get_target('server').run('pkill ipmi_sim')
get_target('server').run('pkill fake_ipmi_host.sh || :')
get_target('server').run("ps aux | grep [f]ake_ipmi_host.sh | awk '{print $2}' | xargs kill")
Copy link
Member

Choose a reason for hiding this comment

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

Let's add here verbose: true, so we can keep track of having duplicated processes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would stick with the previous version, there are two issues with it:

  1. Certain implementations of ps might lead to a different output, thus it may require extra handling in the future.
  2. || : is kind of a safety mechanism, if grep won't find this process it may lead to errors.
  3. It involves more moving components: ps, grep, awk, and xargs, kill whereas there you just have pkill.

Copy link
Member

Choose a reason for hiding this comment

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

I agree in your points @szachovy but @maximenoel8 said that pkill was not killing successfully, and we need to address it somehow.

Copy link
Contributor Author

@maximenoel8 maximenoel8 Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If you look in uyuni podman server you will see the pkill is not working:

uyuni-master-podman-srv:~ #  ps aux | grep fake_ipmi_host.sh
root     10582  0.0  0.0   4236  3120 ?        S    Oct16   0:03 /bin/bash /etc/ipmi/fake_ipmi_host.sh
root     11787  0.0  0.0   9156  2336 pts/0    S+   09:48   0:00 grep --color=auto fake_ipmi_host.sh
root     13152  0.0  0.0   4236  3104 ?        S    Oct16   0:03 /bin/bash /etc/ipmi/fake_ipmi_host.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

--full is missing:

uyuni-master-podman-srv:~ # ps aux | grep fake_ipmi_host.sh
root     10582  0.0  0.0   4236  3120 ?        S    Oct16   0:03 /bin/bash /etc/ipmi/fake_ipmi_host.sh
root     13152  0.0  0.0   4236  3104 ?        S    Oct16   0:03 /bin/bash /etc/ipmi/fake_ipmi_host.sh
root     13807  0.0  0.0   8780  2328 pts/1    S+   10:03   0:00 grep --color=auto fake_ipmi_host.sh
uyuni-master-podman-srv:~ # pkill --full fake_ipmi_host.sh
uyuni-master-podman-srv:~ # ps aux | grep fake_ipmi_host.sh
root     13877  0.0  0.0   8780   816 pts/1    S+   10:03   0:00 grep --color=auto fake_ipmi_host.sh

Copy link
Member

Choose a reason for hiding this comment

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

Nice, it seems the proper fix for that killing, indeed.

end

When(/^the controller starts mocking a Redfish host$/) do
Expand Down
Loading