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 packetbeat pipeline #37825

Merged
merged 40 commits into from
Feb 8, 2024
Merged

Conversation

sharbuz
Copy link
Contributor

@sharbuz sharbuz commented Feb 1, 2024

What is the problem this PR solves?

Jenkins->Buildkite pipelines migration

How does this PR solve the problem?

Migrates the packetbeat pipeline
https://buildkite.com/elastic/beats-packetbeat/builds?branch=sharbuz%3Amigrate-packetbeat-pipeline

Example of the pipeline:
https://buildkite.com/elastic/beats-packetbeat/builds/1081
Examples of dependencies:
https://buildkite.com/elastic/beats-metricbeat/builds/2310
https://buildkite.com/elastic/beats-libbeat/builds/1083

Related issues

https://github.com/elastic/ingest-dev/issues/1693

@sharbuz sharbuz added Packetbeat macOS Enable builds in the CI for darwin testing labels Feb 1, 2024
@sharbuz sharbuz self-assigned this Feb 1, 2024
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Feb 1, 2024
@botelastic
Copy link

botelastic bot commented Feb 1, 2024

This pull request doesn't have a Team:<team> label.

@sharbuz sharbuz added backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.12.0 Automated backport with mergify labels Feb 1, 2024
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Duration: 15 min 32 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Collaborator

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Duration: 13 min 10 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 1, 2024

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Duration: 73 min 42 sec

Pipeline error 1

This error is likely related to the pipeline itself. Click here
and then you will see the error (either incorrect syntax or an invalid configuration).

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@sharbuz sharbuz added the arm Enable builds in the CI for ARM testing label Feb 2, 2024
Copy link
Contributor

mergify bot commented Feb 2, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b migrate-packetbeat-pipeline upstream/migrate-packetbeat-pipeline
git merge upstream/main
git push upstream migrate-packetbeat-pipeline

Copy link
Contributor

mergify bot commented Feb 5, 2024

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b migrate-packetbeat-pipeline upstream/migrate-packetbeat-pipeline
git merge upstream/main
git push upstream migrate-packetbeat-pipeline

Comment on lines 89 to 90
echo "The conditions don't match to requirements for generating pipeline steps."
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

what's the reason for exit 1? I'd expect it should do nothing

Copy link
Contributor Author

@sharbuz sharbuz Feb 8, 2024

Choose a reason for hiding this comment

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

If the conditions don't meet the requirements the $pipelineName won't be created and the commands cat $pipelineName and buildkite-agent pipeline upload $pipelineName will return the error File not found. I used the exit 1:
to avoid additional conditions for the commands above
to avoid creating an empty file
to speed up the script if the changes don't meet the requirements
Probably, in this case, I could use exit 0, but I just wanted to highlight the empty pipeline, and maybe I just need to add buildkite-agent annotate option here. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

If the conditions don't meet the requirements the $pipelineName won't be created and the commands cat $pipelineName and buildkite-agent pipeline upload $pipelineName will return the error File not found.

I understand the rationale for reporting an error if cat or similar commands failed.

I understand the reason for filtering the filters defined in https://github.com/elastic/beats/blob/main/.buildkite/pull-requests.json.

However, I cannot understand the rationale for using a condition for the mandatory stages in the packetbeat pipeline for something other than doc changes.

Any PR touching the docs should not run the mandatory or any other stages unless a GitHub comment or similar UI interactions are used.

Is there any other condition for the mandatory stages that must be considered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The condition related to the DOCS is contained in the main pipeline and it will be implemented a bit later. Right now we have to see all pipelines are GREEN whenever we change something to understand how current changes affect our previous job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the reason for exit 1? I'd expect it should do nothing

BTW, I've changed to exit 0, in this case, if nothing changed - it's fine and should be GREEN, I think.

@sharbuz sharbuz requested a review from v1v February 8, 2024 06:59
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.

Basically LGTM from me.
I left an ask for better handling of when we run buildkite-agent ... in common.sh to make manual reproductions easier.

@@ -10,34 +10,45 @@ GITHUB_PR_TRIGGER_COMMENT=${GITHUB_PR_TRIGGER_COMMENT:-""}
ONLY_DOCS=${ONLY_DOCS:-"true"}
runLibbeat="$(buildkite-agent meta-data get runLibbeat --default ${runLibbeat:-"false"})"
runMetricbeat="$(buildkite-agent meta-data get runMetricbeat --default ${runMetricbeat:-"false"})"
runPacketbeat="$(buildkite-agent meta-data get runPacketbeat --default ${runPacketbeat:-"false"})"
Copy link
Contributor

Choose a reason for hiding this comment

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

A little unrelated to this PR, but what we are doing here makes manual reproductions difficult (i.e. this command fails when run manually). Could we improve the conditional such that buildkite-agent meta-data get is run only if the env var runPackagebeat is undefined or not false? (applies to all declarations here).

For context: https://elastic.slack.com/archives/C0D8SUKB2/p1707380999484699?thread_ts=1707312641.347669&cid=C0D8SUKB2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This option is for the future main pipeline, where we will define the steps of the pipeline...
We have a similar variable in the main pipeline - runAllStages

@oakrizan oakrizan mentioned this pull request Feb 8, 2024
6 tasks
@dliappis dliappis self-requested a review February 8, 2024 17:18
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

@sharbuz sharbuz enabled auto-merge (squash) February 8, 2024 17:19
@jlind23 jlind23 disabled auto-merge February 8, 2024 17:42
@jlind23 jlind23 merged commit e4756b2 into elastic:main Feb 8, 2024
27 of 34 checks passed
mergify bot pushed a commit that referenced this pull request Feb 8, 2024
mergify bot pushed a commit that referenced this pull request Feb 8, 2024
@elasticmachine
Copy link
Collaborator

💔 Build Failed

Failed CI Steps

History

cc @sharbuz

sharbuz added a commit that referenced this pull request Feb 8, 2024
(cherry picked from commit e4756b2)

Co-authored-by: sharbuz <[email protected]>
sharbuz added a commit that referenced this pull request Feb 8, 2024
(cherry picked from commit e4756b2)

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 libbeat macOS Enable builds in the CI for darwin testing Metricbeat Metricbeat needs_team Indicates that the issue/PR needs a Team:* label Packetbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants