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

Migrate xpack packetbeat #38135

Merged
merged 67 commits into from
Mar 14, 2024
Merged

Conversation

sharbuz
Copy link
Contributor

@sharbuz sharbuz commented Feb 26, 2024

What is the problem this PR solves?

Jenkins->Buildkite pipelines migration

Migrate x-pack packetbeat pipeline

Example of the pipelines:
beats-xpack-packetbeat
Examples of the affected pipelines:
beats-libbeat
beats-metricbeat
beats-packetbeat
beats-winlogbeat
beats-xpack-libbeat
beats-xpack-metricbeat

Related issues

https://github.com/elastic/ingest-dev/issues/1693
#38142 - related to commented-out Windows system tests

mage build unitTest
}
}
elseif ($testType -eq "systemtest") {
Copy link
Member

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 about unit_tests while it also runs mage 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 for mageGoals and then simplify the script with the below snippet:

if ($mageGoals -eq "systemTest") {
    google_cloud_auth
}

mage $mageGoals

Change the pipeline stages with:

# .buildkite/scripts/generate_xpack_packetbeat_pipeline.sh
      - label: ":windows: Windows Unit Tests - {{matrix.image}}"
        command: ".buildkite/scripts/win_mage.ps1 build unitTest"
     ...
      ##  TODO: uncomment when the issue https://github.com/elastic/beats/issues/38142 is solved
      # - label: ":windows: Windows 10 System Tests"
      #   key: "extended-win-10-system-tests"
      #.  command: ".buildkite/scripts/win_mage.ps1 systemTest"
# .buildkite/scripts/generate_xpack_ libbeat_pipeline.sh
      - label: ":windows: Windows 11 Unit Tests"
        command: ".buildkite/scripts/win_mage.ps1 -w reader/etw build goUnitTest"

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM thanks all for the great work and review discussions!

mage build unitTest
}
}
elseif ($testType -eq "systemtest") {
Copy link
Contributor

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.

@@ -172,6 +172,13 @@ func SystemTest(ctx context.Context) error {
return devtools.GoTest(ctx, args)
}

func getBucketName() string {
Copy link
Contributor

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sharbuz sharbuz enabled auto-merge (squash) March 13, 2024 14:13
@jlind23
Copy link
Collaborator

jlind23 commented Mar 13, 2024

Hey @nfritts @norrietaylor, your team approval is required here even though this is purely a CI change that we own.
Can we please get your thumbs up?

@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 14, 2024

@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 14, 2024

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @sharbuz

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @sharbuz

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @sharbuz

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @sharbuz

@jlind23 jlind23 disabled auto-merge March 14, 2024 08:04
@jlind23 jlind23 merged commit e0d5fe5 into elastic:main Mar 14, 2024
38 of 46 checks passed
@jlind23
Copy link
Collaborator

jlind23 commented Mar 14, 2024

Force merging this as it is blocked due to unrelated problems, no MacOS workers are available to execute the job

mergify bot pushed a commit that referenced this pull request Mar 14, 2024
* migrate xpack-metricbeat

---------

Co-authored-by: Victor Martinez <[email protected]>
Co-authored-by: Julien Lind <[email protected]>
(cherry picked from commit e0d5fe5)

# Conflicts:
#	.buildkite/scripts/setenv.sh
#	x-pack/packetbeat/magefile.go
mergify bot pushed a commit that referenced this pull request Mar 14, 2024
* migrate xpack-metricbeat

---------

Co-authored-by: Victor Martinez <[email protected]>
Co-authored-by: Julien Lind <[email protected]>
(cherry picked from commit e0d5fe5)
mergify bot pushed a commit that referenced this pull request Mar 14, 2024
* migrate xpack-metricbeat

---------

Co-authored-by: Victor Martinez <[email protected]>
Co-authored-by: Julien Lind <[email protected]>
(cherry picked from commit e0d5fe5)
sharbuz added a commit that referenced this pull request Mar 14, 2024
* migrate xpack-metricbeat

---------

Co-authored-by: Victor Martinez <[email protected]>
Co-authored-by: Julien Lind <[email protected]>
(cherry picked from commit e0d5fe5)

Co-authored-by: sharbuz <[email protected]>
sharbuz added a commit that referenced this pull request Mar 14, 2024
* migrate xpack-metricbeat

---------

Co-authored-by: Victor Martinez <[email protected]>
Co-authored-by: Julien Lind <[email protected]>
(cherry picked from commit e0d5fe5)

Co-authored-by: sharbuz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm Enable builds in the CI for ARM testing backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.12.0 Automated backport with mergify backport-v8.13.0 Automated backport with mergify macOS Enable builds in the CI for darwin testing needs_team Indicates that the issue/PR needs a Team:* label Packetbeat x-pack Issues and pull requests for X-Pack features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants