-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 Integration tests to Auditbeat CI pipeline #37892
Conversation
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
db2bb07
to
622fdfe
Compare
d98e7d0
to
b42246a
Compare
e1f1adc
to
5d64714
Compare
Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices) |
I guess these can be removed then https://github.com/elastic/beats/blob/main/auditbeat/Jenkinsfile.yml#L33-L50 |
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.
Looks good, thank you!
Left a comment to raise awareness on GH status notifications and that by using a matrix, failures will be collapsed under one entry. If you are ok with this I won't object, just wanted to make sure the behavior is clearly understood.
machineType: "${GCP_DEFAULT_MACHINE_TYPE}" | ||
artifact_paths: | ||
- "auditbeat/build/*.xml" | ||
- "auditbeat/build/*.json" | ||
notify: | ||
- github_commit_status: | ||
context: "auditbeat: Linux Integration Tests" | ||
context: "auditbeat: Linux x86_64 Integration Tests" |
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.
By using a matrix and having a static context we'll be facing the issue that all failures will be collapsed under the same GH status notification.
For example, right now, with passing tests, the notification looks like:
If we click the link it takes us to:
which represents both Auditbeat Linux x86_64 Integration Tests -- family/platform-ingest-beats-ubuntu-2004
and Auditbeat Linux x86_64 Integration Tests -- family/platform-ingest-beats-ubuntu-2204
So if there is a failure, clicking on the link will take the user to the top of the matrix section, not the specific failed job. Looking at the official BK docs it might be impossible to add {{matrix.image}}
-- or another additional property like {{matrix.name}}
-- under notify: ["github_commit_stats": {"context": "the message ... {{matrix.property}}"}]
(maybe worth testing to be 100% sure).
In any case, my main comment here is to raise awareness about the behavior and that with failures it won't be able to quickly see in the status section which specific image failed.
AWS_IMAGE_UBUNTU_2004_ARM64: "platform-ingest-beats-ubuntu-2004-aarch64" | ||
AWS_IMAGE_UBUNTU_2204_ARM64: "platform-ingest-beats-ubuntu-2204-aarch64" | ||
|
||
IMAGE_MACOS_ARM64: "generic-13-ventura-arm" |
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.
nit: re: renaming IMAGE_MACOS_ARM
to IMAGE_MACOS_ARM64
, ideally we do this in a future PR where we change this as a convention for all the pipelines. For now, it might be ok to leave the name as is, given that there is no 32bit ARM architecture for macOS?
This pull request is now in conflicts. Could you fix it? 🙏
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
With auditbeat, there can be differences in Linux kernels and system architectures that can affect behaviour. This adds additional system types to the integration tests that will test more of these configurations.
023021a
to
15d5be4
Compare
I'd like to revive this PR. I've added all the Linux 'platform-ingest-beats' VM images that currently exist and removed the variable name changes that weren't necessary. @leehinman, @dliappis, @pkoutsovasilis could you review this again, since it's changed since you last looked at it? |
LGTM. @oakrizan can you please review? Any overlap with your current changes? |
We deliberately were avoiding matrix usage. In addition to what Dimitrios said about matrix - if one of the matrix-based steps fails, then particular GH check will hang in "Pending" state. |
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.
lgtm
Refactor the CI integration test steps for the Auditbeat pipeline to use build matrices for Linux tests. By doing this, it will be easier to add additional VM types to the integration tests in the future. With this, the integration tests will run on Ubuntu 20.04, 22.04, 24.04 for both x86_64 and arm64, and RHEL 9 x86_64. (cherry picked from commit efbc4ff)
Refactor the CI integration test steps for the Auditbeat pipeline to use build matrices for Linux tests. By doing this, it will be easier to add additional VM types to the integration tests in the future. With this, the integration tests will run on Ubuntu 20.04, 22.04, 24.04 for both x86_64 and arm64, and RHEL 9 x86_64. (cherry picked from commit efbc4ff)
Refactor the CI integration test steps for the Auditbeat pipeline to use build matrices for Linux tests. By doing this, it will be easier to add additional VM types to the integration tests in the future. With this, the integration tests will run on Ubuntu 20.04, 22.04, 24.04 for both x86_64 and arm64, and RHEL 9 x86_64. (cherry picked from commit efbc4ff) Co-authored-by: Michael Wolf <[email protected]> Co-authored-by: Andrew Kroh <[email protected]>
Refactor the CI integration test steps for the Auditbeat pipeline to use build matrices for Linux tests. By doing this, it will be easier to add additional VM types to the integration tests in the future. With this, the integration tests will run on Ubuntu 20.04, 22.04, 24.04 for both x86_64 and arm64, and RHEL 9 x86_64. (cherry picked from commit efbc4ff) Co-authored-by: Michael Wolf <[email protected]>
Proposed commit message
Refactor the CI integration test steps for the Auditbeat pipeline to use build matrices for Linux tests. By doing this, it will be easier to add additional VM types to the integration tests in the future.
With this, the integration tests will run on Ubuntu 20.04, 22.04, 24.04 for both x86_64 and arm64, and RHEL 9 x86_64.
Checklist
I have commented my code, particularly in hard-to-understand areasI have made corresponding changes to the documentationI have made corresponding change to the default configuration filesI have added an entry inCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.