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

Report correct RemoteURI #12895

Closed
wants to merge 1 commit into from
Closed

Report correct RemoteURI #12895

wants to merge 1 commit into from

Conversation

jwhonce
Copy link
Member

@jwhonce jwhonce commented Jan 17, 2022

Rather than assuming a filesystem path, the API service URI is recorded
in the libpod runtime configuration and then reported as requested.

Cleanup some godoc comments.

Note: All schemes other than "unix" are hard-coded to report URI exists.

Fixes #12023

Signed-off-by: Jhon Honce [email protected]

@jwhonce jwhonce added kind/bug Categorizes issue or PR as related to a bug. HTTP API Bug is in RESTful API labels Jan 17, 2022
@jwhonce jwhonce requested a review from edsantiago January 17, 2022 19:22
@jwhonce jwhonce self-assigned this Jan 17, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 17, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwhonce

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 bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 17, 2022
@edsantiago
Copy link
Member

edsantiago commented Jan 17, 2022 •
edited
Loading

Nice! Would you mind un-commenting this out?

podman/test/system/272-system-connection.bats

Lines 102 to 105 in 8514ebd

# FIXME: #12023, RemoteSocket is always /run/something
# run_podman info --format '{{.Host.RemoteSocket.Path}}'
# is "$output" "tcp:localhost:$_SERVICE_PORT" \
# "podman info works, and talks to the correct server"

@jwhonce jwhonce force-pushed the issues/12023 branch 4 times, most recently from 06bd1f9 to bd621a1 Compare January 18, 2022 16:09
@TomSweeneyRedHat
Copy link
Member

Change LGTM
but the tests are not at all happy.

@jwhonce jwhonce marked this pull request as draft January 20, 2022 18:05
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 20, 2022
@edsantiago
Copy link
Member

@jwhonce root tests failing with a FIXME, rootless failing with some other error, all of them failing within mere minutes, and all of them pretty much skipping all tests

@jwhonce jwhonce force-pushed the issues/12023 branch 4 times, most recently from 658966d to 873c7cc Compare January 25, 2022 16:07
@jwhonce jwhonce marked this pull request as ready for review January 25, 2022 16:13
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 25, 2022
@umohnani8
Copy link
Member

Changes LGTM

@rhatdan
Copy link
Member

rhatdan commented Jan 27, 2022

@jwhonce Is this something we need to get into podman 4.0, or can it wait for podman 4.1?

@jwhonce
Copy link
Member Author

jwhonce commented Jan 31, 2022

@rhatdan Podman 4.1, python-py higher priority

@rhatdan rhatdan added the 4.1 label Jan 31, 2022
@github-actions
Copy link

github-actions bot commented Mar 3, 2022

A friendly reminder that this PR had no activity for 30 days.

@rhatdan rhatdan removed the stale-pr label Mar 3, 2022
@jwhonce
Report correct RemoteURI
336354a 
Rather than assuming a filesystem path, the API service URI is recorded
in the libpod runtime configuration and then reported as requested.

Note: All schemes other than "unix" are hard-coded to report URI exists.

Fixes containers#12023

Signed-off-by: Jhon Honce <[email protected]>
@edsantiago
Copy link
Member

I restarted a bunch of look-like-flakes, the quay.io outage we've been seeing the past week.

This one, though, looks real:

        # podman [options] play kube --down /tmp/podman_test708766852/kube.yaml
         time="2022-03-24T16:23:45Z" level=error msg="Deleting
           pod b9b143d677ef50f6e9dc1628ddaabeb2a208681ac483adab1999c0aeeddfc4af cgroup
          /libpod_parent/b9b143d677ef50f6e9dc1628ddaabeb2a208681ac483adab1999c0aeeddfc4af: 
          remove /sys/fs/cgroup/libpod_parent/b9b143d677ef50f6e9dc1628ddaabeb2a208681ac483adab1999c0aeeddfc4af/conmon:
          remove /sys/fs/cgroup/libpod_parent/b9b143d677ef50f6e9dc1628ddaabeb2a208681ac483adab1999c0aeeddfc4af/conmon: device or resource busy"

edsantiago
edsantiago reviewed Mar 24, 2022
View reviewed changes
test/e2e/play_kube_test.go
@@ -3149,6 +3149,9 @@ invalid kube kind
Expect(ls.OutputToStringArray()).To(HaveLen(1))

containerLen := podmanTest.Podman([]string{"pod", "inspect", pod.Name, "--format", "'{{len .Containers}}'"})
containerLen.WaitWithDefaultTimeout()
Expect(ls).Should(Exit(0))
Expect(ls.OutputToStringArray()).Should(BeNumerically(">", 1))
Copy link
Member

@edsantiago edsantiago Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Missing len() no wait this looks like copy-paste; it needs more significant cleanup (s/ls/containerLen/, OutputToString() instead of Array).

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 22, 2022

@jwhonce: PR needs rebase.

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/test-infra repository.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2022
@rhatdan rhatdan mentioned this pull request Apr 27, 2022
Merged
@rhatdan rhatdan closed this Apr 27, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@edsantiago edsantiago edsantiago left review comments

Assignees

@jwhonce jwhonce

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. HTTP API Bug is in RESTful API kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

podman system service: what should .Host.RemoteSocket.Path be?
5 participants
@jwhonce @edsantiago @TomSweeneyRedHat @umohnani8 @rhatdan