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.

Sign up for GitHub

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

Jump to bottom

[INFRA] Updating forked repo logic to build on every branch #6991

Merged
merged 7 commits into from
Jul 25, 2023
Merged

[INFRA] Updating forked repo logic to build on every branch #6991

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
88d59ed
[TEST] Removing default branch from PR jobs.
1Copenut Jul 25, 2023
f94661a
Removing target branch for forked repo PRs so it will run against all…
1Copenut Jul 25, 2023
9513161
Restoring the default branch declaration to jobs.
1Copenut Jul 25, 2023
92bc2bc
Changing GH branch variable to a Buildkite default.
1Copenut Jul 25, 2023
4f9fad3
Merge branch 'main' into infra/buildkite-pr-job-branches
1Copenut Jul 25, 2023
55f5abe
Fixed deploy docs job env vars not being passed correctly.
1Copenut Jul 25, 2023
b850f46
Merge branch 'infra/buildkite-pr-job-branches' of github.com:elastic/…
1Copenut Jul 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .buildkite/pipelines/pipeline_pull_request_test_and_deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ steps:
build:
env:
GIT_BRANCH: "${BUILDKITE_BRANCH}"
GIT_PULL_REQUEST_ID: "${GITHUB_PR_NUMBER}"
GIT_PULL_REQUEST_ID: "${BUILDKITE_PULL_REQUEST}"
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 move away from the feature branch target caused Buildkite to disregard GH-specific env variables. Dunno. But I did find this Buildkite default for branches, so updated both instances where I need it to create the comment link.

BUILDKITE_CI: "${BUILDKITE}"
2 changes: 1 addition & 1 deletion .buildkite/pipelines/pipeline_release_deploy_docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ steps:
- build:
env:
GIT_BRANCH: "${BUILDKITE_BRANCH}"
GIT_PULL_REQUEST_ID: "${GITHUB_PR_NUMBER}"
GIT_PULL_REQUEST_ID: "${BUILDKITE_PULL_REQUEST}"
BUILDKITE_CI: "${BUILDKITE}"
3 changes: 1 addition & 2 deletions .buildkite/pull-requests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
"trigger_comment_regex": "^(?:(?:buildkite\\W+)?(?:build|test)\\W+(?:this|it))",
"always_trigger_comment_regex": "^(?:(?:buildkite\\W+)?(?:build|test)\\W+(?:this|it))",
"set_commit_status": true,
"skip_ci_on_only_changed": ["^.github/", "^generator-eui/", "^wiki/"],
"target_branch": "feature/buildkite-migration"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this line to allow Buildkite (with Elastic org approval still) to kick off jobs against any branch. Kibana does not have this line in the pull-request.json file either. Feel it's right to remove it here.

Copy link
Member

Choose a reason for hiding this comment

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

Just curious, why was this not caught in #6965?

Copy link
Contributor Author

@1Copenut 1Copenut Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That branch was built on the upstream EUI repo, not a fork. I didn't notice it until I made the forked PR this morning to update Cypress.

Wrong branch I was looking at. I didn't put two and two together that #6965 was ignoring Buildkite because of this config change needed. I didn't notice it until I made the forked PR this morning to update Cypress, then talked through the solution with one of the platform engineers.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha - I'm more asking about the process used in #6965 that caused this instance to be missed. I would have assumed that searching/grepping for all instances of feature/buildkite-migration and either changing them to main or removing the logic completely would have been the approach to take. If it wasn't, I'd be curious why not / what other approach was used that caused this last reference to be missed.

"skip_ci_on_only_changed": ["^.github/", "^generator-eui/", "^wiki/"]
}
]
}
Loading