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.
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
Migrate xpack packetbeat #38135
Migrate xpack packetbeat #38135
Changes from 59 commits
2b5dd7b
cf44101
e432525
0783eee
2a1de26
7e55863
cf55e5b
def5673
85bfec8
5af9510
90bec63
bd038c2
77b29b2
5b41d3b
2751e38
88800c8
4805ccb
e242568
d41b68d
5319753
32229ce
a09803e
271c788
cd2282d
8eba9dc
1781fc0
c12fe5a
dd1e93e
d0f433b
751403b
39caa7b
2c4eb33
5696ea2
b6347ec
c3f04b3
420cd1b
2711e89
dfe71d0
91e7176
d48fd20
ebf072a
577a435
7c9fcd8
3de4774
fc5d80a
86bf174
fbc0703
a158886
781181b
e202bb7
67ff799
2a1eb07
d0d786e
74a4d1f
70bf4de
426014f
1a839aa
5543e57
554c3a5
d02580b
31358c9
b23146a
2954618
e2e12d1
37b956a
9585c26
5aa00de
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The name of the file
.buildkite/scripts/win_unit_tests.ps1
is specific aboutunit_tests
while it also runsmage systemTest
.What do you think about this proposal that can avoid maintaining the
.buildkite/scripts/win_unit_tests.ps1
but the pipeline itself?rename the file for
buildkite/scripts/win_mage.ps1
, it's more generic and could accept different mage goals.testType
to be renamed formageGoals
and then simplify the script with the below snippet:Change the pipeline stages with:
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 agree with you that it could be done as you suggest. There is only one remark. We gonna decline to use Windows scripts soon (I hope in 1 week). If it's possible to avoid additional changes right now, let's do that. @v1v WDYT?
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.
Do you mean the file
.buildkite/scripts/win_unit_tests.ps1
will not be used anymore soon? I still think the name of the file is not correct for the nature of what it does, so.buildkite/scripts/win_tests.ps1
could be more meaningful.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.
Correct.
We are going to use our own BK Windows images and @dliappis is working on it.
When it's ready we are going to use command steps with 1-3 commands without using PowerShell scripts.
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.
Renaming this script requires a lot of changes in the already prepared dynamic pipelines.
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'll then wait for @dliappis 's feedback; if he has no concerns, I'm happy with this.
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 @v1v for the great feedback (as usual!) The custom Windows images are now ready, so I am good with letting this as is and get rid of many custom scripts and snippets, including the one discussed here, ASAP.
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.
@sharbuz if we haven't it yet, let's create a post migration ticket in the usual repo and link it to https://github.com/elastic/ingest-dev/issues/2794 to remind us that the conditional behavior here should be removed once Jenkins has been sunset (I realize that it should be possible to run the tests locally by devs too but I believe they have access to it already).
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.
done.
https://github.com/elastic/ingest-dev/issues/3006