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

Add Primary Buildkite Pipeline Migration #571

Merged
merged 2 commits into from
Aug 31, 2023

Conversation

markjhoy
Copy link
Contributor

Migrates the primary pipeline from Jenkins to Buildkite

Checklists

Pre-Review Checklist

  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

@markjhoy markjhoy changed the title add primary pipeline migration Add Primary Buildkite Pipeline Migration Aug 30, 2023
Copy link

@acrewdson acrewdson left a comment

Choose a reason for hiding this comment

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

looks good, in addition to a couple nits, just a few questions:

  • the jenkins pipeline appears to do some artifact archiving here; we don't need that any longer?
  • also looks like maybe we had slack notifications setup (although not 100% sure they were enabled); confirming we no longer need this?

Comment on lines 11 to 22
- "./.buildkite/scripts/run_command.sh tests"
artifact_paths:
- "coverage/index.html"
- label: ":wrench: Linter"
commands:
- "./.buildkite/scripts/run_command.sh linter"
- label: ":package: Docker"
commands:
- "./.buildkite/scripts/run_command.sh docker"
- label: ":package: Packaging"
commands:
- "./.buildkite/scripts/run_command.sh packaging"

Choose a reason for hiding this comment

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

nit: are the leading ./s needed here?

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 don't think so... fixing...

agents:
provider: "gcp"
machineType: "n1-standard-8"
useVault: true

Choose a reason for hiding this comment

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

just checking: do we need this? not immediately seeing where we need a Vault token in this pipeline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call - let's find out ;) incoming commit

@markjhoy
Copy link
Contributor Author

  • the jenkins pipeline appears to do some artifact archiving here; we don't need that any longer?

The artifact_paths in the pipeline file will upload the artifacts that are the same from the Jenkins version (there's the coverage report in tests and the gemfile itself in packaging).

  • also looks like maybe we had slack notifications setup (although not 100% sure they were enabled); confirming we no longer need this?

This will be enabled automagically once this gets into main as we have the notifications set for for all pipelines that are under the enterprise-search team to flow into the alerting Slack channel.

@acrewdson
Copy link

The artifact_paths in the pipeline file will upload the artifacts that are the same from the Jenkins version (there's the coverage report in tests and the gemfile itself in packaging).

oh nice, totally missed that, thank you!

This will be enabled automagically once this gets into main as we have the notifications set for for all pipelines that are under the enterprise-search team to flow into the alerting Slack channel.

awesome!

@markjhoy markjhoy requested a review from acrewdson August 30, 2023 21:45
@markjhoy markjhoy enabled auto-merge (squash) August 30, 2023 21:47
@markjhoy markjhoy disabled auto-merge August 30, 2023 21:47
@markjhoy markjhoy enabled auto-merge (squash) August 30, 2023 21:54
@markjhoy markjhoy disabled auto-merge August 30, 2023 22:00
@markjhoy markjhoy enabled auto-merge (squash) August 30, 2023 22:00
@markjhoy markjhoy requested a review from a team August 30, 2023 22:01
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov left a comment

Choose a reason for hiding this comment

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

Good one!

@markjhoy markjhoy merged commit a393128 into main Aug 31, 2023
@markjhoy markjhoy deleted the markjhoy/add_main_buildkite_pipeline branch August 31, 2023 09:23
@github-actions
Copy link

💔 Failed to create backport PR(s)

Status Branch Result
8.11 The branch "8.11" is invalid or doesn't exist

To backport manually run:
backport --pr 571 --autoMerge --autoMergeMethod squash

Copy link

github-actions bot commented Feb 5, 2024

💔 Failed to create backport PR(s)

Status Branch Result
8.3 Commit could not be cherrypicked due to conflicts
8.11 Cherrypick failed because the selected commit (a393128) is empty. Did you already backport this commit?

To backport manually run:
backport --pr 571 --autoMerge --autoMergeMethod squash

artem-shelkovnikov pushed a commit that referenced this pull request Feb 5, 2024
* add primary pipeline migration

* remove leading ./ ; remove useVault

(cherry picked from commit a393128)

# Conflicts:
#	.buildkite/pipeline.yml
@artem-shelkovnikov
Copy link
Member

💚 All backports created successfully

Status Branch Result
8.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

artem-shelkovnikov added a commit that referenced this pull request Feb 5, 2024
* Add Primary Buildkite Pipeline Migration (#571)

* add primary pipeline migration

* remove leading ./ ; remove useVault

(cherry picked from commit a393128)

# Conflicts:
#	.buildkite/pipeline.yml

* Remove not needed CI steps

---------

Co-authored-by: Mark J. Hoy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants