Skip to content

Remove refs to crio/conmon - #3429

Merged
openshift-merge-robot merged 1 commit into
podman-container-tools:masterfrom
cevich:strip_crio_conmon
Jun 27, 2019
Merged

Remove refs to crio/conmon#3429
openshift-merge-robot merged 1 commit into
podman-container-tools:masterfrom
cevich:strip_crio_conmon

Conversation

@cevich

@cevich cevich commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: Chris Evich cevich@redhat.com

@haircommander

Copy link
Copy Markdown
Contributor

while we're here can you add to the second line /usr/local/libexec/podman/conmon ?

@haircommander

Copy link
Copy Markdown
Contributor

regardless of if you do LGTM thanks @cevich

@cevich
cevich force-pushed the strip_crio_conmon branch from 22b4522 to c4bb6d6 Compare June 25, 2019 19:16
@rhatdan

rhatdan commented Jun 25, 2019

Copy link
Copy Markdown
Contributor

Tests are not happy with this PR.
@mheon PTAL

@cevich

cevich commented Jun 25, 2019

Copy link
Copy Markdown
Contributor Author

@rhatdan @mheon it's only my first try, TBH I'm surprised it even compiled 😁

Goal here is to get podman in-line with a new reality of conmon being separated from cri-o when installed via package (the #1936 connection). So until that PR is ready, I think perhaps I need to set $CONMON_BINARY for all distros. so the tests can pass.

while we're here can you add to the second line /usr/local/libexec/podman/conmon ?

I can, and am happy to adjust anything else as well. I'm not very happy about the separate paths we search through in the int. tests, there's gotta be a better way to handle that.

@cevich cevich changed the title Remove refs to crio/conmon WIP: Remove refs to crio/conmon Jun 25, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2019
@cevich
cevich force-pushed the strip_crio_conmon branch 3 times, most recently from e8fa652 to d69f7ca Compare June 25, 2019 20:22
@cevich cevich mentioned this pull request Jun 26, 2019
7 tasks
Conmon has moved out of cri-o and into it's own dedicated repository.
This commit updates configuration and definitions which referenced
the old cri-o based paths.

Signed-off-by: Chris Evich <cevich@redhat.com>
@cevich
cevich force-pushed the strip_crio_conmon branch from d69f7ca to fc32439 Compare June 26, 2019 16:45
@cevich cevich changed the title WIP: Remove refs to crio/conmon Remove refs to crio/conmon Jun 26, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2019
@cevich

cevich commented Jun 26, 2019

Copy link
Copy Markdown
Contributor Author

@mheon @haircommander @rhatdan PTAL, tests passing now. I tried to change as little as possible here, so it's certainly not ideal. However, for the vast majority of cases it should allow CI and human testing to function with multiple versions of conmon available at once. I'm still open if any of the search path order needs to be tweaked, juss lemmie know.

@haircommander

Copy link
Copy Markdown
Contributor

LGTM, I like how podman specific locations are searched first. I can't comment on which should be first /usr/local/.. vs /usr.. and so on though.

@cevich

cevich commented Jun 26, 2019

Copy link
Copy Markdown
Contributor Author

I can't comment on which should be first

In my experience, typically the list should go in order of most-special to least-special, with env. vars and config files being in the former category. So less obvious places like /usr/local/... should come first, and more "standard" places like /usr/bin should be last.

@rhatdan

rhatdan commented Jun 27, 2019

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, rhatdan

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

The pull request process is described here

Details 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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2019
@rhatdan

rhatdan commented Jun 27, 2019

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2019
@openshift-merge-robot
openshift-merge-robot merged commit 7705f99 into podman-container-tools:master Jun 27, 2019
@cevich
cevich deleted the strip_crio_conmon branch June 30, 2021 18:11
@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 22, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants