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.

Sign up for GitHub

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

Jump to bottom

Cirrus: Use pre-installed bats #14719

Merged
merged 2 commits into from
Jul 28, 2022
Merged

Cirrus: Use pre-installed bats #14719

merged 2 commits into from
Jul 28, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented Jun 23, 2022 •
edited
Loading

Ref: containers/automation_images#140
and
containers/automation_images#149
and
containers/automation_images#146

Does this PR introduce a user-facing change?

None

@cevich cevich requested a review from edsantiago June 23, 2022 21:06
@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none labels Jun 23, 2022
@cevich cevich mentioned this pull request Jun 23, 2022
Merged
@edsantiago
Copy link
Member

LGTM

@edsantiago
Copy link
Member

ubuntu remote root failed every single test. Reason: /run/podman does not exist. Solution: re-run with terminal, mkdir /run/podman before the tests start. This seems to have fixed things.

@containers/podman-maintainers someone please figure out why /run/podman doesn't exist on ubuntu, and then figure out how to fix that please. @giuseppe I'm tagging you because I suspect this has something to do with XDG, whatever that is.

@cevich
Copy link
Member Author

cevich commented Jun 24, 2022

@edsantiago & @containers/podman-maintainers I'm 80% certain (didn't look at logs) these are most likely reproductions of the same/similar issues @lsm5 uncovered in his (open) image update PR. That's why I marked this one as dependent on his. That said, they are actual issues we need to fix, but I'm content to let Lokesh drive the update in his PR first, since he's learning how to "solo" the complete workflow. So please, head over to #14397 and see how you can help 😉

@cevich cevich mentioned this pull request Jun 24, 2022
Closed
@cevich cevich force-pushed the use_preinstalled_bats branch from 971e55d to 277ac44 Compare June 29, 2022 17:09
@cevich cevich mentioned this pull request Jun 30, 2022
Merged
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2022
@cevich cevich mentioned this pull request Jul 11, 2022
Closed
@cevich cevich force-pushed the use_preinstalled_bats branch from 277ac44 to 0578020 Compare July 14, 2022 19:57
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 14, 2022
@cevich
Copy link
Member Author

cevich commented Jul 14, 2022

Force-push: Rebased & resolved conflict

@cevich cevich force-pushed the use_preinstalled_bats branch from 0578020 to 20f6005 Compare July 15, 2022 14:45
@cevich
Copy link
Member Author

cevich commented Jul 15, 2022

Force-push: Updated to newer images (built yesterday).

@edsantiago
Copy link
Member

Reminder: you cannot use newer images. We are frozen until criu goes stable. Which, as of this moment, it is not.

@lsm5
Copy link
Member

lsm5 commented Jul 18, 2022

Reminder: you cannot use newer images. We are frozen until criu goes stable. Which, as of this moment, it is not.

It's set to land in stable, so should hopefully be done in 24 hrs or less.

@cevich cevich force-pushed the use_preinstalled_bats branch from 20f6005 to ab6d642 Compare July 19, 2022 14:15
@cevich
Copy link
Member Author

cevich commented Jul 19, 2022

Force-push: Rebased on #14963 and main, + added updated VM images built yesterday (excluding F35).

@cevich
Copy link
Member Author

cevich commented Jul 20, 2022

Note (happening in parallel): #14972

@cevich cevich force-pushed the use_preinstalled_bats branch from ab6d642 to 80447d2 Compare July 20, 2022 15:25
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2022
@cevich cevich force-pushed the use_preinstalled_bats branch from 80447d2 to d960fde Compare July 20, 2022 16:08
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2022
@cevich cevich force-pushed the use_preinstalled_bats branch from d960fde to b4aed3e Compare July 20, 2022 19:54
@cevich cevich mentioned this pull request Jul 21, 2022
Merged
@edsantiago edsantiago mentioned this pull request Jul 21, 2022
Merged
@edsantiago
Copy link
Member

@cevich please rebase when convenient - now I can focus on reviewing this. I'm mostly out today, but will try to make time early afternoon.

edsantiago
edsantiago reviewed Jul 22, 2022
View reviewed changes
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.

I won't block over it, but I can't approve either.

.cirrus.yml Show resolved Hide resolved
@cevich cevich force-pushed the use_preinstalled_bats branch from 21a6100 to d31f9a5 Compare July 25, 2022 13:27
@cevich
Copy link
Member Author

cevich commented Jul 25, 2022

/hold

There's a problem with these images, new ones are needed.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 25, 2022
@edsantiago
Copy link
Member

If by "problem" you mean the ubuntu runc bug, I don't think there's any reason to block this PR on that. I'm fine with merging this.

@edsantiago
Copy link
Member

/lgtm

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

rhatdan commented Jul 26, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 26, 2022

[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 Jul 26, 2022
@cevich cevich force-pushed the use_preinstalled_bats branch from d31f9a5 to 1c699ad Compare July 27, 2022 18:19
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2022
@cevich
Copy link
Member Author

cevich commented Jul 27, 2022

Force-push: Rebased + separated VM Image update commit + use same images from #14801

@vrothberg
Copy link
Member

Ready to go in?

vrothberg
vrothberg reviewed Jul 28, 2022
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

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

rhatdan commented Jul 28, 2022

Needs a rebase.

cevich added 2 commits July 28, 2022 11:15
@cevich
Remove bats installation script and make target
a24cc46 
While convenient, it can be problematic to rely on a Makefile to install
software.  This was found to be the case across multiple environments
WRT `bats`.  Fix this by removing the install script and target.  A
future commit will ensure the correct version of `bats` is present in
all CI environments where it's required.

Signed-off-by: Chris Evich <[email protected]>
@cevich
Cirrus: Minor YAML cleanup
a53a0fc 
A prior change added extra whitespace when commenting out several
sections to temporarily disable F35 testing.  This restores the sections
to proper indentation, so (in the future) only the `#` character needs
to be removed.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the use_preinstalled_bats branch from 1c699ad to a53a0fc Compare July 28, 2022 15:29
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2022
@edsantiago
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2022
@edsantiago edsantiago removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 28, 2022
@openshift-merge-robot openshift-merge-robot merged commit 509ad42 into containers:main Jul 28, 2022
@cevich cevich deleted the use_preinstalled_bats branch April 18, 2023 14:44
@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 Aug 31, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vrothberg vrothberg vrothberg left review comments

@edsantiago edsantiago edsantiago left review comments

Assignees

@edsantiago edsantiago

@vrothberg vrothberg

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. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cevich @edsantiago @lsm5 @rhatdan @vrothberg @openshift-merge-robot