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.

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 serverless beats tests #3658

Merged

Conversation

fearful-symmetry
Copy link
Contributor

What does this PR do?

This is (hopefully) the final stage of getting serverless testing working. This Adds 17 serverless tests, as well as a mage role to run the tests. Assuming you have a beats binary built, you can run mage integration:testBeatServerless metricbeat

This does not include any buildkite steps to run the tests; I'm not totally sure how we want to schedule them, and if that should be part of this PR.

Why is it important?

We need serverless tests.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • I have added an integration test or an E2E test

- label: "Serverless Beats Tests"
key: "serverless-beats-integration-tests"
command: ".buildkite/scripts/steps/beats_tests.sh"
if: "build.env('CRON') == 'yes'"
Copy link
Member

Choose a reason for hiding this comment

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

Where is the cron schedule controlled? Can you link to it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I was going to add it once the PR was merged, but the control panel for it is here: https://buildkite.com/elastic/elastic-agent/settings/schedules

Copy link
Member

Choose a reason for hiding this comment

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

Add a link to that as a comment right above this so future us can find it easily

@cmacknz
Copy link
Member

cmacknz commented Nov 6, 2023

Do you have a link to the Beats tests passing on Buildkite, since they aren't running as part of the PR automation?

@fearful-symmetry
Copy link
Contributor Author

fi


echo "testing auditbeat..."
Copy link
Member

Choose a reason for hiding this comment

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

We can skip testing auditbeat since our team doesn't maintain it. None of the code we are testing is really Beat specific regardless.

mkdir -p build
cd build

git clone [email protected]:elastic/beats.git
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to clone and build Beats?

Why don't we just download the latest SNAPSHOT image using the artifacts API?

Copy link
Member

Choose a reason for hiding this comment

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

+1

If we need to clone the repo, maybe we can get by with a partial clone (for example git clone --filter=tree:0 [email protected]:elastic/beats.git) to improve speed ?

@cmacknz
Copy link
Member

cmacknz commented Nov 10, 2023

Your sample test run doesn't contain any Beat logs. If these tests fail how do we debug them?

There are no test artifacts generated either https://buildkite.com/elastic/elastic-agent/builds/4490#018b87a1-0854-4559-b675-20e3033f15e4

So there is no test report generated to make the failures easier to see.

It also looks like it is running every other test. Is that the intent for the scheduled job? I would have it only run the serverless Beats test so it isn't going to fail for any other random reason.

Copy link
Member

@pchila pchila left a comment

Choose a reason for hiding this comment

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

A couple of comments left about the pipeline step script and the mage target.
Overall code is ok, I am not too fond of the mixing of beats serverless and elastic agent integration tests being guarded only by a condition on the TestSuite and a mage target that would allow to run those tests against a stateful stack (not sure that we have a usecase for that).

.buildkite/scripts/common.sh Show resolved Hide resolved
mkdir -p build
cd build

git clone [email protected]:elastic/beats.git
Copy link
Member

Choose a reason for hiding this comment

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

+1

If we need to clone the repo, maybe we can get by with a partial clone (for example git clone --filter=tree:0 [email protected]:elastic/beats.git) to improve speed ?

Comment on lines 60 to 62
if ! run_test_for_beat metricbeat; then
exit 1
fi
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: the script would fail anyway if run_test_for_beat metricbeat returns != 0 thanks to the set -e above
This means we don't need to handle the return code unless we wanna try to run all the test and return a correct exit status at the end (similar to what's done on lines 53-54)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, good point!

magefile.go Show resolved Hide resolved
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

//go:build integration
Copy link
Member

Choose a reason for hiding this comment

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

@cmacknz my understanding is that today we have the guard in the SetupSuite() below (lines 71-73):

if runner.testbeatName == "elastic-agent" {
		runner.T().Skipf("tests must be run against a beat, not elastic-agent")
	}

@fearful-symmetry is this what's stopping these tests from failing when launching elastic-agent integration tests or am I missing something ?

@fearful-symmetry
Copy link
Contributor Author

@pchila not sure what you mean? The if runner.testbeatName == "elastic-agent" line was just the simplest way to constrain the tests, considering this entire PR is meant to be reverted once buildkite is available in the beats repo.

Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@fearful-symmetry fearful-symmetry merged commit d5f34df into elastic:main Nov 20, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants