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

RHOAIENG-18459: chore(tests/containers): handle multiple podman machines and improve debugging output when podman machine is not found #869

Merged
merged 2 commits into from
Jan 28, 2025
Merged

RHOAIENG-18459: chore(tests/containers): handle multiple podman machines and improve debugging output when podman machine is not found #869

merged 2 commits into from
Jan 28, 2025

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Jan 27, 2025 •
edited
Loading

Followup on

Description

Thanks @andyatmiami for help figuring this out. podman machine inspect replies with a json list, but it's always a json list of one element! podman machine list does give all machines, but it does not have access to their socket paths.

# From screwing around with a Go template string.. i'm not sure we can use the ls command of machine list - i'm beginning to think the information is moreso high level there.. and PodmanSocket is just literally not exposed :confused:

$ podman machine ls --format '{{range .}}{{.}}{{end -}}'  <-- doesn't contain what we actually care about

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@openshift-ci openshift-ci bot requested review from caponetto and dibryant January 27, 2025 16:57
@openshift-ci openshift-ci bot added size/s and removed size/s labels Jan 27, 2025
@jiridanek
RHOAIENG-18459: chore(tests/containers): reduce the number of podman …
679cf5f 
…machine inspect calls, by giving it all machine names at once
@openshift-ci openshift-ci bot added size/xs and removed size/s labels Jan 27, 2025
@jiridanek jiridanek changed the title RHOAIENG-18459: chore(tests/containers): improve debugging output when podman machine is not found RHOAIENG-18459: chore(tests/containers): handle multiple podman machines and improve debugging output when podman machine is not found Jan 27, 2025
@openshift-ci openshift-ci bot added size/xs and removed size/xs labels Jan 27, 2025
@jiridanek jiridanek added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 27, 2025
jiridanek
jiridanek commented Jan 27, 2025
View reviewed changes
tests/containers/workbenches/workbench_image_test.py
Copy link
Member Author

Choose a reason for hiding this comment

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

/override ci/prow/images
n/a

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 27, 2025

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images

In response to this:

/override ci/prow/images
n/a

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@andyatmiami
Copy link
Contributor

/lgtm

given my current configuration:

➜ notebooks/ git:(jd_initial_ipv6_w_macos) $ podman machine ls
NAME                    VM TYPE     CREATED      LAST UP            CPUS        MEMORY      DISK SIZE
rhoai-default           applehv     5 weeks ago  Currently running  4           10GiB       200GiB
podman-machine-default  applehv     2 hours ago  2 hours ago        5           2GiB        100GiB
➜ notebooks/ git:(jd_initial_ipv6_w_macos) $ podman system connection list
Name                         URI                                                         Identity                                                        Default     ReadWrite
podman-machine-default       ssh://[email protected]:54845/run/user/501/podman/podman.sock  /Users/astonebe/.local/share/containers/podman/machine/machine  false       true
podman-machine-default-root  ssh://[email protected]:54845/run/podman/podman.sock           /Users/astonebe/.local/share/containers/podman/machine/machine  false       true
rhoai-default                ssh://[email protected]:50943/run/user/501/podman/podman.sock  /Users/astonebe/.local/share/containers/podman/machine/machine  false       true
rhoai-default-root           ssh://[email protected]:50943/run/podman/podman.sock           /Users/astonebe/.local/share/containers/podman/machine/machine  true        true

Attempting to run the command poetry run pytest tests/containers -k 'TestWorkbenchImage and test_ipv6_only' --image=quay.io/opendatahub/workbench-images:codeserver-ubi9-python-3.11-20250124-6a93e34 -vv now passes! 💯

  • Was failing on earlier revision of this logic
...
DEBUG:root:socket_path='/private/var/folders/l9/x2ws27hs76x0mw8j7ljk3dmm0000gn/T/podman/rhoai-default-api.sock'
INFO:root:SSH tunnel opened for rhoai-default: [fd00::2]:57839 -> localhost:8888
2025-01-27 13:19:16 INFO SSH tunnel opened for rhoai-default: [fd00::2]:57839 -> localhost:8888
...
PASSEDDEBUG:urllib3.connectionpool:http://localhost:None "DELETE /v1.41/networks/a29c69d14850c11bd47782d98e90fbf1198eec9ca33de1af8543a3a6b903f4a5 HTTP/1.1" 204 0


========================================================================================================= 1 passed, 4 deselected in 4.07s =========================================================================================================
  • notice additional debug logging for socket_path included

@openshift-ci openshift-ci bot added the lgtm label Jan 27, 2025
jstourac
jstourac reviewed Jan 28, 2025
View reviewed changes
tests/containers/workbenches/workbench_image_test.py
@@ -89,6 +89,7 @@ def test_ipv6_only(self, image: str, test_frame):
host = "localhost"
port = podman_machine_utils.find_free_port()
socket_path = os.path.realpath(docker_utils.get_socket_path(client.client))
logging.debug(f"{socket_path=}")
Copy link
Member

Choose a reason for hiding this comment

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

Just curious - are we keeping this on purpose?

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, otherwise the exception after podman machine was not found (that may be printed later) is meaningless. This writes down the socket it was looking for.

@jstourac
Copy link
Member

/lgtm

@jiridanek
Copy link
Member Author

/approve
thanks @jstourac and @andyatmiami for review and help
/override ci/prow/images

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jan 28, 2025

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images

In response to this:

/approve
thanks @jstourac and @andyatmiami for review and help
/override ci/prow/images

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 22f1d9c into opendatahub-io:main Jan 28, 2025
7 checks passed
@jiridanek jiridanek deleted the jd_initial_ipv6_w_macos branch January 28, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstourac jstourac jstourac left review comments

@caponetto caponetto Awaiting requested review from caponetto

@dibryant dibryant Awaiting requested review from dibryant

Assignees

@jstourac jstourac

@andyatmiami andyatmiami

Labels
approved lgtm size/xs tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jiridanek @andyatmiami @jstourac