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 CI images to 20230824 #19699

Closed
wants to merge 2 commits into from
Closed

update CI images to 20230824 #19699

wants to merge 2 commits into from

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Aug 22, 2023

includes the en_US.UTF-8 locale

Does this PR introduce a user-facing change?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

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 22, 2023
@Luap99
Copy link
Member Author

Luap99 commented Aug 22, 2023

weird

/usr/share/automation/lib/utils.sh: line 107: nc: command not found

Did we remove nc from the images? cc @cevich

@cevich
Copy link
Member

cevich commented Aug 22, 2023

Not intentionally, and maybe an interesting note is this didn't fail in the slightly-older 20230816t191118z-f38f37d13. For Debian there is some nc/ncat Debian-specific magic @edsantiago helped me put in. Maybe the image build logs have some clues...

@cevich
Copy link
Member

cevich commented Aug 22, 2023

...oof, it gets more strange. The build log shows the magic happening:

 debian: update-alternatives: using /usr/bin/ncat to provide /bin/nc (nc) in auto mode

Maybe we need a nc symlink also? Maybe there's a bug in SID that caused this?

@Luap99
Copy link
Member Author

Luap99 commented Aug 22, 2023

Well it fails on both debian and fedora so this is really odd that both are breaking at the same time.

@cevich
Copy link
Member

cevich commented Aug 22, 2023

Oh good point, I didn't notice the Fedora failures. Hrmmmm, that's concerning. Let's see what get_ci_vm.sh says...

@edsantiago
Copy link
Collaborator

Nothing is installed:

[root@cirrus-task-4949830783467520 .cirrus]# rpm -q pandoc
package pandoc is not installed
[root@cirrus-task-4949830783467520 .cirrus]# rpm -q libtool
package libtool is not installed

("pandoc" and "libtool" are taken from the list in cache_images/*.sh)

Of course by "nothing" I don't mean it literally, only that things-that-I-expect-to-be-installed are not-installed. nc seems to be only the first one detected.

@Luap99
Copy link
Member Author

Luap99 commented Aug 22, 2023

Ok I assume the images are just broken then??? @cevich can you respin a new build.

.cirrus.yml Outdated Show resolved Hide resolved
@edsantiago
Copy link
Collaborator

Pasta is broken:

  • OLD: passt-0^20230625.g32660ce-1.fc38-x86_64 - works
  • NEW: passt-0^20230818.g0af928e-1.fc38-x86_64 - fails

@sbrivio-rh it looks like passt 0818 is broken, could you PTAL at the failing tests, and fix either pasta or the tests?

@sbrivio-rh
Copy link
Collaborator

Pasta is broken:

* OLD:  passt-0^2023**0625**.g32660ce-1.fc38-x86_64  - works
* NEW: passt-0^2023**0818**.g0af928e-1.fc38-x86_64 - fails

@sbrivio-rh it looks like passt 0818 is broken, could you PTAL at the failing tests, and fix either pasta or the tests?

Oops, yes, sorry, I guess pasta is broken, we get EINVAL while copying IPv4 routes. I'm having a look now, and tagging @dgibson in case I fall asl

@dgibson
Copy link
Collaborator

dgibson commented Aug 23, 2023

So, the change to report (most) errors on netlink operations made the failure here more evident, but it already existed. Previously rather than outright failing, we would simply have omitted copying certain routes from the container. Depending what those routes are the container might still operate more or less ok, or it might completely.

The reason certain routes are seeing EINVAL - at least in the cases I've reproduced is that those routes have an RTA_SRC, limiting the route to a specific source address on the host. When we use the -a option, as the failing examples do, the guest no longer has that address, and so it's invalid to add a route with that address as a source.

@dgibson
Copy link
Collaborator

dgibson commented Aug 23, 2023

Correction: the issue described in the previous comment is only causing one of the podman failures. There's another bug causing some others, which @sbrivio-rh is looking into.

@cevich
Copy link
Member

cevich commented Aug 23, 2023

So for this PR (which is not networking related?), it sounds like the "file-issue + skip tests" strategy is most appropriate. I'm going to merge containers/automation_images#297 as-is for now. But if some quick change is needed in the CI VM images, I'm happy to assist with that.

@sbrivio-rh
Copy link
Collaborator

So for this PR (which is not networking related?), it sounds like the "file-issue + skip tests" strategy is most appropriate. I'm going to merge containers/automation_images#297 as-is for now. But if some quick change is needed in the CI VM images, I'm happy to assist with that.

As you wish -- I'm preparing new packages for Fedora and Debian at the moment, but it might take a while and if you have a way out...

By the way, fixes are:

https://passt.top/passt/commit/?id=5e4f7b92b0b0bf4724c505fa95fcae1526a8f88b
https://passt.top/passt/commit/?id=a7e4bfb857cb5d0e111ab74b6ace47eea15d2078

@Luap99
Copy link
Member Author

Luap99 commented Aug 23, 2023

There is nothing urgent in this PR. This is a simple cleanup of my previous work, we can wait until packages land in stable then rebuild images.
Although I will be on PTO the next two weeks so would be much appreciated if someone else could push the image updates if that happens in the meantime.

@cevich
Copy link
Member

cevich commented Aug 23, 2023

There is nothing urgent in this PR. This is a simple cleanup of my previous work, we can wait until packages land in stable then rebuild images.

Okay SGTM. @sbrivio-rh do you have a way to monitor those packages? Once they land in SID and Fedora updates-testing, I'm happy to roll new CI VM images and get them updated for Paul.

@sbrivio-rh
Copy link
Collaborator

There is nothing urgent in this PR. This is a simple cleanup of my previous work, we can wait until packages land in stable then rebuild images.

Okay SGTM. @sbrivio-rh do you have a way to monitor those packages? Once they land in SID and Fedora updates-testing [...]

I'm the maintainer for both -- I'll post here when they do.

@cevich
Copy link
Member

cevich commented Aug 23, 2023

Fantastic, that will help a lot, thanks.

@sbrivio-rh
Copy link
Collaborator

Fedora 38 update should reach testing in a bit, Debian Unstable package available for upload. I'll tell you anyway once they reach the repositories.

@TomSweeneyRedHat
Copy link
Member

changes LGTM, but the tests still need to be muscled over the line.

@sbrivio-rh
Copy link
Collaborator

Finally, passt-0.0~git20230823.a7e4bfb-1 reached Debian unstable, and passt-0^20230823.ga7e4bfb-1.fc38 is in Fedora's updates-testing.

@cevich
Copy link
Member

cevich commented Aug 24, 2023

Great thanks, I'll start building fresh CI VM images.

AkihiroSuda pushed a commit to AkihiroSuda/passt-mirror that referenced this pull request Aug 25, 2023
…sses

Otherwise, we actually configure the address, but it's not usable
because no local route is added by the kernel.

Link: containers/podman#19699
Fixes: cfe7509 ("netlink: Use struct in_addr for IPv4 addresses, not bare uint32_t")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
AkihiroSuda pushed a commit to AkihiroSuda/passt-mirror that referenced this pull request Aug 25, 2023
Host routes can include a preferred source address (RTA_PREFSRC), which
must be one of the host's addresses.  However when using pasta with -a the
namespace might be given a different address, not on the host.  This seems
to occur pretty routinely depending on the network configuration systems
in place on the host.

With --config-net we will try to copy host routes to the namespace.  If
one of those includes an RTA_PREFSRC, but the namespace doesn't have the
host address, this will fail with -EINVAL, causing pasta to fail.

Fix this by stripping off RTA_PREFSRC attributes from routes as we copy
them to the namespace.  This is by no means infallible, bit it should at
least handle common cases for the time being.

Link: https://bugs.passt.top/show_bug.cgi?id=71
Link: containers/podman#19699 (comment)
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
These images include the en_US.UTF-8 locale so we can remove the
workaroud here in podman.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This reverts commit ed1f514.

The en_US.UTF-8 locale is now added in the images at build time,
containers/automation_images#295

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99 Luap99 changed the title update CI images to 20230821 update CI images to 20230824 Aug 25, 2023
@Luap99
Copy link
Member Author

Luap99 commented Aug 25, 2023

@cevich I used the image ID from containers/automation_images#298

@sbrivio-rh looks like fedora is working again but there is still a pasta issue on debian:

[+1329s] # [13:49:24.955653397] Error: pasta failed with exit code 1:
[+1329s] # Can't run AVX2 build, using non-AVX2 version: No such file or directory
[+1329s] # No external routable interface for IPv6
[+1329s] # Couldn't get any nameserver address
[+1329s] # Template interface: ens4 (IPv4)
[+1329s] # Namespace interface: myname
[+1329s] # MAC:
[+1329s] #     host: 42:01:0a:80:00:ed
[+1329s] # DHCP:
[+1329s] #     assign: 10.128.0.237
[+1329s] #     mask: 255.255.255.255
[+1329s] #     router: 10.128.0.1
[+1329s] # mount /: Permission denied
[+1329s] # Failed to sandbox process, exiting

@cevich
Copy link
Member

cevich commented Aug 25, 2023

Paul! You're suppose to be on PTO!

Moving this issue to #19751

@cevich cevich closed this Aug 25, 2023
@Luap99
Copy link
Member Author

Luap99 commented Aug 25, 2023

I am on PTO the next two weeks, so I still have an hour until PTO/weekend starts for me.

@cevich
Copy link
Member

cevich commented Aug 25, 2023

No worries, I was just teasing. Enjoy your PTO 😄

AkihiroSuda pushed a commit to AkihiroSuda/passt-mirror that referenced this pull request Nov 2, 2023
Ugly as hell, but we keep breaking things otherwise, and I keep
forgetting to run this manually (as long as it's based on my local
Podman setup, that's the only alternative).

We need to clone the Podman repository as distribution packages don't
contain test scripts, typically. While at it, build the latest
version which is what really matters.

As we're planning anyway to revamp the test framework, I'd be
inclined to just add this without too many thoughts, and have it as
a nice-to-have requirement reminder for the new framework.

Link: containers/podman#19699
Suggested-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
@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 Nov 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 24, 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. 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