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

Add libkrun Mac task #23290

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Conversation

cevich
Copy link
Member

@cevich cevich commented Jul 16, 2024

Depends on: containers/automation#208 + deployment into worker pool.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 16, 2024
@cevich cevich force-pushed the add_libkrun_task branch 6 times, most recently from 4c03ab7 to 9cd83de Compare July 16, 2024 17:18
@cevich
Copy link
Member Author

cevich commented Jul 16, 2024

Initial test-run

  1. Note: w/o brew install vfkit commented out, the brew install krunkit command fails with some "unlink" error. It's essentially complaining that /some/path/bin/vfkit already exists.

@cevich
Copy link
Member Author

cevich commented Jul 16, 2024

Testing note: Confirmed (expected) libkrun tests all fail when run on Mac's w/o libkrun.

@cevich
Copy link
Member Author

cevich commented Jul 16, 2024

Opened #23296

@cevich
Copy link
Member Author

cevich commented Jul 17, 2024

@edsantiago and/or @baude if you want to give the first two commits an initial look over, this PR is code-complete once containers/automation#208 is merged and deployed to the pool management VM (after I'm back from PTO).

@cevich
Copy link
Member Author

cevich commented Jul 19, 2024

FYI- I'll be off on PTO next week, so won't be enabling libkrun testing in Podman until I return. There are changes needed on the Mac's used for CI and I don't want to risk breaking something while I'm away.

Copy link
Member

@edsantiago edsantiago left a comment

Choose a reason for hiding this comment

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

Tentative LGTM with two concerns

.cirrus.yml Outdated Show resolved Hide resolved
contrib/cirrus/mac_runner.sh Outdated Show resolved Hide resolved
@cevich
Copy link
Member Author

cevich commented Jul 31, 2024

The libkrun mac tests are also passing. I will merge containers/automation#208 shortly, then after a day or so this PR can have final review and testing performed (i.e. after the pool has updated).

@cevich
Copy link
Member Author

cevich commented Jul 31, 2024

This PR can move forward on/after 2024-08-01T18:50:12+00:00, by then all the Macs in the pool will be running the new setup.

@cevich cevich force-pushed the add_libkrun_task branch 2 times, most recently from 8be721b to 48eef6d Compare August 1, 2024 13:26
@cevich
Copy link
Member Author

cevich commented Aug 1, 2024

Force-push:

  • Removed DO NOT MERGE commit to clear up diff (this still isn't ready to merge).
  • Renamed mac TEST_FLAVOR values to be consistent w/ other podman-machine testing tasks.
    (so they all sort together in the status page).

Signed-off-by: Chris Evich <[email protected]>
Issue 23296

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich marked this pull request as ready for review August 1, 2024 19:02
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 1, 2024
@cevich
Copy link
Member Author

cevich commented Aug 1, 2024

This is ready for final review and (assuming CI passes) merging.

@cevich
Copy link
Member Author

cevich commented Aug 1, 2024

Looks like the MacOS PM tests are both green, applehv and libkrun

@rhatdan
Copy link
Member

rhatdan commented Aug 1, 2024

LGTM
@mheon @baude @Luap99 PTAL

@mheon
Copy link
Member

mheon commented Aug 1, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2024
@rhatdan
Copy link
Member

rhatdan commented Aug 2, 2024

/approve

Copy link
Contributor

openshift-ci bot commented Aug 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, 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 /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 2, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 0bc0739 into containers:main Aug 2, 2024
90 of 91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. machine release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants