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.

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

update c/{common,image,storage} to latest #18835

Merged
merged 4 commits into from
Jun 12, 2023

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Jun 9, 2023

Also contains cherry-pick of #18369 to make validate happy.
And another fix for the upgrade tests.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 9, 2023
@rhatdan
Copy link
Member

rhatdan commented Jun 9, 2023

Yikes that is some bloating.

@Luap99
Copy link
Member Author

Luap99 commented Jun 9, 2023

Yikes that is some bloating.

Only for rootlessport though. Somehow some massive transitive dependency must have been slipped in there. I can take a look but for now we should focus on getting this in first.

@lsm5 Any ideas about the rpm failures? They are consistent and only on this vendor update PR. The centos builds are complaining about btrfs which seems weird, that should be excluded via build tag?
However I am unable to understand the problem in the fedora builds, I cannot find any meaningful erros in these cryptic rpm logs.

@lsm5
Copy link
Member

lsm5 commented Jun 9, 2023

On fedora and c9s I see:

cd /builddir/build/BUILD/podman-4.6.0-dev/_build/src/github.com/containers/podman/vendor/github.com/containers/storage/pkg/devicemapper
pkg-config --cflags -- devmapper
go build github.com/containers/podman/vendor/github.com/containers/storage/pkg/devicemapper:
# pkg-config --cflags  -- devmapper
Package devmapper was not found in the pkg-config search path.
Perhaps you should add the directory containing `devmapper.pc'
to the PKG_CONFIG_PATH environment variable
Package 'devmapper', required by 'virtual:world', not found
pkg-config: exit status 1

and the btrfs/ioctl.h in c8s. Looks like something went wrong with buildtags ?
If it's occurring only here and nowhere else, is one of the vendored libs breaking buildtag behaviour perhaps?

@Luap99
Copy link
Member Author

Luap99 commented Jun 9, 2023

If it's occurring only here and nowhere else, is one of the vendored libs breaking buildtag behaviour perhaps?

Yes I assume that is the case but I could not find the error in the build logs.

@lsm5
Copy link
Member

lsm5 commented Jun 9, 2023

@Luap99 ack, I'll check if the spec file needs further adjustment.

I think it might be useful to have copr build tasks on each of the libraries: mainly check if any PR breaks a podman (and/or buildah and/or skopeo) rpm build.

@Luap99
Copy link
Member Author

Luap99 commented Jun 9, 2023

Is there any way to see the build tags in the log? I cannot see any build tags set.

@Luap99
Copy link
Member Author

Luap99 commented Jun 9, 2023

Yes build issue and rootlessport bloat is because of containers/storage#1618 (comment). We now need to set the build tags for rootlessport as well but I rather try to get the c/storage change fixed to avoid the 3 MB bloat in rootlessport.

renovate bot and others added 4 commits June 12, 2023 10:31
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
A c/storage PR[1] chnage the behavior to correctly report umount errors.
This is causing problem in the updgrade tests. The problem is that a
cotnainer is mounted inside another container and then unmounted on the
host. Therefore both operations happen in different mount namespaces.
this is expcted but we want to share the mounts between them. This is
the default but c/stroage make the root private by default thus the
mounts were not shared. To fix this use the `skip_mount_home` storage
option so the mount is kept shared.

[1] containers/storage#1607

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
add routes using the --route flag.
the no_default_route option in --opt prevents a default route from
getting added automatically.

Signed-off-by: Jan Hendrik Farr <github@jfarr.cc>
Because of a c/storage change[1] all we get a lot of new dependencies in
rootlessport despite not using them. Add build tags to exclude storage
drivers to make the binary smaller until it get addressed in c/storage.

This saves about 800 MB but the bloat due that change is still causing
us to gain over 2 MB. This is not ideal but we should get vendoring
going and not wait any longer.

[1] containers/storage#1618

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99 Luap99 added the bloat_approved Approve a PR in which binary file size grows by over 50k label Jun 12, 2023
@Luap99
Copy link
Member Author

Luap99 commented Jun 12, 2023

@containers/podman-maintainers PTAL

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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, vrothberg

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

@rhatdan
Copy link
Member

rhatdan commented Jun 12, 2023

/lgtm
Once we get storage fixed the bloat will get removed hopefully.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2023
@openshift-merge-robot openshift-merge-robot merged commit c83589a into containers:main Jun 12, 2023
87 checks passed
@Luap99 Luap99 deleted the update-container-deps branch June 12, 2023 12:36
lsm5 added a commit to lsm5/podman that referenced this pull request Jul 11, 2023
Document `--route` for `podman network create`.
Feature included via PR: containers#18835

Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>
lsm5 added a commit to lsm5/podman that referenced this pull request Jul 11, 2023
Document `--route` for `podman network create`.
Feature included via PR: containers#18835

Signed-off-by: Lokesh Mandvekar <lsm5@fedoraproject.org>
@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 11, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 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. bloat_approved Approve a PR in which binary file size grows by over 50k 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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants