Skip to content

Conversation

@flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Jul 3, 2025

Closes #6182

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

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?

None

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 4, 2025

@mtrmac PTAL

Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for handling this!

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 4, 2025

@containers/buildah-maintainers PTAL

@lsm5
Copy link
Member

lsm5 commented Jul 4, 2025

I think we'll need to update rpm/buildah.spec as well to include the passwd binary in the buildah-tests subpackage.

@lsm5
Copy link
Member

lsm5 commented Jul 4, 2025

I'm fine with the current PR as-is if it helps to speed up merge.

I can add below patch in a followup. Haven't actually tested it yet and TMT jobs are slowly trudging along given the recent fedora outage and resulting backlog. So, I guess we can't count on the TMT jobs here anyway.

From e6657927b2ce9c8f528914f4e5e61d6f6b2277e4 Mon Sep 17 00:00:00 2001
From: Lokesh Mandvekar <[email protected]>
Date: Fri, 4 Jul 2025 11:33:17 -0400
Subject: [PATCH] RPM/TMT: account for passwd binary moving to tests

Signed-off-by: Lokesh Mandvekar <[email protected]>
---
 rpm/buildah.spec   | 3 +++
 tests/helpers.bash | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/rpm/buildah.spec b/rpm/buildah.spec
index 0a5e22c8e..4b9d4f454 100644
--- a/rpm/buildah.spec
+++ b/rpm/buildah.spec
@@ -138,6 +138,7 @@ export BUILDTAGS+=" libtrust_openssl"
 %gobuild -o bin/tutorial ./tests/tutorial
 %gobuild -o bin/inet ./tests/inet
 %gobuild -o bin/dumpspec ./tests/dumpspec
+%gobuild -o bin/passwd ./tests/passwd
 %{__make} docs

 %install
@@ -150,6 +151,7 @@ cp bin/copy    %{buildroot}/%{_bindir}/%{name}-copy
 cp bin/tutorial %{buildroot}/%{_bindir}/%{name}-tutorial
 cp bin/inet     %{buildroot}/%{_bindir}/%{name}-inet
 cp bin/dumpspec %{buildroot}/%{_bindir}/%{name}-dumpspec
+cp bin/passwd %{buildroot}/%{_bindir}/%{name}-passwd

 rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/*

@@ -175,6 +177,7 @@ rm %{buildroot}%{_datadir}/%{name}/test/system/tools/build/*
 %{_bindir}/%{name}-tutorial
 %{_bindir}/%{name}-inet
 %{_bindir}/%{name}-dumpspec
+%{_bindir}/%{name}-passwd
 %{_datadir}/%{name}/test

 %changelog
diff --git a/tests/helpers.bash b/tests/helpers.bash
index d38e8c65f..0e80d5262 100644
--- a/tests/helpers.bash
+++ b/tests/helpers.bash
@@ -9,6 +9,7 @@ COPY_BINARY=${COPY_BINARY:-$TEST_SOURCES/../bin/copy}
 TUTORIAL_BINARY=${TUTORIAL_BINARY:-$TEST_SOURCES/../bin/tutorial}
 INET_BINARY=${INET_BINARY:-$TEST_SOURCES/../bin/inet}
 DUMPSPEC_BINARY=${DUMPSPEC_BINARY:-$TEST_SOURCES/../bin/dumpspec}
+PASSWD_BINARY=${PASSWD_BINARY:-$TEST_SOURCES/../bin/passwd}
 STORAGE_DRIVER=${STORAGE_DRIVER:-vfs}
 PATH=$(dirname ${BASH_SOURCE})/../bin:${PATH}
 OCI=${BUILDAH_RUNTIME:-$(${BUILDAH_BINARY} info --format '{{.host.OCIRuntime}}' || command -v runc || command -v crun)}
@@ -833,7 +834,7 @@ auth:
 '
   # roughly equivalent to "htpasswd -nbB testuser testpassword", the registry uses
   # the same package this does for verifying passwords against hashes in htpasswd files
-  htpasswd=${testuser}:$(passwd ${testpassword})
+  htpasswd=${testuser}:$(${PASSWD_BINARY} ${testpassword})

   # generate the htpasswd and config.yml files for the registry
   mkdir -p "${TEST_SCRATCH_DIR}"/registry/root "${TEST_SCRATCH_DIR}"/registry/run "${TEST_SCRATCH_DIR}"/registry/certs "${TEST_SCRATCH_DIR}"/registry/config
--
2.50.0

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 6, 2025

@lsm5 Done.

@flouthoc flouthoc requested a review from mtrmac July 6, 2025 15:16
@mtrmac
Copy link
Contributor

mtrmac commented Jul 7, 2025

@flouthoc the tests are failing.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 7, 2025

Integration tests were flakes I have restarted them.

@lsm5
Copy link
Member

lsm5 commented Jul 7, 2025

@flouthoc ohh my bad. Need this diff too:

$ git diff
diff --git a/tests/tmt/system.fmf b/tests/tmt/system.fmf
index 11b49d9cb..eb6b766f0 100644
--- a/tests/tmt/system.fmf
+++ b/tests/tmt/system.fmf
@@ -10,6 +10,7 @@ environment:
     COPY_BINARY: /usr/bin/buildah-copy
     TUTORIAL_BINARY: /usr/bin/buildah-tutorial
     DUMPSPEC_BINARY: /usr/bin/buildah-dumpspec
+    PASSWD_BINARY: /usr/bin/buildah-passwd
     TMPDIR: /var/tmp

 adjust:

@lsm5
Copy link
Member

lsm5 commented Jul 7, 2025

not ok 395 bud with --cpu-shares is failing consistently on TMT jobs and I don't see the cirrus jobs either.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 8, 2025

not ok 395 bud with --cpu-shares is failing consistently on TMT jobs and I don't see the cirrus jobs either.

@lsm5 I don't think this failure is related to patch in this PR. I see following failure in other PR's too #6267

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 8, 2025

@nalind PTAL

@Luap99
Copy link
Member

Luap99 commented Jul 8, 2025

not ok 395 bud with --cpu-shares is failing consistently on TMT jobs and I don't see the cirrus jobs either.

@lsm5 I don't think this failure is related to patch in this PR. I see following failure in other PR's too #6267

Is this testing podman-next, I guess the thing here is crun chnage how the value gets calculated
containers/crun#1767
So I guess the test needs to be updated so that it works with both crun versions
cc @giuseppe

@giuseppe
Copy link
Member

giuseppe commented Jul 8, 2025

Is this testing podman-next, I guess the thing here is crun chnage how the value gets calculated
containers/crun#1767
So I guess the test needs to be updated so that it works with both crun versions
cc @giuseppe

I don't think there should be any assumption in the way cpu-shares are converted, this is an implementation detail and my suggestion is to skip the test on cgroup v2.

For cgroup v2, we'd need to expose the equivalent of --cgroup-conf that we already have for Podman and use --cgroup-conf cpu.weight=X instead

@nalind
Copy link
Member

nalind commented Jul 8, 2025

Opened #6271, though it could instead have switched over to using "dumpspec" as a runtime and limited itself to checking that we set the right field in the runtime spec.

@TomSweeneyRedHat
Copy link
Member

LGTM
once the tests get hip

flouthoc and others added 2 commits July 9, 2025 06:31
Signed-off-by: Lokesh Mandvekar <[email protected]>
Co-authored-by: flouthoc <[email protected]>
Signed-off-by: flouthoc <[email protected]>
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

2 similar comments
@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@packit-as-a-service
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@flouthoc
Copy link
Collaborator Author

flouthoc commented Jul 9, 2025

@containers/buildah-maintainers PTAL

@flouthoc flouthoc requested review from nalind and removed request for mtrmac July 9, 2025 17:35
@nalind
Copy link
Member

nalind commented Jul 9, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jul 9, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit d23f641 into containers:main Jul 9, 2025
37 checks passed
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Oct 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move cmd/buildah/passwd.go to tests/

7 participants