Skip to content

[NO TESTS NEEDED] Shrink the size of podman bindings#9516

Merged
openshift-merge-robot merged 1 commit intocontainers:masterfrom
rhatdan:shrink
Mar 29, 2021
Merged

[NO TESTS NEEDED] Shrink the size of podman bindings#9516
openshift-merge-robot merged 1 commit intocontainers:masterfrom
rhatdan:shrink

Conversation

@rhatdan
Copy link
Member

@rhatdan rhatdan commented Feb 25, 2021

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@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 Feb 25, 2021
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 Feb 25, 2021
@rhatdan rhatdan force-pushed the shrink branch 2 times, most recently from caf504b to ccb2018 Compare February 25, 2021 18:25
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 25, 2021
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 25, 2021
@rhatdan rhatdan force-pushed the shrink branch 4 times, most recently from 3800cc2 to 159357c Compare February 25, 2021 21:48
@rhatdan rhatdan force-pushed the shrink branch 3 times, most recently from 4be3f79 to e215284 Compare March 4, 2021 19:14
@rhatdan rhatdan changed the title [WIP] [NO TESTS NEEDED] Shrink the size of podman bindings [NO TESTS NEEDED] Shrink the size of podman bindings Mar 4, 2021
@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 Mar 4, 2021
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2021
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2021
@TomSweeneyRedHat
Copy link
Member

@rhatdan needs a rebase

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2021
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2021
@rhatdan rhatdan force-pushed the shrink branch 3 times, most recently from 5c2c434 to f1be4e4 Compare March 11, 2021 11:19
@rhatdan
Copy link
Member Author

rhatdan commented Mar 12, 2021

@cevich Still getting the same errors, I don't believe the VM was updated.

@cevich
Copy link
Member

cevich commented Mar 15, 2021

FWIW this error is all over the F33 logs, in nearly/every test:

time="2021-03-13T11:19:02Z" level=warning msg="Error updating pod 2b8...3f4 conmon cgroup PID limit: open /sys/fs/cgroup/libpod_parent/2b8...3f4/conmon/pids.max: no such file or directory"```

@rhatdan
Copy link
Member Author

rhatdan commented Mar 16, 2021

@giuseppe PTAL, would this be causing all of the failures?

@giuseppe
Copy link
Member

time="2021-03-13T11:19:02Z" level=warning msg="Error updating pod 2b8...3f4 conmon cgroup PID limit: open /sys/fs/cgroup/libpod_parent/2b8...3f4/conmon/pids.max: no such file or directory"```

I don't think so, it is just a warning when we cannot set the cgroup limits for conmon

@rhatdan rhatdan force-pushed the shrink branch 4 times, most recently from 2c137ba to 35c934c Compare March 20, 2021 12:30
@rhatdan rhatdan force-pushed the shrink branch 5 times, most recently from a8592c5 to 566a0cf Compare March 27, 2021 02:04
@rhatdan
Copy link
Member Author

rhatdan commented Mar 27, 2021

@containers/podman-maintainers This one is ready to merge PTAL

Copy link
Member

Choose a reason for hiding this comment

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

Is github.com/containers/podman/v3/libpod/define going away eventually? If not, rather than buildah here and just define elsewhere, I think this should be bdefine or something that's easily identified as being different than Podman's define.

Copy link
Member

Choose a reason for hiding this comment

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

I concur with Tom. I would prefer to alias all imports of this package as buildahDefine. My eyes are used to read define as the Podman package.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am waiting for my other pull-never gets merged, before fixing this, in case of conflicts.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan
Copy link
Member Author

rhatdan commented Mar 29, 2021

Ok they are all buildahDefine now.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@baude
Copy link
Member

baude commented Mar 29, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2021
@openshift-merge-robot openshift-merge-robot merged commit 26b0ebd into containers:master Mar 29, 2021
@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 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 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.

8 participants