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: enable Fedora 36 aarch64 tasks on EC2 #14801

Merged
merged 1 commit into from
Jul 28, 2022
Merged

Cirrus: enable Fedora 36 aarch64 tasks on EC2 #14801

merged 1 commit into from
Jul 28, 2022

Conversation

lsm5
Copy link
Member

@lsm5 lsm5 commented Jul 1, 2022

Signed-off-by: Lokesh Mandvekar [email protected]

Does this PR introduce a user-facing change?

None

Piggybacks on #14569

@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 approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 1, 2022
@lsm5 lsm5 force-pushed the ec2-aarch64 branch 5 times, most recently from 57346a1 to 8785850 Compare July 1, 2022 14:58
lsm5
lsm5 commented Jul 1, 2022
View reviewed changes
.cirrus.yml Show resolved Hide resolved
@lsm5 lsm5 force-pushed the ec2-aarch64 branch 8 times, most recently from 37bb6e9 to 48d1ba2 Compare July 1, 2022 19:36
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
.cirrus.yml Outdated Show resolved Hide resolved
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
.cirrus.yml Show resolved Hide resolved
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
.cirrus.yml Outdated Show resolved Hide resolved
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
Copy link
Member

@cevich cevich left a comment

Choose a reason for hiding this comment

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

Looking good so far

@lsm5 lsm5 force-pushed the ec2-aarch64 branch 2 times, most recently from 14cd682 to c7a7aad Compare July 1, 2022 20:08
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
.cirrus.yml Outdated Show resolved Hide resolved
cevich
cevich reviewed Jul 1, 2022
View reviewed changes
.cirrus.yml Outdated Show resolved Hide resolved
@lsm5 lsm5 force-pushed the ec2-aarch64 branch 5 times, most recently from 9a01a58 to 2ab62ba Compare July 4, 2022 18:39
@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 27, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2022
@edsantiago
Copy link
Member 

        _ruamel_yaml.c:6:10: fatal error: Python.h: No such file or directory
            6 | #include "Python.h"
              |          ^~~~~~~~~~

@cevich
Copy link
Member

cevich commented Jul 27, 2022

Oof, WTF. Doing some digging...

@edsantiago
Copy link
Member

nonono, that's just because @lsm5 tried removing something, but I guess it can't be removed. Should be easy to just add it back.

cevich
cevich reviewed Jul 27, 2022
View reviewed changes
.cirrus.yml Outdated Show resolved Hide resolved
@cevich
Copy link
Member

cevich commented Jul 27, 2022

nonono, that's just because @lsm5 tried removing something, but I guess it can't be removed. Should be easy to just add it back.

Our VM images should be uniform across architectures. Since we're not seeing this error on the x86 tasks, something is clearly not uniform.

Looking in c/automation_images, cache_images/fedora_packaging.sh we don't mention python-devel anywhere. I'm taking a quick poke around to see if I can find where it gets brought.

@cevich
Copy link
Member

cevich commented Jul 27, 2022

Okay, it looks like the python-devel is missing from the regular images too. If you want to add back in the dnf install for those two tasks (Validate and consistency?) I think there's no need to check the architecture, just always install it. Maybe drop a TODO comment suggesting we bundle it into the images for stability. But I don't see that work as critical ATM.

@cevich
Copy link
Member

cevich commented Jul 27, 2022

Opened containers/automation_images#159

@lsm5 @cevich @edsantiago
Cirrus: enable Fedora 36 aarch64 tasks on EC2
da98c88 
new file:   test/e2e/config_arm64.go

Tests that fail on aarch64 have been skipped with
`skip_if_aarch64`.

Co-authored-by: Chris Evich <[email protected]>
Co-authored-by: Ed Santiago <[email protected]>
Signed-off-by: Lokesh Mandvekar <[email protected]>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2022
@lsm5
Copy link
Member Author

lsm5 commented Jul 27, 2022

@edsantiago @cevich removed that smallcontainer line and added python3-devel back. PTAL once again please, sorry about the PITA :(

cevich
cevich approved these changes Jul 27, 2022
View reviewed changes
Copy link
Member

@cevich cevich 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 27, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, lsm5

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

@cevich
Copy link
Member

cevich commented Jul 27, 2022

sorry about the PITA :(

It's no problem bud.

@edsantiago
Copy link
Member

remote is a flake which should go away as soon as #15090 merges.

@edsantiago
Copy link
Member

LGTM but that resource flake is going to cause a lot of pain. I foresee an immediate followup PR to skip it.

Thanks for your perseverance @lsm5.

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

Thanks everybody for working on this. I am very excited!

@vrothberg
Copy link
Member

/hold cancel

@openshift-ci openshift-ci bot 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 e1238ce into containers:main Jul 28, 2022
@lsm5 lsm5 deleted the ec2-aarch64 branch July 28, 2022 12:36
This was referenced Aug 10, 2022
Closed
Closed
@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 Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 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

@cevich cevich cevich approved these changes

Assignees

@cevich cevich

@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.
8 participants
@lsm5 @TomSweeneyRedHat @cevich @rhatdan @edsantiago @cdoern @vrothberg @openshift-merge-robot