-
Notifications
You must be signed in to change notification settings - Fork 36
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
release: automate regular SAR #758
Conversation
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ needs.release.outputs.tag }} |
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.
avoid surprises and use the tag that has been created earlier
sha: context.sha | ||
}) | ||
|
||
regular-sar: |
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.
Run this job only if there is a new tag and in a separate job so we can see if things work as expected.
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.
I agree with this.
We also only upload dependencies to the bucket when this workflow succeeds:
on: | |
workflow_run: | |
workflows: [release] | |
types: | |
- completed |
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, there is a corner case in
- uses: actions/checkout@v4 |
2 or more
commits happened in a row in share/version.py
while upload-dependencies.yml
has not been executed yet.
We can do a follow-up and see if that's a genuine reason
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.
I agree with you. There is also another problem: if a commit happens and the tag is not updated (so no version upgrade). I don't see why a commit would not cause a version upgrade, but we do not check that.
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.
Good catch!
|
||
- name: Build and package | ||
run: | | ||
.internal/aws/scripts/dist.sh \ |
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.
It runs the same command in #279 (comment)
Co-authored-by: Jan Calanog <[email protected]>
I think
"
#805 should help |
Yeah! You've been faster than me! Closing https://github.com/elastic/elastic-serverless-forwarder/pull/806/files |
What does this PR do?
.internal/aws/scripts/dist.sh
aws-actions/setup-sam
to install SAM cli in the worker.Why is it important?
Remove the manual step to deploy in `elastic-observability-prod
Follow-ups
To support GovCloud - However, AFAIK we don't admin that AWS account and we are using OIDC/Keyless for simplicity.
Questions
internal/aws/scripts/dist.sh
elastic-observability
AWS account to validate if this works in my forked repository.Checklist
CHANGELOG.md
Author's Checklist
How to test this PR locally
Related issues
Part of #279
Use cases
Screenshots
Logs