-
Notifications
You must be signed in to change notification settings - Fork 3.5k
GH action for updating logstash version #18035
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
base: main
Are you sure you want to change the base?
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
This pull request does not have a backport label. Could you fix it @donoghuc? 🙏
|
Use of updatecli for dep bumps was based on this comment #17945 (comment) |
To test locally: install updatecli
generate config file:
run
|
410469e
to
2514eec
Compare
0c6def8
to
1292374
Compare
This was tested with #18050 and produced a PR with a version bump! Ready for review. |
956fd79
to
38c0131
Compare
.ci/updatecli/values.d/scm.yml
Outdated
# begin updatecli-compose policy values | ||
user: 'github-actions[bot]' | ||
email: '41898282+github-actions[bot]@users.noreply.github.com' | ||
# end updatecli-compose policy values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need those values, that's when using a shared policy with updatecli, in this case, you only added the .ci/updatecli/bump-logstash-version.yml
manifest.
# begin updatecli-compose policy values | |
user: 'github-actions[bot]' | |
email: '41898282+github-actions[bot]@users.noreply.github.com' | |
# end updatecli-compose policy values |
@@ -1,19 +1,34 @@ | |||
name: Stub GH action for devoping new workflows [STUB] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicates .github/workflows/bump-version.yml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as #18035 (comment)
pull_request: | ||
types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this event will not work with forked PRs, since the GH token will never have any elevated access but read-only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the idea here was that this "stub" action is already checked in with a pull_request trigger. I was hoping that by doing that and targeting that workflow file from a branch that pushed to the elastic master copy of logstash would be enough to actually give a valid token for raising a PR. It may not actually be necessary though. Either way I will be removing the "stub" before this is actually ready for review.
This commit adds a GH action for bumping the logstash version. It uses the updatescli based on a suggestion for how the beats team is doing file updates. There is a workflow_dispatch trigger that accepts a new logstash version. Both the versions.yml and lock file are updated. If it is a branch where we are not vendoring a lockfile that file is skipped without failing the workflow step.
use pattern suggested in code review
5118e44
to
83568e4
Compare
@@ -0,0 +1,47 @@ | |||
--- | |||
name: Update logstash lockfile | |||
pipelineid: "logstash/version-updates" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to use this updatecli manifest to target multiple branches, then use {{ requiredEnv "LOGSTASH_BRANCH" }}
in tghe pipelineid, that's the name of the base branch when a PR is created
pipelineid: "logstash/version-updates" | |
pipelineid: "logstash/version-updates-{{ requiredEnv "LOGSTASH_BRANCH" }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure open to this change, but i'm not quite following where this is used in PR branching. In #18163 it seems like the branch is updatecli_8.18_logstash/version-updates
. If somehow this was not auto-deleted on merged I could see there being a conflict (and thus probably not a PR raised).
Thanks @v1v for taking a look. I was working yesterday on figuring out how to implement the following using updatecli: For unreleased branches (for example So far i cant figure out how to express this in updatecli. Is that something you have an idea of how to do? |
I guess you can use the conditionals with sources and targets. The example below will search for the given sources:
lock_file_exists:
kind: shell
spec:
command: test -f file.lock
targets:
update_lock_file:
disablesourceinput: true
dependson:
- 'source#lock_file_exists'
kind: file
spec:
file: "file.lock"
content: "foo1"
update_versions.yml:
kind: file
disablesourceinput: true
spec:
file: "versions.yml"
content: "bar1" If
Otherwise:
|
|
@@ -0,0 +1,81 @@ | |||
--- | |||
name: Update logstash version files | |||
pipelineid: "logstash/version-updates" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you run this manifest targeting multiple branches, then the pipelineid
should be unique. I'd recommend using LOGSTASH_BRANCH
.
pipelineid: "logstash/version-updates" | |
pipelineid: "logstash/version-updates-{{ requiredEnv "LOGSTASH_BRANCH" }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
@@ -0,0 +1,31 @@ | |||
name: bump-logstash-version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, .github/workflows/bump-java-version.yml
and .github/workflows/bump-logstash.yml
do the same, don't they?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #18035 (comment)
I've reverted all the changes to that file. I'll deal with it separately.
.ci/updatecli/values.d/scm.yml
Outdated
enabled: true | ||
owner: elastic | ||
repository: logstash | ||
commitusingapi: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
: this is hardcoded in the updatecli manifest
commitusingapi: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
.ci/updatecli/values.d/scm.yml
Outdated
@@ -0,0 +1,5 @@ | |||
scm: | |||
enabled: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
: unused
enabled: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
on: | ||
workflow_dispatch: | ||
inputs: | ||
logstash_version: | ||
description: 'Logstash version' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think adding an example might be useful here?
description: 'Logstash version' | |
description: 'Logstash version (i.e: 1.2.3)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, added one
required: true | ||
type: string | ||
logstash_branch: | ||
description: 'Logstash branch' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description: 'Logstash branch' | |
description: 'Logstash branch (i.e: 8.19)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, added one
|
💚 Build Succeeded
History
|
This commit adds a GH action for bumping the logstash version. It uses the updatescli based on a suggestion for how the beats team is doing file updates. There is a workflow_dispatch trigger that accepts a new logstash version. Both the versions.yml and lock file are updated. If it is a branch where we are not vendoring a lockfile that file is skipped without failing the workflow step.
Implements https://github.com/elastic/ingest-dev/issues/5977