-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
v4.4.1-rhel backports #20906
v4.4.1-rhel backports #20906
Conversation
@edsantiago any idea whatsup with |
I have no idea. It reproduces 100% on my laptop. Probably something that needs to be wired into the podman-remote side. @flouthoc this is your code (from over a year ago), any idea why dockerignore 6 fails under podman-remote? If you need to skip it, here's how: diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas
index 4c4ab17af..c8bdbcf38 100755
--- a/test/buildah-bud/apply-podman-deltas
+++ b/test/buildah-bud/apply-podman-deltas
@@ -248,3 +248,4 @@ skip_if_remote "different error messages between podman & podman-remote" \
"bud with .dockerignore #2" \
- "bud with .dockerignore #4"
+ "bud with .dockerignore #4" \
+ "bud with .dockerignore #6"
|
LGTM |
added in description. |
@edsantiago I'll check it and report back on this PR. I'm unable to quickly recall so I'll have to go again through this. |
@lsm5 can you push this one along please? |
Drop support for remote use-cases when `.containerignore` or `.dockerignore` is a symlink pointing to arbitrary location on host. Signed-off-by: Aditya R <[email protected]>
@edsantiago @cevich can you take another look at the failures please? Would this be better off with getting rid of f36 and replacing f37 with f38 ? |
Changing the CI environment on a release branch breaks the reason we have CI on release branches 😉 Though it's not unprecedented to remove/disable some tests if they're no-longer maintainable or broken due to some externality. Looking at failures... |
...what I see:
I see many of these failures haven't been retried. I'll restart the ones that seem like flakes to me. Hopefully that will help. |
test/buildah-bud/apply-podman-deltas
Outdated
@@ -245,7 +245,8 @@ skip_if_remote "Explicit request in buildah PR 4190 to skip this on remote" \ | |||
# BEGIN tests which are skipped due to actual podman or podman-remote bugs. | |||
|
|||
skip_if_remote "different error messages between podman & podman-remote" \ | |||
"bud with .dockerignore #2" | |||
"bud with .dockerignore #2" \ | |||
"bud with .dockerignore #4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bud test seems to be a hard failure. I would just add one more line here, "bud with .dockerignore #6"
@lsm5 in all of your copious spare time, can you take a look at moving this one along again please? |
looks like new failures:
and a machine test fail that I have retriggered just now. |
test/system/helpers.bash
Outdated
@@ -8,7 +8,7 @@ QUADLET=${QUADLET:-quadlet} | |||
PODMAN_TEST_IMAGE_REGISTRY=${PODMAN_TEST_IMAGE_REGISTRY:-"quay.io"} | |||
PODMAN_TEST_IMAGE_USER=${PODMAN_TEST_IMAGE_USER:-"libpod"} | |||
PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"testimage"} | |||
PODMAN_TEST_IMAGE_TAG=${PODMAN_TEST_IMAGE_TAG:-"20221018"} | |||
PODMAN_TEST_IMAGE_TAG=${PODMAN_TEST_IMAGE_TAG:-"20240123"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the cause of the system-test failures. Why are you bumping this? If there is no compelling reason, please just revert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cevich commented at #20906 (comment) . Did I bump in the wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the comment but not what it refers to. Is there any way to get more context? If not or if it's too hard, I suggest simply reverting that change, i.e. going back to 20221018
Signed-off-by: Lokesh Mandvekar <[email protected]>
@edsantiago alright, only 1 job to go with 1 fail and 1 flake. could you PTAL? |
In [containerized[(https://api.cirrus-ci.com/v1/artifact/task/6222347462508544/html/int-podman-fedora-37-root-container.log.html#t--podman-Netavark-network-works-across-user-ns--1): # podman [options] run --log-driver k8s-file --rm --net 913c5fb620 --uidmap 0:1:4096 quay.io/libpod/alpine:latest sh -c echo podman | nc -w 1 nc-server.dns.podman 9480
nc: bad address 'nc-server.dns.podman' I do not see anything in this PR that could possibly cause that failure. I've hit rerun and am crossing fingers. |
@@ -456,6 +456,33 @@ RUN find /test`, ALPINE) | |||
Expect(session.OutputToString()).To(ContainSubstring("/test/dummy")) | |||
}) | |||
|
|||
It("podman remote build must not allow symlink for ignore files", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By a curious coincidence, I've spent the morning working on this very test. As written, this test is completely useless. If for any reason you need to repush, like to disable the failing containerized test, you might want to cherrypick #20582
It seems certain test infrastructure prevents cloning repo which contains symlink outside of the repo itself, generate symlink for such test by the testsuite itself just before running test and remove it when test is completed. Signed-off-by: Aditya R <[email protected]> (cherry picked from commit 607aff5) Signed-off-by: Lokesh Mandvekar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are green! LGTM but I can't review this entire PR
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, lsm5 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 |
@edsantiago @TomSweeneyRedHat @cevich good to go. Thanks all |
ah I see your comment. Given these are backports and tests are green maybe it's safe (enough) to merge |
LGTM |
/lgtm |
661113d
into
containers:v4.4.1-rhel
Does this PR introduce a user-facing change?
Related: https://issues.redhat.com/browse/RHEL-16395