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

[ci] Fix scheduled JDK matrix CI jobs #15623

Merged

Conversation

dliappis
Copy link
Contributor

Release notes

[rn:skip]

What does this PR do?

This commit fixes failed scheduled JDK matrix CI jobs, that can't access the default values for the OS and JDK from the input steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150 [^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c

Related issues

@dliappis dliappis added the bug label Nov 28, 2023
@dliappis dliappis self-assigned this Nov 28, 2023
@dliappis dliappis force-pushed the fix-logstash-jdk-matrix-scheduled-builds branch 2 times, most recently from 164fe7f to 6d07291 Compare November 28, 2023 12:27
This commit fixes failed scheduled JDK matrix CI jobs, that
can't access the default values for the OS and JDK from the input
steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150
[^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c
@dliappis dliappis force-pushed the fix-logstash-jdk-matrix-scheduled-builds branch from 6d07291 to 162ae8c Compare November 28, 2023 12:30
@dliappis
Copy link
Contributor Author

Because scheduled job testing is tricky, this is how this PR has been tested:

  1. Using 162ae8c (which is the final commit that we intend to merge) we confirm that manually triggering the Linux and Windows jobs, honor the defined defaults:

Linux: https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/55

image

Windows: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/41

image

  1. Using c540971 which commented out all the input steps, effectively mocking a scheduled run:

The PR is now ready for review. I've reverted c540971 and we are back to the state described in 1.

@dliappis dliappis marked this pull request as ready for review November 28, 2023 13:06
@dliappis dliappis requested a review from alexsapran November 28, 2023 13:06
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@@ -1,5 +1,9 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/buildkite/pipeline-schema/main/schema.json

env:
DEFAULT_MATRIX_OS: "ubuntu-2204"
DEFAULT_MATRIX_JDK: "adoptiumjdk_17"
Copy link
Contributor Author

@dliappis dliappis Nov 28, 2023

Choose a reason for hiding this comment

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

we discussed offline with @alexsapran who pointed out that another alternative for this would be to define it as env vars in the schedule branch section. In that case, the env vars here will always override any values in the schedule.

I think I prefer things as they are here, though, because we won't have different defaults per branch and will not need to define them in catalog-info.yaml and the source of truth for the defaults remains in the pipeline file.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @dliappis

@dliappis dliappis merged commit f0019bf into elastic:main Nov 28, 2023
5 checks passed
@dliappis
Copy link
Contributor Author

@logstashmachine backport 8.11

@dliappis
Copy link
Contributor Author

@logstashmachine backport 7.17

github-actions bot pushed a commit that referenced this pull request Nov 28, 2023
This commit fixes failed scheduled JDK matrix CI jobs, that
can't access the default values for the OS and JDK from the input
steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150
[^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c

(cherry picked from commit f0019bf)
github-actions bot pushed a commit that referenced this pull request Nov 28, 2023
This commit fixes failed scheduled JDK matrix CI jobs, that
can't access the default values for the OS and JDK from the input
steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150
[^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c

(cherry picked from commit f0019bf)
dliappis added a commit to dliappis/logstash that referenced this pull request Nov 29, 2023
This commit fixes failed scheduled JDK matrix CI jobs, that
can't access the default values for the OS and JDK from the input
steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150
[^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c

(cherry picked from commit f0019bf)

Co-authored-by: Dimitrios Liappis <[email protected]>
jsvd pushed a commit to jsvd/logstash that referenced this pull request Dec 5, 2023
This commit fixes failed scheduled JDK matrix CI jobs, that
can't access the default values for the OS and JDK from the input
steps, as observed in [^1] and [^2].

[^1] https://buildkite.com/elastic/logstash-linux-jdk-matrix-pipeline/builds/53#018c1371-b760-4c28-9203-340c0a1df150
[^2]: https://buildkite.com/elastic/logstash-windows-jdk-matrix-pipeline/builds/35#018c1371-b72e-48b4-b707-ce103eb6039c

(cherry picked from commit f0019bf)

Co-authored-by: Dimitrios Liappis <[email protected]>
@roaksoax roaksoax added the ci label Dec 13, 2023
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.

4 participants