-
Notifications
You must be signed in to change notification settings - Fork 154
ci: Split RPM building into separate job #1813
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
Conversation
a8675d7 to
960b29e
Compare
|
OK pushed this too early, this one has some broken bits, still iterating. But hopefully illustrates the idea |
4281a51 to
bb2026e
Compare
bb2026e to
a53efec
Compare
|
Hi @cgwalters, can we land this PR? Thanks. |
|
/rebase |
a214a35 to
6ddbd01
Compare
|
Hm, did the rebase bot not work? |
I'm working on a PR to grant our bot |
Yep agree. That said, I think it can't be a PR, an organization admin needs to edit the bot permissions right now (unless we switch to something to git-ops it in the infra repo). Also on this topic, while I agree workflow edit perms should be fine, we also do want to be sure we actually are careful that no code submitted as part of a pull request gets access to that bot token. |
|
Granting workflow write permission to our bot is safe because rebase.yml only checks out the base repo. It's not checking out untrusted PR code — it's checking out the base repository. No attacker can inject code into that unless they already have write access. I think we can grant our bot |
47d58b3 to
4341f39
Compare
cgwalters
left a comment
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.
Thanks for helping push this forward!
I'd like to get #1809 in first though
| sudo podman build -t localhost/bootc-fsverity -f ci/Containerfile.install-fsverity | ||
| # Grant permission | ||
| sudo chown -R "$(id -u):$(id -g)" /home/runner/work/bootc/bootc |
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.
Yeah, we need to get these tests out of live mutating the host. There's no need for it anymore; the nested virt stuff is working well.
This splits the RPM package building into a separate CI job that runs before the integration tests. The built packages are then downloaded and used by the integration test jobs, avoiding redundant builds. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
The download-artifact already save RPMs into target/packages/ Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
4341f39 to
c4e2302
Compare
And collect info for flaky "error: System transaction in progress" Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
c0d9914 to
4e1d6ad
Compare
|
@cgwalters, I changed this PR to |
|
Looks sane to me, but this is technically my PR, so I can't approve - please go ahead and do so! |
This splits the RPM package building into a separate CI job that runs
before the integration tests. The built packages are then downloaded
and used by the integration test jobs, avoiding redundant builds.
Assisted-by: Claude Code (Sonnet 4.5)
Signed-off-by: Colin Walters walters@verbum.org