-
Notifications
You must be signed in to change notification settings - Fork 152
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
Integration tests: Parse agent version from version.go #6611
base: main
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
# Parsing version.go. Will be simplified here: https://github.com/elastic/ingest-dev/issues/4925 | ||
AGENT_VERSION=$(grep "const defaultBeatVersion =" version/version.go | cut -d\" -f2) | ||
AGENT_VERSION="${AGENT_VERSION}-SNAPSHOT" | ||
export AGENT_VERSION | ||
echo "~~~ Agent version: ${AGENT_VERSION}" |
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.
I see this in two places, do you mind moving this logic to a make goal or something else? Otherwise, it's hard to maintain.
For instance, the APM Server uses:
- https://github.com/elastic/apm-server/blob/42e0cbf5b049a92b5536f34090b15f5eb0a7167e/go.mk#L11
- and https://github.com/elastic/apm-server/blob/42e0cbf5b049a92b5536f34090b15f5eb0a7167e/Makefile#L146-L149
Therefore the call is as simple as this https://github.com/elastic/apm-server/blob/42e0cbf5b049a92b5536f34090b15f5eb0a7167e/.buildkite/scripts/dra.sh#L39
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 is the second place? If it's the PS file for Windows then unfortunately it's hard to avoid(Possible but the solution is controversial).
I also added a comment above(in the code). The mentioned issue will replace it with AGENT_VERSION=$(cat .agent-version)
.
I would like to ask you a QQ about your example if you don't mind. Wouldn't it be easier to create a similar .version
file and embed it to the version.go
? Then getting the version will be as simple as it mentioned above
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 is the second place? If it's the PS file for Windows then unfortunately it's hard to avoid(Possible but the solution is controversial).
Yep
Wouldn't it be easier to create a similar .version file and embed it to the version.go?
I think so, thanks for bringing this to my attention, if there is a plan, then all good.
What does this PR do?
The way how we defined the agent version for integration tests wasn't reliable. It works correctly on the main branch but might work incorrectly on 8.17. That blocks the backports to 8.17 and 8.16. The new approach parses version.go.
Why is it important?
Checklist
./changelog/fragments
using the changelog toolDisruptive User Impact
How to test this PR locally
Related issues
Questions to ask yourself