-
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
[Filebeat] Integration tests in CI for AWS-S3 input #27491
[Filebeat] Integration tests in CI for AWS-S3 input #27491
Conversation
Pinging @elastic/integrations (Team:Integrations) |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
there was an error in the directory name and terraform apply failed:
anyway the CI status was green cc @v1v |
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.
Nice, added some suggestions.
x-pack/filebeat/Jenkinsfile.yml
Outdated
withModule: true ## run the ITs only if the changeset affects a specific module. | ||
dirs: ## run the cloud tests for the given modules. | ||
- "x-pack/filebeat/input/awss3/_meta/terraform" # needed by startCloudTestEnv in order to apply terraform | ||
- "x-pack/filebeat/input/awss3" # the actual dir where tests reside |
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 looks a bit hacky, I would prefer if we don't need to add both directories.
I see some options:
- Add a
terraformDir
config to thiscloud
step, so we can explicitly configure where is the terraform scenario and where the tests. This would have the advantage of allowing to use the same scenario for tests residing in multiple scenarios. - Add a terraform file where the tests reside, so we don't have to add both directories. This terraform file could use
_meta/terraform
as a module. - odify
startCloudTestEnv
to use./_meta/terraform
if there are no terraform files in the
fixes: This version of Terraform has an outdated GPG key and is unable to verify new provider releases
|
@@ -876,6 +876,7 @@ def startCloudTestEnv(Map args = [:]) { | |||
// If it failed then cleanup without failing the build | |||
sh(label: 'Terraform Cleanup', script: ".ci/scripts/terraform-cleanup.sh ${folder}", returnStatus: true) | |||
} | |||
error('startCloudTestEnv: terraform apply failed.') |
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.
Enforce the error is reported after the catch closure
not sure it is correct: I can see that if I can define |
@aspacca , we can syncup if needed, please poke me. 5bf9090 is half done, there are a couple of changes to be done:
|
Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
@v1v everything seems fine: we don't need to introduce As far as I can see only problem left is that the step is triggered when github label It would be valuable in my opinion to add something like |
// Allow AWS credentials when the build was configured to do so with: | ||
// - the cloudtests build parameters | ||
// - the aws github label | ||
if (params.allCloudTests || params.awsCloudTests || matchesPrLabel(label: 'aws')) { |
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 should help to configure the AWS credentials when the aws
github label was assigned to the Pull Request as explained in #27491 (comment)
There are some failures when applying the terraform, see the below screenshot As far as I see it's related to this terraform resource:
No idea whether it's a temporary glitch or something else Let me rerun the build again |
/test |
/test |
|
/test |
@v1v all good, let's merge? |
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 @aspacca for making this happen!! 💯
* [Filebeat] Integration tests in CI for AWS-S3 input (cherry picked from commit 9d4597a)
* [Filebeat] Integration tests in CI for AWS-S3 input (cherry picked from commit 9d4597a)
* master: (39 commits) [Heartbeat] Move JSON tests from python->go (elastic#27816) docs: simplify permissions for Dockerfile COPY (elastic#27754) Osquerybeat: Fix osquery logger plugin severy levels mapping (elastic#27789) [Filebeat] Update compatibility function to remove processor description on ES < 7.9.0 (elastic#27774) warn log entry and no validation failure when both queue_url and buck… (elastic#27612) libbeat/cmd/instance: ensure test config file has appropriate permissions (elastic#27178) [Heartbeat] Add httpcommon options to ZipURL (elastic#27699) Add a header round tripper option to httpcommon (elastic#27509) [Elastic Agent] Add validation to ensure certificate paths are absolute. (elastic#27779) Rename dashboards according to module.yml files for master (elastic#27749) Refactor vagrantfile, add scripts for provisioning with docker/kind (elastic#27726) Accept syslog dates with leading 0 (elastic#27775) [Filebeat] Add timezone config option to decode_cef and syslog input (elastic#27727) [Filebeat] Threatintel compatibility updates (elastic#27323) Add support for ephemeral containers in elastic agent dynamic provider (elastic#27707) [Filebeat] Integration tests in CI for AWS-S3 input (elastic#27491) Fix flakyness of TestFilestreamEmptyLine (elastic#27705) [Filebeat] kafka v2 using parsers (elastic#27335) Update Kafka version parsing / supported range (elastic#27720) Update Sarama to 1.29.1 (elastic#27717) ...
* [Filebeat] Integration tests in CI for AWS-S3 input
Enhancement
What does this PR do?
#27199 introduced new integration tests that rely on terraform (https://github.com/elastic/beats/blob/master/x-pack/filebeat/input/awss3/input_integration_test.go)
We will run them in CI, similarly to what happens for Metricbeat (https://github.com/elastic/beats/blob/master/x-pack/metricbeat/Jenkinsfile.yml#L35-L47)
Why is it important?
Avoiding regression
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Closes #27469
Use cases
Screenshots
Logs