-
Notifications
You must be signed in to change notification settings - Fork 24
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
ppc64le core-bpf testing and PR testing #1448
ppc64le core-bpf testing and PR testing #1448
Conversation
Skipping CI for Draft Pull Request. |
b093279
to
40d3d97
Compare
40d3d97
to
e86fcce
Compare
73a3609
to
c6b7918
Compare
e86fcce
to
622b6d6
Compare
bdfede0
to
81d8f68
Compare
4e05a88
to
de0f77e
Compare
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, looks good. I've left few commentaries, but suggest to follow up on them in a separate PR. Otherwise not sure what's your plan to proceed -- s390x tests seems to pass, but few others are failing, is that expected?
@@ -11,11 +11,12 @@ on: | |||
outputs: | |||
collector-builder-tag: | |||
description: The builder tag used by the build | |||
value: ${{ jobs.build-builder-image.outputs.collector-builder-tag || 'master' }} | |||
value: ${{ jobs.build-builder-image.outputs.collector-builder-tag || '3.16.x-195-g8f32e71fad' }} |
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.
What was this image exactly? And would it be dropped before merging?
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.
This was taken from mauro's original vanilla PR and will be dropped before merging.
@@ -110,6 +143,24 @@ jobs: | |||
-e @'${{ github.workspace }}/ansible/secrets.yml' \ | |||
ansible/ci-build-builder.yml | |||
|
|||
- name: Build s390x images |
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.
This feels a bit weird, is it easier to build s390x images in a separate step, not as part of the previous one?
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.
For now, yeah it's a little simpler, just because we can force the previous step to use localhost only, but this step will use the entire inventory (which is only one VM at the moment)
When I implement full multi-arch native builds I'll hopefully combine these steps
.github/workflows/collector-slim.yml
Outdated
ANSIBLE_CONFIG: ansible/ansible.cfg | ||
VM_TYPE: rhel-s390x | ||
|
||
- name: Save CMake cache |
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.
Will this cache be used automatically, or does it have to be explicitly included? It's probably a good idea to add a condition to allow running without cache if specified via a label.
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.
This was just for debugging the build, I'll remove it.
@@ -19,7 +19,7 @@ | |||
- name: Build full image | |||
when: | |||
- build_full_image | |||
- arch != 'arm64' | |||
- arch != 'arm64' and arch != 'ppc64le' and arch != 's390x' |
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.
This is the part where we temporarily disable full builds for multi-arch, right?
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, it is. I'll remove before merging.
version: "{{ collector_git_sha }}" | ||
refspec: "+{{ collector_git_ref | replace('refs/', '') }}" | ||
recursive: true | ||
when: arch == "s390x" |
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.
Not sure I follow, why to clone only for s390x?
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.
Because it's the only platform (currently) that is building on a separate machine to the GHA runner. All the other platforms can build from the existing checked out source
Seems to have been an SSH key mix up following my ansible changes; I've fixed that now so this run should pass 🤞 |
5528455
to
dc521ae
Compare
Description
Should enable core-bpf testing for ppc64le. Once it is working/failing, I'll rebase it over the vanilla branch.
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why you did not do so. (Valid reasons include "CI is sufficient" or "No testable changes")
In addition to reviewing your code, reviewers must also review your testing instructions, and make sure they are sufficient.
For more details, ref the Confluence page about this section.