Open obrassard opened 4 years ago
@julienma I suggest we try to add dokku apps:unlock <appname>
as a step directly in the action's entrypoint.sh
script and validate that it doesn't break the workflows that currently depends on this action.
This would allow users to add the cancel-workflow-action
to their workflows without concern !
@obrassard actually I'm not sure this is a good idea to always unlock deploy when this action runs. There might be genuine reason for the deploy to be locked, e.g. if another deploy has been requested manually, or if there are other things happening on dokku's side (e.g. rebuild, update, etc.).
I see 3 options:
cancel-workflow-action
and do nothing about the locked deploy. This means that successive builds may fail because of the deploy lock. Not great, but jobs can be manually re-run, so not blocking either. At least GHActions quota is optimized, as "outdated" builds can be canceled.
cancel-dokku-workflow-action
which would cancel both the workflow action AND the deploy lock only if there's queued/ongoing runs. Ideally we would want to know if the deploy lock has been set by a previous run of the workflow (vs by any other unrelated dokku action), but I think we can't.I'm not really sure of the implications of canceling the deploy lock in any case...
Your third option (creating our own cancel action) seems like a good compromise since the force unlock will only be triggered if there is currently a build running. This reduce the chances of errors and I think we can assume that if a build is running it is likely that an update or or other locking process isn't running on Dokku's side.
Else maybe we could include those features in this action by adding optional parameters that would let users choose if they want to cancel running builds and then if they want to force unlock their app before deployments in this case. What do you think of that?
I've made a test with one of my dokku apps that uses this action. I created a dummy commit, waited for the action to start, and then committed again and I manually cancelled the last running build. The second build then failed because of a deploy lock.
This leads me to believe that we will have no choice but to somehow unlock the app deployment if we want next builds to work.
I am also wondering if it would be possible to add a "post job cleanup" step to our action that would call dokku unlock
whenever the build is canceled.
This is an example of "post job" in github's action-checkout
that executed even if I cancelled the build:
Update : it seems to be possible with the post-entrypoint
setting, however we will need to confirm that this step is run even if an action is cancelled
I am also wondering if it would be possible to add a "post job cleanup" step to our action that would call dokku unlock whenever the build is canceled.
Oooh this is a great idea!
I won't have time to help with this now, but I'll happily comment on your experiments ;)
Thanks ! I am too limited in time, however i'll try to implement this in the near future.
I've opened a new topic on the github community forum to see if I can get more information about this post-entrypoint
hook.
For now I will create a release with your changes (#4)
FYI @julienma I received a pretty neat answer to my question and I confirm that post-entrypoints will always run even if a build is cancelled! Moreover it is possible to run the post script only on cancellation!
Knowing this, we could simply add a post-script to the action so we can unlock the app on cancellation and then add ˋstyfle/cancel-workflow-actionˋ to the suggested workflow (no need to develop a new cancel workflow action in this case) 😀
Ha, that’s neat!
If I understand correctly, you can embed a post
command right into this action, so no need to manually add it to our workflow?
If so, that seems perfect.
Salut @julienma je viens de remarquer que vous êtes en France !! C'est drôle que nous parlons tous les deux français, mais que nous nous écrivions en anglais depuis le début ! On peut continuer ce thread en français si vous le voulez !
As you may have seen, I created a PR (#7) with the modifications we discussed and it works quite well! When we manually cancel a build, everything works as expected and the application is indeed unlocked on Dokku. Also, if the build is not cancelled the post-entrypoint
step is not executed.
However, sometimes there is an issue when used alongside with styfle/cancel-workflow-action
. Let's assume that a build A is in progress and a new commit triggers a build B. In this case, cancel-workflow-action
in the build B will correctly cancel build A and the application will be unlocked. However, it can happen by the time the "build A cancellation request" is sent (and the dokku apps:unlock
is applied), that the dokku push
step in B has already started while the app is not yet unlocked, so dokku rejects the commit (and the build fails).
I don't know if there's anything we can do about it. In any case, it is certain that "Rerun all jobs" will work afterwards, since the app will be unlocked in build A. But for sure it is less convenient to manually re-run the build.
Salut Olivier :) Tu es québecois ? Le français me va très bien, mais ce sera sans doute plus facile pour d'autres de collaborer si on continue en anglais ?
Cependant, il peut arriver entre le temps où la requête d'annulation du build A est envoyée (et que le dokku apps:unlock soit appliqué), que l'étape dokku push dans B ait déjà débutée alors que l'app n'est pas encore dévérouillée, et donc dokku rejette le commit (et le build échoue).
Weird, I'd have thought that canceling the job and unlocking would happen before the push. What about adding a slight delay right before the push, so it leaves a bit of room for the unlock to happen?
It could be done either in dokku-unlock.sh or in the workflow file:
jobs:
deploy:
runs-on: ubuntu-latest
steps:
- name: Cancel Previous Runs # Optional step
uses: styfle/cancel-workflow-action@0.4.0
with:
access_token: ${{ github.token }}
- name: Cloning repo # This step is required
uses: actions/checkout@v2
with:
fetch-depth: 0 # This is required or you might get an error from Dokku
+ - run: echo 'Giving some time for apps:unlock to happen...'; sleep 5;
- name: Push to dokku
uses: obrassard/action-dokku-deploy@v1.0.3
with:
dokku_repo: 'ssh://dokku@dokku.myhost.ca:22/appname'
ssh_key: ${{ secrets.SSH_KEY }}
deploy_branch: 'develop'
WDYT?
Weird, I'd have thought that canceling the job and unlocking would happen before the push. What about adding a slight delay right before the push, so it leaves a bit of room for the unlock to happen?
Yeah the dokku unlock
is done in the first build (the one that is getting cancelled) and when the second build calls github api to cancel the first (with cancel-workflow-action
) there is a slight delay before the first build actually stop and the post-entrypoint action run. However during this time the dokku push may start in the second build. So, it couldn't be done in dokku-unlock.sh
. We could indeed add a delay in the workflow or in entrypoint.sh
, but it would slow down a bit every other builds (even when there is no cancellation).
Do you know if we can listen for an action result in order to add a step ? I mean if we could catch the result of styfle/cancel-workflow-action
we might be able to add a delay only if a previous build was cancelled by the action ?
Else we could fork cancel-workflow-action
and add the delay on cancellation 😄
but it would slow down a bit every other builds (even when there is no cancellation).
Yeah, bummer.
Do you know if we can listen for an action result in order to add a step ? I mean if we could catch the result of
styfle/cancel-workflow-action
we might be able to add a delay only if a previous build was cancelled by the action ?
No idea, but that'd be great. Guess we could ask @styfle?
Else we could fork
cancel-workflow-action
and add the delay on cancellation 😄
Agree, if the above doesn't work.
Maybe you're looking for Advanced Usage? https://github.com/styfle/cancel-workflow-action#advanced
Or maybe you need outputs? https://github.com/actions/hello-world-javascript-action/blob/ae53f59fd519c0006ceb494ecbfed5f05d4151cf/action.yml#L8-L10
I'm not sure I understand the problem.
Hello @styfle, I hope you're doing well !
To resume, action-dokku-push
aim to automatically deploy apps to a "dokku" remote server using git.
Our concern is that we would like to use your action cancel-workflow-action
alongside this action in order to stop concurrent deployments on new pushes, but we must find a way to add a "sleep" delay on previous jobs cancellations.
Indeed, when dokku deployments starts, the remote server add a "deployment lock" on apps to prevent concurrent deployments. To fix that we've implemented a post-entrypoint
step that runs exclusively when a running workflow is cancelled, in order to cancel and "unlock" ongoing deployments on the remote server (so we can deploy again). However, since there is a delay between the time when the previous build is cancelled by cancel-workflow-action
and the time where the app is actually unlocked on the remote server, we must wait a bit before starting a new deployment.
Since we don't want to add a useless delay when there is no cancellation (when no other builds are running), we must find a way to run the "delay step" only if cancel-workflow-action
actually cancelled a job. So, yes I guess, outputs could be the easiest solution if your action supports it! We could then add a "wait step" in our workflow with an if
parameter that verify this output.
So, yes I guess, outputs could be the easiest solution if your action supports it!
What output would you expect? If you submit a PR to styfle/cancel-workflow-action
I'll take a look 👍
I really liked the direction you were going! What is the current status and what is needed to achieve this successfully?
Is adding outputs and capturing that on #7 the remaining piece of the puzzle?
This would take this action to a new level. 👏
Posting this for reference. When implemented, I believe this would be the best way to cancel a dokku build as it also cleans up containers.
What is the current status and what is needed to achieve this successfully? Is adding outputs and capturing that on #7 the remaining piece of the puzzle?
Hello @miguelcobain I hope you're doing well!
Were you asking me this because you would like to work on this feature ? 😄
I kinda ran out of time in the last weeks but indeed, all that is missing is the possibility to perform an action (i.e. wait x seconds) when a previous build is cancelled by styfle/cancel-workflow-action
in order to let the dokku app unlock on the remote server.
In other words, we need to make a PR to styfle/cancel-workflow-action
to add an output that we can catch in a step of our workflow before running action-dokku-deploy
.
@obrassard yes, I'll try to work on it. Thank you for clarifying. 👍
@miguelcobain Nice ! I am glad to hear that ! Don't hesitate to ask me if you have any question !
For your information, I just came across this issue : https://github.com/styfle/cancel-workflow-action/issues/26 Maybe you should check with styfle what's the status of that before starting 😉
For your information, I just came across this issue : styfle/cancel-workflow-action#26
@miguelcobain Someone is working on that actually.
Well, finally this issue is not resolved! @miguelcobain are you still interested working on that ?
We should find a way to cancel a running workflow without locking the dokku app
Suggested solution by @julienma :