Skip to content

Conversation

donoghuc
Copy link
Contributor

@donoghuc donoghuc commented Aug 6, 2025

Previously logstash versions were tracked in the logstash repo and curled during
CI steps. This commit moves the source of truth to the .ci repo where the
consumer of that information lives. This also changes the format from json to
yaml to allow commenting in the file.

Previously logstash versions were tracked in the logstash repo and curled during
CI steps. This commit moves the source of truth to the `.ci` repo where the
consumer of that information lives. This also changes the format from json to
yaml to allow commenting in the file.
Temporarily include logstash-versions until logstash-plugins/.ci#77
is merged.
@@ -0,0 +1,16 @@
# For all active streams track the latest 2 releases
Copy link
Contributor

Choose a reason for hiding this comment

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

As I understand we will remove this file once we land logstash-plugins/.ci#77, right?

@@ -34,26 +34,22 @@ fi
# The ELASTIC_STACK_VERSION may be an alias, save the original before translating it
Copy link
Contributor

Choose a reason for hiding this comment

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

(flaky memory) Isn't this docker-setup.sh same with .ci one? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its slightly different. The pattern is that we download only the files in the upstream .ci repository that do not exist in the child plugin repo. So in this case, given we have a slightly different docker-setup.sh we use the one in this repo, not the upstream.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this task but when I took a look at the difference between es-output .ci/docker-setup.sh and central .ci/docker-setup.sh, the only difference (assuming we land this PR) I found is if [ "$INTEGRATION" == "true" ]; then.. (below) at the end.

if [ "$INTEGRATION" == "true" ]; then
      pull_docker_snapshot "elasticsearch" $ELASTIC_STACK_VERSION_ALIAS
    fi

I was curious if we move this logic to the central .ci, that would allow us removing this repo .ci/docker-setup.sh as well. Just want DRY if possible. We can follow up later with cleaning processes if it makes to you too.

@donoghuc
Copy link
Contributor Author

donoghuc commented Aug 6, 2025

logstash-plugins/.ci#79 should fix CI

Copy link
Contributor

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -34,26 +34,22 @@ fi
# The ELASTIC_STACK_VERSION may be an alias, save the original before translating it
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this task but when I took a look at the difference between es-output .ci/docker-setup.sh and central .ci/docker-setup.sh, the only difference (assuming we land this PR) I found is if [ "$INTEGRATION" == "true" ]; then.. (below) at the end.

if [ "$INTEGRATION" == "true" ]; then
      pull_docker_snapshot "elasticsearch" $ELASTIC_STACK_VERSION_ALIAS
    fi

I was curious if we move this logic to the central .ci, that would allow us removing this repo .ci/docker-setup.sh as well. Just want DRY if possible. We can follow up later with cleaning processes if it makes to you too.

@donoghuc donoghuc closed this Aug 7, 2025
@donoghuc donoghuc reopened this Aug 7, 2025
@donoghuc donoghuc merged commit 7618e9b into logstash-plugins:main Aug 7, 2025
4 checks passed
@donoghuc
Copy link
Contributor Author

donoghuc commented Aug 7, 2025

Thanks @mashhurs for the suggestion. I filed https://github.com/elastic/ingest-dev/issues/5960 to track that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants