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 images #10829

Merged
merged 5 commits into from
Aug 18, 2021
Merged

Update images #10829

merged 5 commits into from
Aug 18, 2021

Conversation

cevich
Copy link
Member

@cevich cevich commented Jun 30, 2021

Revert temporary workaround commit and update all VM images.

Depends on: #11164, #11166

@TomSweeneyRedHat
Copy link
Member

A quick scan and the changes look OK, but the test system is an ugly red....

@cevich
Copy link
Member Author

cevich commented Jul 1, 2021

OK, but the test system is an ugly red

Super-ugly actually...I wonder if the SELinux issue really isn't fixed yet. I think I'll let this PR sit until after the holiday week, then re-build VM images, cross my fingers, and hope.

@cevich cevich changed the title Update images [WIP] Update images Jul 2, 2021
@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 Jul 2, 2021
@cevich
Copy link
Member Author

cevich commented Jul 12, 2021

I'm back now, started fresh build of VM images for CI.

@cevich
Copy link
Member Author

cevich commented Jul 12, 2021

Ref: containers/automation_images#82

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2021

@cevich Is this still valid?

@cevich
Copy link
Member Author

cevich commented Jul 12, 2021

@cevich Is this still valid?

Yep, it would be nice for consistency sake. More importantly it's not good for us to continue w/ giuseppe's workaround (different Ubuntu vs Fedora images) long-term. I'll look at the failures tomorrow and see what's what (we were waiting on an update to container-selinux or some other selinux related packages in Fedora).

@cevich cevich force-pushed the update_images branch 2 times, most recently from 5ecaff7 to 61980c8 Compare July 13, 2021 17:37
@cevich
Copy link
Member Author

cevich commented Jul 14, 2021

Fedora 33:

Error: open /dev/dma_heap: permission denied

Fedora 34 and Ubuntu:

Failed to decode the keys [\"secret\" \"secret.opts\"] from \"/usr/share/containers/containers.conf\".

@rhatdan
Copy link
Member

rhatdan commented Jul 15, 2021

Error: open /dev/dma_heap: permission

Looks like an out of date selinux policy?
Latest is supposed to be: selinux-policy-3.14.6-38.fc33

Failed to decode the keys ["secret" "secret.opts"] from "/usr/share/containers/containers.conf".

Needs an updated containers-common. But this is not supposed to cause a failure.

@rhatdan
Copy link
Member

rhatdan commented Jul 15, 2021

I am updating the containers-common package in f34 with the correct secrets config options.

@rhatdan
Copy link
Member

rhatdan commented Jul 15, 2021

container-selinux-2.163.0-2.fc33 is needed to fix this.

@cevich
Copy link
Member Author

cevich commented Jul 15, 2021

Error: open /dev/dma_heap: permission
Looks like an out of date selinux policy?
Latest is supposed to be: selinux-policy-3.14.6-38.fc33

For F33, I don't have the policy package version at hand, but we're using container-selinux-2.160.2-1.fc33-noarch, so it sounds like that's too old anyway. Let me see if I can side-load in FEDORA-2021-862d1936a6 and FEDORA-2021-01325d81c4 into VMs manually, confirm they address some issues, and then I can also give karma...

@vrothberg
Copy link
Member

Note: #10848 is now skipping "bud with --runtime and --runtime-flag" of the buildah-bud tests. Once the images are updated, we can revert that in test/buildah-bud/apply-podman-deltas.

vrothberg added a commit to vrothberg/libpod that referenced this pull request Jul 16, 2021
The `IgnorePlatform` options has been removed from the
`LookupImageOptions` in libimage to properly support multi-arch images.

Skip one buildah-bud test which requires updated CI images.  This is
currently being done in github.com/containers/pull/10829 but
we need to unblock merging common and buildah into podman.

[NO TESTS NEEDED]

Signed-off-by: Valentin Rothberg <[email protected]>
@cevich
Copy link
Member Author

cevich commented Jul 16, 2021

Once the images are updated, we can revert that in test/buildah-bud/apply-podman-deltas.

Oh gosh, I will be the first one to completely forget to do that. Let me add a commit here to take care of it...

@cevich
Copy link
Member Author

cevich commented Jul 16, 2021

Manual testing results:

  • On int podman fedora-34 root host tests + fully updated VM + FEDORA-2021-01325d81c4 the situation is greatly improved. Only a few tests flaked.
  • On int podman fedora-33 root host + fully updated VM + FEDORA-2021-862d1936a6 I did not see a noticeable improvement. The dma_heap permissions errors seem equally as plentiful 😞

@cevich
Copy link
Member Author

cevich commented Jul 16, 2021

@rhatdan would you mind poking me when the selinux-policy package update is ready for F33? I'm happy to test it / give bodhi karma.

cevich added a commit to cevich/podman that referenced this pull request Aug 17, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2021
@TomSweeneyRedHat
Copy link
Member

Changes LGTM still, but needs a rebase.

cevich added 5 commits August 18, 2021 12:04
This reverts commit 404d5ed.

The replacement (updated) images include a fix for:
containers/common#631

Also minor update to an unrelated FIXME comment.

Signed-off-by: Chris Evich <[email protected]>
This becomes a problem on hosts with upgraded policies.  Ref:
containers#10522

Also, made a small change to compose-test setup to reduce runtime.

Signed-off-by: Chris Evich <[email protected]>
These tests were originally enabled in a situation where CI provided
false-positive results.  Now that has been corrected, these tests all
fail under a CGv1 container environment with the error:

```
Error: unable to load cgroup at
/machine.slice/libpod-e4f...086.scope/libpod_parent/libpod-fbd...425:
cgroup deleted
```

This commit simply disables the tests under this specific environment.

Signed-off-by: Chris Evich <[email protected]>
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 18, 2021
@cevich
Copy link
Member Author

cevich commented Aug 18, 2021

In case it helps re: int podman ubuntu-2010 rootless host

This appears to be a flake, as now the test has passed. Sadly, I don't think it's the last time we'll encounter it w/o further deep investigation 😞

@cevich
Copy link
Member Author

cevich commented Aug 18, 2021

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -208,7 +222,7 @@ case "$TEST_FLAVOR" in
unit) ;;
apiv2) ;& # use next item
compose)
dnf install -y $PACKAGE_DOWNLOAD_DIR/podman-docker*
rpm -ivh $PACKAGE_DOWNLOAD_DIR/podman-docker*
Copy link
Member

Choose a reason for hiding this comment

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

This is the only eyebrow-raiser I found: it could fail if a dependency of podman-docker is missing. I choose not to worry about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is accounted for when the packages are downloaded. This same 'install from cache' mechanism is used in CI for other containers repos, so we'll notice pretty quickly if there's a problem. I think it's fine to not worry about it.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 18, 2021

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Aug 18, 2021
@TomSweeneyRedHat
Copy link
Member

@cevich, is this still a WIP as the title claims?

@cevich cevich changed the title [WIP] Update images Update images Aug 18, 2021
@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 Aug 18, 2021
@cevich
Copy link
Member Author

cevich commented Aug 18, 2021

@cevich, is this still a WIP as the title claims?

Oh thank :godmode: finally, it passed with only 1 test re-run.

@containers/podman-maintainers PTAL - this is ready along with #11163

@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2021
@openshift-merge-robot openshift-merge-robot merged commit 4ec2270 into containers:main Aug 18, 2021
@cevich cevich deleted the update_images branch April 18, 2023 14:43
@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 Aug 31, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 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.

9 participants