A friendly reminder that this PR had no activity for 30 days.

A friendly reminder that this PR had no activity for 30 days.

A friendly reminder that this PR had no activity for 30 days.

@lsm5 are you still working on this?

(If so, this is not urgent from my POV, no need to push it to the front of your queue — I’d just like to close the PR if there is absolutely no chance this work will continue.)

@mtrmac I do plan to work on this. But may take a while. I have bookmarked it for myself so I don't lose track. So please feel free to close it.

Thanks, if it is still relevant, let’s keep it open. It might help, or inspire, other contributors.

A friendly reminder that this PR had no activity for 30 days.

A friendly reminder that this PR had no activity for 30 days.

@mtrmac @vrothberg @cevich RE: system tests in CI, is it critical to run them in a container or would we be ok with running test-system-local ? Asking mainly RE: packit + tmt tests.

@edsantiago is authoritative for system tests (question above).

Forgive me please, I don't understand the question. It's been a while since I've looked at skopeo gating tests, but my recollection is that they run on a plain system, via dnf install skopeo-tests. What's the context for "in a container"?

@mtrmac @vrothberg @cevich RE: system tests in CI, is it critical to run them in a container or would we be ok with running test-system-local ?

IIRC the system tests run a registry server (container). So… running -local would be much simpler in that there would be no need for nested Podman; OTOH the tests would be much less isolated from the surrounding environment, which might be a difficulty.

I’m not immediately sure how much custom tooling exists in the CI container, and would need to be reproduced; https://github.com/containers/automation_images/blob/main/skopeo_cidev/setup.sh is relevant to that, but IIRC almost all of that exists for integration, not system, tests.

/packit test

So, unfortunately, the issue we are hitting is a bug in Packit:

packit/packit-service#2028

It was not considered a bug before, as people were not hitting it and we did not know it had this consequence.
They are working on a fix.

So, unfortunately, the issue we are hitting is a bug in Packit:

packit/packit-service#2028

It was not considered a bug before, as people were not hitting it and we did not know it had this consequence. They are working on a fix.

Thanks @thrix . I'll disable the rhel tests for now.

@mtrmac @vrothberg @cevich RE: system tests in CI, is it critical to run them in a container or would we be ok with running test-system-local ?

IIRC the system tests run a registry server (container). So… running -local would be much simpler in that there would be no need for nested Podman; OTOH the tests would be much less isolated from the surrounding environment, which might be a difficulty.

I’m not immediately sure how much custom tooling exists in the CI container, and would need to be reproduced; https://github.com/containers/automation_images/blob/main/skopeo_cidev/setup.sh is relevant to that, but IIRC almost all of that exists for integration, not system, tests.

ack, the test-system target seems to spawn some container too. Anyway, I'll try to mimic the cirrus setup in tmt. Thanks

@mtrmac PTAL. Only added x86_64 integration, validate and unit tests for now.

@mtrmac PTAL. Only added x86_64 integration, validate and unit tests for now.

If -local targets in CI tests are acceptable, that'd save us the pain of quay.io/libpod/skopeo_cidev image maintenance. Else, I'll look at making that image multiarch.

Hmm, and I guess running the tests in the skopeo_cidev container renders the multiple testing-farm- distro tests pointless.

If -local targets in CI tests are acceptable, that'd save us the pain of quay.io/libpod/skopeo_cidev image maintenance. Else, I'll look at making that image multiarch.

From an extremely brief look at the Packit / TMT documentation, the tests seem to run in an isolated VM (chroot? container?) and install RPM test subjects into that. Running an extra level of isolation in a container seems unnecessary in that case. (Also, to my knowledge no-one has been using the $CONTAINER_RUN targets for some time, they might have been broken for months without anyone noticing.) So, using the -local targets does seem better to me.

Also, testing the binary, if it comes from a RPM, in its expected distribution, seems clearly more valuable than running it in some other frozen / differently-built container.

OTOH, at least one reason why the container exists, and is regularly rebuilt, is so that individual Skopeo CI runs don’t need to install and compile all of those tools, which rarely change, on every single CI RUN. Hence, in Cirrus, runner.sh uses the -local targets but fetches pre-built binaries from that container.

If the Packit tests are ever intended to be merge-blocking, something like that to shorten the CI time would be very valuable.

And, also, I’d prefer not to maintain the test tool setup code ( https://github.com/containers/automation_images/blob/main/skopeo_cidev/setup.sh and make tools ) in two duplicated copies, if that could be helped.

@mtrmac thanks a lot for all the comments. Setting back to draft while I work through them.

Failed to load packit config file:

Please correct data and retry.

For more info, please check out the documentation or contact the Packit team. You can also use our CLI command validate-config or our pre-commit hooks for validation of the configuration.

Tests failed. @containers/packit-build please check.

@mtrmac RE: your earlier questions:

What is the longer-term plan? Is this going intended to be in addition to .cirrus.yml, or eventually replace it?

Plan is to eventually get rid of cirrus and depend only on TMT assuming we'll have all functionality sooner or later via TMT.

Update: I have pretty much redone the whole PR with scope limited to only triggering system tests as we'll need those for official Fedora and CentOS Stream gating too. So, I feel best to get those in first. Please also check out the updated PR description ^ for details on the 2 included commits.

You'll see a bunch of new jobs in CI. The ones with suffix dev will run on main branch. The ones with suffix release will show as Queued but are expected to not run on main. Reverse should happen on release-* branches once this change lands there.(Untested on release branches but I guess we'll find out when we cherrypick).

• systemtest/tmt/* lists the actual test and the script.
• plans/main.fmf configures the test environment based on the initiator (packit, fedora ci, human etc.). Any filtering of tests to run based on the environment will also be done here. For example, see the same on container-selinux

In this PR, I'm no longer changing the Makefile or scripts in hack/. So, that should make things a lot simpler.

I'll work on the integration / lint and ostree tests in followup PRs. I'll keep your other comments here in mind while I work on those later.

Another advantage of TMT is convenient reverse dependency testing. See this c/common PR. That will run the tests maintained in skopeo on every PR in c/common.

/packit test

One more thing — not blocking this PR, just to possibly think about.

For historical reasons, c/image’s CI, via the test-skopeo Make rule, tests each c/image PR against the Skopeo unit+integration+system tests, by vendoring the proposed c/image PR into the Skopeo codebase, building Skopeo, and running tests. And these tests are important — they are ~the only tests of the c/image registry client, as well as important e2e tests of other c/image functionality like signing.

If we completely removed the current way of running system tests in favor of TMT testing (based on Skopeo RPMs?), we would need some way to continue to run the tests against c/image PRs. I have no idea whether that would be trivial, difficult, or how it might affect the packit/TMT test automation design.

Alternatively, the testing structure could be cleaned up — instead of c/image relying on Skopeo’s tests, we could move most of the tests into the c/image codebase, and rework them them not to rely on the Skopeo repo (and codebase?). That would, conceptually, be a better design — but, also, probably quite a lot of work and so far there was never a strong enough reason to spend time on this, so we have been relying on the test-skopeo mechanism.

One more thing — not blocking this PR, just to possibly think about.

For historical reasons, c/image’s CI, via the test-skopeo Make rule, tests each c/image PR against the Skopeo unit+integration+system tests, by vendoring the proposed c/image PR into the Skopeo codebase, building Skopeo, and running tests. And these tests are important — they are ~the only tests of the c/image registry client, as well as important e2e tests of other c/image functionality like signing.

Ack, this should be doable via TMT as well. I'll keep this in mind for the followup PR to enable additional tests.

If we completely removed the current way of running system tests in favor of TMT testing (based on Skopeo RPMs?), we would need some way to continue to run the tests against c/image PRs. I have no idea whether that would be trivial, difficult, or how it might affect the packit/TMT test automation design.

Ack. In this current PR, the system tests use skopeo from the copr rpm, but we can have the tests run independent of rpms too.

Additionally, TMT also allows tests maintained in one repo to be fetched and run in another repo. For example: podman Fedora rpm gating tests fetching additional tests maintained in the toolbox rpm. https://src.fedoraproject.org/rpms/podman/blob/rawhide/f/plans/toolbox.fmf#_7-10
I imagine we can have the testing logic be shared with some conditionals to depend on rpm / vendored source.

Additionally, on the c/image side, there might be some packit action possible to build an rpm by vendoring the PR into skopeo (I have yet to try such a thing).

Alternatively, the testing structure could be cleaned up — instead of c/image relying on Skopeo’s tests, we could move most of the tests into the c/image codebase, and rework them them not to rely on the Skopeo repo (and codebase?). That would, conceptually, be a better design — but, also, probably quite a lot of work and so far there was never a strong enough reason to spend time on this, so we have been relying on the test-skopeo mechanism.

SGTM as well.

Sorry, totally missed this entire comment earlier...

Thanks!

I’m not sure I’m reading that correctly, but some logs seem to suggest that the TMT tests run in ~2 minutes vs. previous 13. If that represents wall-clock time, that would be a really impressive improvement.

Great to hear. IIUC, the time mentioned only includes start to finish of the actual test. Things like rpm build completion, waiting/provisioning testing-farm instance could add up. I'll check with the concerned teams if they could also report total time.

Is there any chance to correctly represent the status in GitHub (either as “skipped”, or not including the other jobs in at all)?

I expect that eventually we’ll want to make these jobs merge-blocking, and I don’t know how that would work with a “queued” never-finishing job. It’s also easier for newcomers/occasional contributors when the tool status matches the intuitive understanding.

Yes, I've notified packit team about changing that: packit/packit-service#2678 (comment)

Rebased and pushed just now. Will merge after the run succeeds. Thanks @mtrmac

Thanks! LGTM, to the limited extent I understand things.

Please merge whenever appropriate.

@mtrmac I think you'll need to Approve this PR before it can be merged.