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

Fix/continuous delivery #834

Open
wants to merge 10 commits into
base: staging
Choose a base branch
from
Open

Fix/continuous delivery #834

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cdad07b
added option to force execute all jobs to the delivery workflow
Ellen-Wittingen Mar 18, 2024
d2edcec
added instructions about problem in the readme
Ellen-Wittingen Mar 18, 2024
ae5a0b2
removed unneccesary braces
Ellen-Wittingen Mar 18, 2024
9d32385
change n/y to boolean
lodewiges Oct 29, 2024
7fbc05f
fixed comment
lodewiges Nov 9, 2024
ae9befd
fix part 2
lodewiges Nov 9, 2024
433f4b2
fix part 3
lodewiges Nov 9, 2024
1087289
Update continuous-delivery.yml
lodewiges Nov 12, 2024
dd37aa9
Merge branch 'staging' into fix/continuous-delivery
lodewiges Nov 12, 2024
d96386b
Update continuous-delivery.yml
lodewiges Nov 12, 2024
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
28 changes: 17 additions & 11 deletions .github/workflows/continuous-delivery.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,15 @@ on:
workflow_dispatch:
inputs:
merge:
description: Merge staging into master first? (y/N)
type: boolean
description: Merge staging into master first?
required: false
default: 'n'
default: false
ignore_metadata_diff:
type: boolean
description: Perform all jobs, regardless of whether there are actual changes?
required: false
default: false

concurrency:
group: cd-${{ github.ref_name }}
Expand Down Expand Up @@ -43,7 +49,7 @@ jobs:
INPUT_MERGE: ${{ github.event.inputs.merge }}
run: |
if [ "$GITHUB_REF_NAME" = 'master' ]; then
if [ "${INPUT_MERGE,,}" = 'y' ]; then
if [ "$INPUT_MERGE" = true ]; then
DrumsnChocolate marked this conversation as resolved.
Show resolved Hide resolved
git fetch origin staging
if ! git diff origin/master origin/staging --exit-code; then
echo 'has_diff=true' >> "$GITHUB_OUTPUT"
Expand All @@ -61,7 +67,7 @@ jobs:
name: Merge
runs-on: ubuntu-latest
needs: metadata
if: github.event.inputs.merge == 'y'
if: github.event.inputs.merge == true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if: github.event.inputs.merge == true
if: github.event.inputs.merge

can't we do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

What i found online is that ==true is better. source: https://stackoverflow.com/questions/2953646/how-can-i-declare-and-use-boolean-variables-in-a-shell-script/21210966#21210966

Copy link
Contributor

@lodewiges lodewiges Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we need to decide what would be better, and implement that everywhere, i forgot it in one line.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are talking about shell expressions, I'm talking about GitHub expressions: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions

They are separate concepts.

I've doublechecked why I suggested this. I come to a slightly more nuanced conclusion now, based on https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#onworkflow_dispatchinputs
I think the conclusion is we can use inputs.merge and we don't need to check against a string, but when we use github.event.inputs.merge we do need to check against a string, because:

The workflow will also receive the inputs in the github.event.inputs context. The information in the inputs context and github.event.inputs context is identical except that the inputs context preserves Boolean values as Booleans instead of converting them to strings. The choice type resolves to a string and is a single selectable option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think implemented it correctly can you check?

outputs:
sha: ${{ steps.get_sha.outputs.sha }}
steps:
Expand All @@ -79,11 +85,11 @@ jobs:
fi

- name: Checkout code
if: fromJSON(needs.metadata.outputs.has_diff)
if: fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff
uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2

- name: Run merge
if: fromJSON(needs.metadata.outputs.has_diff)
if: fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff
uses: devmasx/merge-branch@854d3ac71ed1e9deb668e0074781b81fdd6e771f # tag=v1.4.0
with:
type: now
Expand All @@ -93,23 +99,23 @@ jobs:

- name: Get merge commit SHA
id: get_sha
if: fromJSON(needs.metadata.outputs.has_diff)
if: fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff
run: |
git fetch origin master
echo 'sha='"$(git rev-parse origin/master)" >> "$GITHUB_OUTPUT"

continuous_integration:
name: Continuous Integration
needs: [metadata, merge]
if: fromJSON(needs.metadata.outputs.has_diff)
if: fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff
uses: csvalpha/amber-ui/.github/workflows/continuous-integration.yml@staging
with:
sha: ${{ needs.merge.outputs.sha }}

publish_image:
name: Publish Image
needs: [metadata, merge]
if: fromJSON(needs.metadata.outputs.has_diff)
if: fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff
uses: csvalpha/amber-ui/.github/workflows/publish-image.yml@staging
with:
sha: ${{ needs.merge.outputs.sha }}
Expand All @@ -122,8 +128,8 @@ jobs:
needs: [metadata, merge, continuous_integration, publish_image]
if: |
(github.ref_name == 'staging' || github.ref_name == 'master') && ((github.ref_name == 'master' &&
github.event.inputs.merge == 'y' && fromJSON(needs.metadata.outputs.has_diff) && success()) ||
((github.event.inputs.merge != 'y' || !fromJSON(needs.metadata.outputs.has_diff)) && !cancelled()))
github.event.inputs.merge && (fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff) && success()) ||
((!github.event.inputs.merge || !(fromJSON(needs.metadata.outputs.has_diff) || github.event.inputs.ignore_metadata_diff)) && !cancelled()))
steps:
- name: Get environment URL
id: get_url
Expand Down
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ Deployments are done using GitHub Actions. To deploy a branch, follow the follow
- Choose a branch and if you want to merge the changes on the staging branch into the master branch (only possible when the branch chosen in previous step is master).
- Click the green button "Run workflow".

*Note: If the workflow fails with an error message about blobs along the lines of the following: `buildx failed with: ERROR: failed to solve: [...] blob [...]`: remove all recent cache files for staging on the [Caches page](https://github.com/csvalpha/amber-ui/actions/caches) and try again. You might have to turn on the option `Perform all jobs, regardless of whether there are actual changes?` to force the workflow to re-execute all its jobs.*
DrumsnChocolate marked this conversation as resolved.
Show resolved Hide resolved

## Further Reading / Useful Links

- [ember.js](https://emberjs.com/)
Expand Down
Loading