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

Forge site directory not deleted in teardown #114

Open
dev-anthonyrae opened this issue Aug 23, 2024 · 14 comments
Open

Forge site directory not deleted in teardown #114

dev-anthonyrae opened this issue Aug 23, 2024 · 14 comments

Comments

@dev-anthonyrae
Copy link

When the teardown command runs, the forge site is removed, but the actual folder on the server still exists.
Is this the expected behaviour?
Is it possible to automate the deletion of the the site directory on the server?

@mehrancodes
Copy link
Owner

@dev-anthonyrae The directory should get removed by the Forge in 10 seconds after the teardown commend finishes. There might be nothing Harbor can do about this.

@keybrdist
Copy link

keybrdist commented Nov 15, 2024

@mehrancodes - We are having the same issue, so I did some testing.

  1. The issue is only happening when Harbor issues the teardown
  2. I tested directly with the Forge API (no issue found)
  3. I tested using the Forge SDK (no issue found)

Site files are successfully deleted using SDK and API but not when the delete() is issued via Harbor.

@mehrancodes
Copy link
Owner

@keybrdist, @dev-anthonyrae I just tested it, and it removes the folder on the server. Maybe I can try it again with your Harbor .env public keys?

@keybrdist
Copy link

@mehrancodes I'll try running harbor locally to do some testing. Is it just a matter of setting the env vars and then running ./vendor/bin/harbor teardown ?

@mehrancodes
Copy link
Owner

@mehrancodes I'll try running harbor locally to do some testing. Is it just a matter of setting the env vars and then running ./vendor/bin/harbor teardown ?

@keybrdist, You can do that by running php harbor teardown. Same as when you want to provision a site

It’s built on Laravel Zero, but used for console apps. Let me know if you need any more help!

@keybrdist
Copy link

keybrdist commented Nov 22, 2024

I have run some tests..

tldr; I have to run harbor teardown twice to delete the site directory. The first time I run harbor teardown, the site and database is deleted and disappears from the forge UI; however the directory still remains. When I run it the 2nd time, the site directory is finally deleted.


I setup a base project with the following:

composer.json

{
    "require": {
        "mehrancodes/laravel-harbor": "^1.0"
    }
}

A basic teardown.sh script:

#!/bin/bash

set -e
export FORGE_GIT_BRANCH=xxxxxxx
export FORGE_SERVER=1234
export FORGE_DB_NAME=pr_1128
export SUBDOMAIN_NAME=pr-1128
export FORGE_DOMAIN=tst.xxxxxx.com
export FORGE_GIT_REPOSITORY=xxxxx/xxxxx
export FORGE_TOKEN=eyJ0.....

./vendor/bin/harbor teardown

Output:

➜ ./teardown.sh                    
  INFO    Start finding the server.
  INFO    Finding the associated site.
  INFO    Removing scheduled command.
  INFO    Removing database with user.
  SUCCESS    Environment teardown successful! All provisioned resources have been removed.
(base) 

➜ ./teardown.sh                    
  INFO    Start finding the server.
  INFO    Finding the associated site.
  SUCCESS    Environment teardown successful! All provisioned resources have been removed.
(base) 

➜ ./teardown.sh
  INFO    Start finding the server.
  INFO    Finding the associated site.
  FAIL    ---> Site not found.
(base) 

^^ notice on the 3rd attempt it finally says "Site not found". I had to run it twice to remove all resources.

@keybrdist
Copy link

keybrdist commented Nov 22, 2024

UPDATE: I added sleep(30); to DestroySite.php and it successfully deleted everything including site directory on the first attempt.

I suspect the delete operation is being called before the other operations are actually complete, and that the Forge API operations are asyncronous and have not completed before DestroySite is called.

class DestroySite
{
    use Outputifier;

    public function __invoke(ForgeService $service, Closure $next)
    {
        sleep(30);
        $service->site->delete();

        return $next($service);
    }
}

Note: I think the above is a good test but perhaps it would be better if you add something more dynamic to make sure previous operations in the pipelines have been been completed before DestroySite is invoked.

@keybrdist
Copy link

@keybrdist, @dev-anthonyrae I just tested it, and it removes the folder on the server. Maybe I can try it again with your Harbor .env public keys?

btw, regarding your testing -- This might only be affecting deployments that have other resources. For example.. all my failed tests were launched with FORGE_JOB_SCHEDULER=true. I'm guessing that DestroySite is being called before RemoveTaskScheduler is completed. hth

@keybrdist
Copy link

@mehrancodes sorry to tag you. Just wanted to make sure you saw my latest comments on this matter.

@olivernybroe
Copy link

Hello 👋

When deleting a site in Forge, Forge will automatically delete all related jobs and daemons.
So the RemoveInertiaSupport and RemoveTaskScheduler could be removed here as Forge will automatically do this.

It will require however that enabling inertia is done via the endpoint for enabling Inertia, so Forge knows that the daemon is related to that site.
https://forge.laravel.com/api-documentation#enablecreate-inertia-daemon

The same is also true for the deploy key, when deleting a site, Forge will delete the deploy key from Forge also (not the source control providers).

@mehrancodes
Copy link
Owner

@olivernybroe, @keybrdist, Thanks for following up! Everything's working great without the RemoveTaskScheduler. I've opened a PR to remove it since we don't need it anymore: #120.

Let's see if it works smoothly now without RemoveTaskScheduler :)

Feel free to open a PR for RemoveInertiaSupport if you'd like to tackle it this week.

@olivernybroe
Copy link

@mehrancodes thanks for fixing it, we will report back to the user that reported having this issue :)

Forgot to put disclaimer that I work for Forge 😅

@keybrdist
Copy link

@olivernybroe I think that user is me! 🙏

@dev-anthonyrae
Copy link
Author

Thanks everyone - Will need to update our app with the changes and check.

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

No branches or pull requests

4 participants