This repository has been archived by the owner on Sep 17, 2024. It is now read-only.
feat: support using Beats CI snapshots #205
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
cachedout
approved these changes
Jul 29, 2020
cli/shell/shell.go
Outdated
| func GetEnvBool(key string) bool { | ||
| s := os.Getenv(key) | ||
| if s == "" { | ||
| return false |
There was a problem hiding this comment.
I kind of wonder if this should return something else here instead of false. (Perhaps nil?) It seems like this behavior might introduce hard-to-handle error conditions, where a value is incorrectly interpreted as being truly false instead of an error in processing the lookup itself.
There was a problem hiding this comment.
Mmmm, I see your point: so if the env var is not present, then do not consider it as false... In this particular use case, we are going to check for true, only. If true, then execute the proper code, otherwise use default. But it could be the case that we use this function again checking for false, and if the var is not present, it will satisfy the condition
There was a problem hiding this comment.
I think returning (bool, error) will do the trick, although we'll have to handle errors on the consume side.
There was a problem hiding this comment.
I added your suggestion in 3881a99, including unit tests for verification.
We are removing the OP prefix for vars related to suites
|
@EricDavisX with these changes now we will be able to run tests with the artifacts for a PR :) |
kuisathaverat
approved these changes
Jul 30, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
It supports downloading the elastic-agent binaries from the Google Cloud Storage bucket used by Beats CI on every merge-to-master. For that, as the objects in the bucket are not public, we are listing bucket's contents, looking for a match for the agent we want to use in tests.
To enable this feature, set the
ELASTIC_AGENT_USE_CI_SNAPSHOTSenvironment variable totrue, and the tests will not fetch the binaries from the default location.Besides, we are using the Docker image for all Elastic components from Elastic's docker registry, using the observability-ci namespace. This namespace contains snapshots from each merge-to-master, and all PRs for Beats.
To enable this feature, set the
ELASTIC_AGENT_STAND_ALONE_VERSIONenvironment variable to a valid and puushed tag i.e.78a762c76080aafa34c52386341b590dac24e2dfor eventuallypr-20323, and the tests will not fetch the default image (8.0.0-SNAPSHOT).Finally, we are arranging the existing env vars into a different name convention, prefixing them with the test suite that the var belongs to, removing the
OP_prefix when needed.Why is it important?
The release process for the binaries is not as reliable as we need, as it could take days until we have a new snapshot, blocking this test suite. For that reason we want to support a way to switch to another location if the official binaries are not working or does not contain latest code or fixes.
About the small refactors, consistent names that are easy to read are names easy to understand, that's the motivation for that change.
Checklist
make noticein the proper directory)Author's Checklist
How to test this PR locally
Using the latest snapshot
You should see something similar to these log entries, related to Google Cloud Storage APIs:
On the other hand, if not setting that variable, or setting to something not true:
You should see something similar to these logs entries, related to the official artifacts endpoint:
Stand-alone mode: using another Docker tag
You should see something similar to these logs entries, related to the docker-compose pull command fetching the tag you selected:
Related issues
Follow-ups
An interesting follow-up will be consuming the snapshot for a PR, instead of the merge-to-master snapshot. See elastic/beats#20274 and elastic/beats#20323 for related work on Beats side.