Fix transition test to work with latest selinux policy #4829
Conversation
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
This PR needs an updated container-selinux 2.216.0 PR. |
once that merges, we should use the |
|
Fixes: #4772 |
lsm5
changed the title
|
@cevich is containers/podman#18439 (comment) still valid here for [CI:NEXT] or have things changed? |
also, any reason the PR needs to be turned into draft? |
Nope, that magic string isn't setup in buildah CI. May I assume from the question, that it's needed here?
Yes, this is really important. In podman CI, the |
Yes, we just merged a change in container-selinux, a new version has been cut but updates-testing / stable updates will take a while, so we need to fetch from podman-next.
Got it, thanks |
Okay, I'll start work on porting that here from podman CI. |
|
Opened #4830 |
|
LGTM once the underlying dependencies are tended to. |
rhatdan
changed the title
|
@cevich I am trying to move buildah to testing f37 and f38. But it is blowing up? |
Can I assume this needs the new/recently build container-selinux packages? If so, those are in Otherwise, WRT the Smoke Test failure: Those can be a major PITA to debug 😠 In the distant past, I've seen that failure-mode caused by broken golangci-lint plugins and/or not enough memory on the system. I don't think the later is the problem here since gce_instance:
memory: "12Gb"But maybe that's whats happening? Though it could also easily be another flavor of the golang 1.20 based panic I ran into in #4772. In that PR it was more obvious, with a giant PANIC message and traceback. Hmmm...it's not a timeout problem. Gosh...I'm not really sure what else to suggest 😞 |
|
@cevich could you update to f37 and f38 in a separate PR, to what we should be using. Then I can rebase this PR on the latest. |
I can...but it will immediately fail CI due to golang-ci shenanigans 😢 |
|
Ref: Opened #4863 to see if not including the CI VM image updates makes CI testing any smoother. |
|
FYI: Opened containers/automation_images#282 |
rhatdan
changed the title
rhatdan
changed the title
rhatdan
force-pushed
the
selinux
branch
3 times, most recently
from
fa01395 to
c53074a
Compare
Makefile
Outdated
| @@ -179,7 +179,8 @@ tests/testreport/testreport: tests/testreport/testreport.go | |||
|
|
|||
| .PHONY: test-unit | |||
| test-unit: tests/testreport/testreport | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" -cover $(RACEFLAGS) $(shell $(GO) list ./... | grep -v vendor | grep -v tests | grep -v cmd) -timeout 45m | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" -cover $(RACEFLAGS) $(shell $(GO) list ./... | grep -v vendor | grep -v tests | grep -v cmd | grep -v chroot | grep -v copier) -timeout 45m | |||
| $(GO_TEST) -v -tags "$(STORAGETAGS) $(SECURITYTAGS)" $(RACEFLAGS) ./chroot ./copier | |||
There was a problem hiding this comment.
Looks like we need to add a -timeout with a value larger than the default 10m here after all.
|
Note: In case it helps (i.e. w/ newer SELinux/related packages), I've started building yet-another image update. They're required for podman CI, but I'm happy to coordinate tagging those images until this PR merges - meaning, they'd only be used in podman CI for the near-term. |
Signed-off-by: Daniel J Walsh <[email protected]>
The chrooting causes testing with coverage counting enabled to output a warning message which interferes with how they communicate with child processes. Disable -cover for those modules by testing them separately without it. Signed-off-by: Nalin Dahyabhai <[email protected]> Signed-off-by: Daniel J Walsh <[email protected]>
|
@rhatdan I'll be building a v1.31 this evening. This looks to be the last PR we have pending. If it's still not merged then, we can backport it to the 1.31.0 branch and get it into Podman RC2 next week. @ashley-cui FYI |
|
I don't think we should release buildah until it passes tests on f38. Unless you want to release buildah as a RC1. |
I hate to rush anybody. So I'll just point out it would be "nice" to have the F38 CI VM update (included in this PR) pre-baked into the release branch. It's not completely out of the question to backport the setup, esp. while the new branch is fresh. But it will fairly quickly become a less and less desirable thing to do to a RHEL release branch. My motivation here is trying to keep the Fedora->RHEL gap as small as possible, and F37 is quite far away from F38 WRT what's going into RHEL - at least that's my wishful thinking. I could be wrong. |
|
@cevich Once this completes and merges we can bump the VMs again. |
|
/lgtm |
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?
[CI:NEXT]