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

Fix ssh command usage #14703

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Fix ssh command usage #14703

merged 1 commit into from
Apr 11, 2024

Conversation

tpapaioa
Copy link
Contributor

@tpapaioa tpapaioa commented Apr 9, 2024 •
edited
Loading

Problem Statement

The Base.execute() method, which is used to run hammer CLI commands via broker's ssh client, currently encodes the hammer command as a bytes object before sending it to broker. Broker and its current ssh library (ssh2-python312) expect a str instead, although the library silently accepts the bytes object and runs the command successfully.

With support for other ssh libraries being added to broker in SatelliteQE/broker#282, passing in a bytes object won't work anymore:

09:29:43  robottelo/cli/settings.py:29: in set
09:29:43      return cls.execute(cls._construct_command(options))
09:29:43  robottelo/cli/base.py:202: in execute
09:29:43      response = ssh.command(
09:29:43  robottelo/ssh.py:51: in command
09:29:43      result = client.execute(cmd, timeout=timeout)
09:29:43  ../../lib64/python3.12/site-packages/broker/hosts.py:165: in execute
09:29:43      res = self.session.run(command, timeout=timeout)
09:29:43  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
09:29:43  
09:29:43  self = <broker.ssh_session_hussh.Session object at 0x7f87a3b33950>
09:29:43  command = b'LANG=en_US.UTF-8  hammer -v -u admin -p ****  settings set --name="failed_login_attempts_limit" --value="0" '
09:29:43  timeout = '10m'
09:29:43  
09:29:43      def run(self, command, timeout=0):
09:29:43          """Run a command on the host and return the results."""
09:29:43          # TODO support timeout parameter
09:29:43  >       return self.session.execute(command)
09:29:43  E       TypeError: argument 'command': 'bytes' object cannot be converted to 'PyString'
09:29:43  
09:29:43  ../../lib64/python3.12/site-packages/broker/ssh_session_hussh.py:97: TypeError

Solution

This PR updates Host.execute() to pass a string-valued command instead.

PRT test runs:

Related Issues

@tpapaioa tpapaioa added the No-CherryPick PR doesnt need CherryPick to previous branches label Apr 9, 2024
@tpapaioa
Copy link
Contributor Author

tpapaioa commented Apr 9, 2024

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6400
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : =========== 1 passed, 3 deselected, 5 warnings in 1029.32s (0:17:09) ===========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Apr 9, 2024
@tpapaioa
Copy link
Contributor Author

tpapaioa commented Apr 10, 2024 •
edited
Loading 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: hussh

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6420
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : =========== 3 deselected, 11 warnings, 1 error in 644.86s (0:10:44) ============

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6421
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : =========== 3 deselected, 11 warnings, 1 error in 665.69s (0:11:05) ============

@Satellite-QE Satellite-QE removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: hussh

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6423
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : =========== 1 failed, 3 deselected, 16 warnings in 953.43s (0:15:53) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
@tpapaioa tpapaioa removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 10, 2024
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: hussh

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6424
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : ========== 1 passed, 3 deselected, 17 warnings in 1068.73s (0:17:48) ===========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Apr 10, 2024
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: ansible-pylibssh

@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: ssh2-python312

1 similar comment
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282
env:
    BROKER_SSH_BACKEND: ssh2-python312

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6429
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : ========== 1 passed, 3 deselected, 17 warnings in 1082.65s (0:18:02) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 10, 2024
@tpapaioa tpapaioa removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 10, 2024
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6431
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : ========== 1 passed, 3 deselected, 17 warnings in 1073.87s (0:17:53) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 10, 2024
@tpapaioa tpapaioa changed the title Test commit for PRT Fix ssh command usage Apr 10, 2024
@tpapaioa tpapaioa removed the PRT-Failed Indicates that latest PRT run is failed for the PR label Apr 11, 2024
@tpapaioa tpapaioa marked this pull request as ready for review April 11, 2024 18:36
@tpapaioa tpapaioa requested review from a team as code owners April 11, 2024 18:36
@tpapaioa
Copy link
Contributor Author 

trigger: test-robottelo
pytest: tests/foreman/endtoend/test_cli_endtoend.py -k 'test_positive_cli_end_to_end'
broker: 282

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 6471
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/endtoend/test_cli_endtoend.py -k test_positive_cli_end_to_end --external-logging
Test Result : ========== 1 passed, 3 deselected, 17 warnings in 1204.55s (0:20:04) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Apr 11, 2024
@JacobCallahan JacobCallahan requested a review from a team April 11, 2024 20:23
@ogajduse ogajduse merged commit e3b8137 into SatelliteQE:master Apr 11, 2024
21 checks passed
@tpapaioa tpapaioa deleted the prt_branch branch April 12, 2024 11:24
@JacobCallahan JacobCallahan added CherryPick PR needs CherryPick to previous branches 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 and removed No-CherryPick PR doesnt need CherryPick to previous branches labels Jun 10, 2024
This was referenced Jun 10, 2024
Merged
Merged
Merged
JacobCallahan pushed a commit to JacobCallahan/robottelo that referenced this pull request Jun 10, 2024
@tpapaioa @JacobCallahan
Fix ssh command usage (SatelliteQE#14703)
befbe70
@JacobCallahan JacobCallahan mentioned this pull request Jun 10, 2024
Merged
JacobCallahan pushed a commit to JacobCallahan/robottelo that referenced this pull request Jun 10, 2024
@tpapaioa @JacobCallahan
Fix ssh command usage (SatelliteQE#14703)
ccb575e
@JacobCallahan JacobCallahan mentioned this pull request Jun 10, 2024
Merged
JacobCallahan pushed a commit to JacobCallahan/robottelo that referenced this pull request Jun 10, 2024
@tpapaioa @JacobCallahan
Fix ssh command usage (SatelliteQE#14703)
bc9428c
@JacobCallahan JacobCallahan mentioned this pull request Jun 10, 2024
Merged
Gauravtalreja1 pushed a commit that referenced this pull request Jun 11, 2024
@JacobCallahan @tpapaioa
[6.15.z] Cherry-pick: Fix ssh command usage (#14703) (#15387)
bc83c26 
Fix ssh command usage (#14703)

Co-authored-by: Tasos Papaioannou <tpapaioa@redhat.com>
Gauravtalreja1 pushed a commit that referenced this pull request Jun 11, 2024
@JacobCallahan @tpapaioa
[6.14.z] Cherry-pick: Fix ssh command usage (#14703) (#15388)
aa9891d 
Fix ssh command usage (#14703)

Co-authored-by: Tasos Papaioannou <tpapaioa@redhat.com>
Gauravtalreja1 pushed a commit that referenced this pull request Jun 11, 2024
@JacobCallahan @tpapaioa
[6.13.z] Cherry-pick: Fix ssh command usage (#14703) (#15389)
3421009 
Fix ssh command usage (#14703)

Co-authored-by: Tasos Papaioannou <tpapaioa@redhat.com>
shweta83 pushed a commit to shweta83/robottelo that referenced this pull request Jun 11, 2024
@JacobCallahan @tpapaioa @shweta83
[6.15.z] Cherry-pick: Fix ssh command usage (SatelliteQE#14703) (Sate…
908ec04 
…lliteQE#15387)

Fix ssh command usage (SatelliteQE#14703)

Co-authored-by: Tasos Papaioannou <tpapaioa@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JacobCallahan JacobCallahan JacobCallahan approved these changes

@ogajduse ogajduse ogajduse approved these changes

Assignees

@tpapaioa tpapaioa

Labels
6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tpapaioa @Satellite-QE @JacobCallahan @ogajduse