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.

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

Suggestion : Cancel Previous Runs #5

Open
obrassard opened this issue Jun 30, 2020 · 24 comments · Fixed by #7
Open

Suggestion : Cancel Previous Runs #5

obrassard opened this issue Jun 30, 2020 · 24 comments · Fixed by #7

Comments

@obrassard
Copy link
Owner

We should find a way to cancel a running workflow without locking the dokku app

name: 'Deploy to my Dokku instance'

on:
  push:
    branches:
    - master

jobs:
  deploy:
    runs-on: ubuntu-latest
    steps:
+    - name: Cancel Previous Runs
+      uses: styfle/cancel-workflow-action@0.4.0
+      with:
+        access_token: ${{ github.token }}
    
    - name: Cloning repo # This step is required
      uses: actions/checkout@v2

    # This step is required or you might get an error from Dokku
    - run: git fetch --prune --unshallow 
  
    - name: Push to dokku
      uses: obrassard/action-dokku-deploy@v1.0.0
      with:
       dokku_repo: 'ssh://dokku@dokku.myhost.ca:22/appname'
       ssh_key: ${{ secrets.SSH_KEY }}
       deploy_branch: 'develop'

Suggested solution by @julienma :

I'm wondering if we could systematically unlock the deploy whenever the workflow runs?
See http://dokku.viewdocs.io/dokku/deployment/application-management/#unlocking-app-deploys

@obrassard
Copy link
Owner Author

obrassard commented Jun 30, 2020

@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 !

@julienma
Copy link
Contributor

julienma commented Jul 1, 2020

@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:

  1. Forget about this and do nothing. This means that every push will trigger a deploy, and if there are successive pushes then one may have to wait for a few more minutes to get the latest commit actually deployed. Not optimized, but everything works as expected.
  2. Just add the 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.
    image
  3. Or we could create our own 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...

@obrassard
Copy link
Owner Author

obrassard commented Jul 1, 2020

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?

@obrassard
Copy link
Owner Author

obrassard commented Jul 1, 2020

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.

Capture d’écran, le 2020-07-01 à 10 24 36

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.

@obrassard
Copy link
Owner Author

obrassard commented Jul 1, 2020

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:
Capture d’écran, le 2020-07-01 à 10 38 48

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

@julienma
Copy link
Contributor

julienma commented Jul 1, 2020

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 ;)

@obrassard
Copy link
Owner Author

obrassard commented Jul 1, 2020

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)

@obrassard
Copy link
Owner Author

obrassard commented Jul 2, 2020

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) 😀

@julienma
Copy link
Contributor

julienma commented Jul 2, 2020

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.

@obrassard
Copy link
Owner Author

obrassard commented Jul 2, 2020

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).

Capture d’écran, le 2020-07-02 à 14 26 57

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.

@julienma
Copy link
Contributor

julienma commented Jul 3, 2020

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?

@obrassard
Copy link
Owner Author

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 ?

@obrassard
Copy link
Owner Author

Else we could fork cancel-workflow-action and add the delay on cancellation 😄

@julienma
Copy link
Contributor

julienma commented Jul 3, 2020

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.

@styfle
Copy link

styfle commented Jul 3, 2020

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.

@obrassard
Copy link
Owner Author

obrassard commented Jul 4, 2020

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.

@styfle
Copy link

styfle commented Jul 8, 2020

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 👍

@miguelcobain
Copy link

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. 👏

@miguelcobain
Copy link

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.

dokku/dokku#3697

@obrassard
Copy link
Owner Author

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.

@miguelcobain
Copy link

@obrassard yes, I'll try to work on it.
Thank you for clarifying. 👍

@obrassard
Copy link
Owner Author

@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 : styfle/cancel-workflow-action#26
Maybe you should check with styfle what's the status of that before starting 😉

@obrassard
Copy link
Owner Author

obrassard commented Oct 9, 2020

For your information, I just came across this issue : styfle/cancel-workflow-action#26

@miguelcobain Someone is working on that actually.

@obrassard
Copy link
Owner Author

Well, finally this issue is not resolved! @miguelcobain are you still interested working on that ?

bobwhitelock added a commit to bobwhitelock/todotxt-ui that referenced this issue Sep 28, 2021
Resolves #114.

However this probably still won't work perfectly, as seems to be no easy
way to tell Dokku to cancel an in-progress deploy, but this should at
least ensure lots of deploy actions not in progress/failing at once,
blocking the latest deploy from happening.

Related issue: obrassard/action-dokku-push#5.
Repository owner deleted a comment from tim-tepia Jan 1, 2024
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 a pull request may close this issue.

4 participants