feat: Adds abra app move #605

Closed
p4u1 wants to merge 1 commits from p4u1/abra:abra-app-move into main
Member

See #586 for discussion

This is a very wip implementation to move an app from one server to another.

goals:

  • works consistently
  • it should be easly revertable if something does go wrong

non goals:

  • zero downtime deployment

implementation:

  • access running containers to get secerts and volume info
  • undeploy app on old server
  • create secrets on new server
  • move volumes to new server
  • move env file to new server
  • (note that it does not deploy the app on the new server, instead you need to run abra app deploy, (we could maybe do that in the future)

Usage:
abra app move gitea.example.com serverb.example.com
abra app deploy gitea.example.com

See https://git.coopcloud.tech/toolshed/abra/issues/586 for discussion This is a very wip implementation to move an app from one server to another. goals: - works consistently - it should be easly revertable if something does go wrong non goals: - zero downtime deployment implementation: - access running containers to get secerts and volume info - undeploy app on old server - create secrets on new server - move volumes to new server - move env file to new server - (note that it does not deploy the app on the new server, instead you need to run abra app deploy, (we could maybe do that in the future) Usage: abra app move gitea.example.com serverb.example.com abra app deploy gitea.example.com
p4u1 added 1 commit 2025-08-18 12:34:09 +00:00
feat: Adds abra app move
All checks were successful
continuous-integration/drone/pr Build is passing
9849d47b64
Author
Member

I successfully moved a forgejo app with this, but couldn't get a moved nextcloud app running, because of some weird permission problems...

I successfully moved a forgejo app with this, but couldn't get a moved nextcloud app running, because of some weird permission problems...
Author
Member

also the code is a total mess right now

also the code is a total mess right now
p4u1 added 1 commit 2025-08-18 15:44:12 +00:00
p4u1 added 1 commit 2025-08-19 19:41:49 +00:00
p4u1 force-pushed abra-app-move from 3be23783fe to d2c8aa5659 2025-08-19 19:44:18 +00:00 Compare
Author
Member

I cleaned up code and added fancy move overview:

┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃                      MOVE OVERVIEW                      ┃
┃                                                         ┃
┃ DOMAIN        testcloud.klasse-methode.it               ┃
┃ RECIPE        nextcloud                                 ┃
┃ OLD SERVER    marx.klasse-methode.it                    ┃
┃ New SERVER    gramsci.klasse-methode.it                 ┃
┃               admin_password                            ┃
┃               db_password                               ┃
┃               elasticsearch_password                    ┃
┃               smtp_password                             ┃
┃ SECRETS       db_root_password                          ┃
┃               testcloud_klasse-methode_it_mariadb       ┃
┃               testcloud_klasse-methode_it_elasticsearch ┃
┃               testcloud_klasse-methode_it_nextcloud     ┃
┃               testcloud_klasse-methode_it_nextconfig    ┃
┃               testcloud_klasse-methode_it_nextapps      ┃
┃               testcloud_klasse-methode_it_nextdata      ┃
┃ VOLUMES       testcloud_klasse-methode_it_redis         ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

Would love to get some feedback on the general direction. @decentral1se especially you, since you raised concerns about adding this to abra directly (which I totally get). If we decide not to add this to abra directly maybe we can add a new cmd to this project? similar to kadabra

I cleaned up code and added fancy move overview: ``` ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ MOVE OVERVIEW ┃ ┃ ┃ ┃ DOMAIN testcloud.klasse-methode.it ┃ ┃ RECIPE nextcloud ┃ ┃ OLD SERVER marx.klasse-methode.it ┃ ┃ New SERVER gramsci.klasse-methode.it ┃ ┃ admin_password ┃ ┃ db_password ┃ ┃ elasticsearch_password ┃ ┃ smtp_password ┃ ┃ SECRETS db_root_password ┃ ┃ testcloud_klasse-methode_it_mariadb ┃ ┃ testcloud_klasse-methode_it_elasticsearch ┃ ┃ testcloud_klasse-methode_it_nextcloud ┃ ┃ testcloud_klasse-methode_it_nextconfig ┃ ┃ testcloud_klasse-methode_it_nextapps ┃ ┃ testcloud_klasse-methode_it_nextdata ┃ ┃ VOLUMES testcloud_klasse-methode_it_redis ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ ``` Would love to get some feedback on the general direction. @decentral1se especially you, since you raised concerns about adding this to abra directly (which I totally get). If we decide not to add this to abra directly maybe we can add a new cmd to this project? similar to kadabra
Owner

@p4u1 looking good, i'm all for it! let me know when you want a code review and I can take a pass. from what i can tell, it's all looking pretty "standard" / "maintainable" and it would be great to support it well. I am not sure how we can test this but maybe you have ideas. I imagine validating that all required binaries/system executables are present needs some extra work. Also, some graceful handling for error cases will be tricky. I agree with the separation to not deploy the new moved app, that's for the operator to figure out (probably more chaos 🙃).

@p4u1 looking good, i'm all for it! let me know when you want a code review and I can take a pass. from what i can tell, it's all looking pretty "standard" / "maintainable" and it would be great to support it well. I am not sure how we can test this but maybe you have ideas. I imagine validating that all required binaries/system executables are present needs some extra work. Also, some graceful handling for error cases will be tricky. I agree with the separation to not deploy the new moved app, that's for the operator to figure out (probably more chaos 🙃).
Author
Member

The only special thing is, that it's not only using the dockerclient but also running ssh and scp, wich has not been done before in Abra I think.

I will clean the code some more but should be ready for review soon.

One thing that troubles me is the file permission problem when moving a next cloud app. All other apps I moved worked, but when moving nextcloud it does not start because a test to create a file does not work. And I'm not quite sure what to try next because I already set the flags on scp to keep permission and ownership. If you have any ideas, please let me know!

Regarding testing I will try to come up with something, but we will probably need a second server

The only special thing is, that it's not only using the dockerclient but also running ssh and scp, wich has not been done before in Abra I think. I will clean the code some more but should be ready for review soon. One thing that troubles me is the file permission problem when moving a next cloud app. All other apps I moved worked, but when moving nextcloud it does not start because a test to create a file does not work. And I'm not quite sure what to try next because I already set the flags on scp to keep permission and ownership. If you have any ideas, please let me know! Regarding testing I will try to come up with something, but we will probably need a second server
p4u1 force-pushed abra-app-move from d2c8aa5659 to b8dbc5d65d 2025-08-22 14:07:20 +00:00 Compare
Author
Member

@decentral1se I fixed the premission problem (I had to create the volume before copying the files, otherwise docker wouls reset the folder ownership). I also cleaned to code a bit and added some info at the end.

So this is ready for a review :)

@decentral1se I fixed the premission problem (I had to create the volume before copying the files, otherwise docker wouls reset the folder ownership). I also cleaned to code a bit and added some info at the end. So this is ready for a review :)
p4u1 force-pushed abra-app-move from b8dbc5d65d to 126acee958 2025-08-22 14:21:39 +00:00 Compare
p4u1 changed title from WIP feat: Adds abra app move to feat: Adds abra app move 2025-08-22 14:21:48 +00:00
decentral1se reviewed 2025-08-22 15:17:40 +00:00
decentral1se left a comment
Owner

Great work 👏👏👏

Great work 👏👏👏
@ -0,0 +97,4 @@
secretName := strings.Join(sname[:len(sname)-1], "_")
data := resources.Secrets[secretName]
if err := client.StoreSecret(cl2, s.Spec.Name, data); err != nil {
log.Infof("creating secret: %s", s.Spec.Name)
Owner

Is this log.Infof in the right place? Seems like it should log earlier in the success case?

Is this `log.Infof` in the right place? Seems like it should log earlier in the success case?
decentral1se marked this conversation as resolved
@ -0,0 +109,4 @@
// docker creates a new volume it set the folder permissions to
// root, which might be wrong. This ensures we always have the
// correct folder permissions inside the volume.
log.Debug("creating volume: %s", v.Name)
Owner

log.Debugf? (there are more cases of this below)

`log.Debugf`? (there are more cases of this below)
decentral1se marked this conversation as resolved
@ -0,0 +174,4 @@
fmt.Println("Run the following command to deploy the app", app.Name, newServer)
fmt.Println(" abra app deploy --no-domain-checks", app.Domain)
fmt.Println()
fmt.Println("And don't forget to update you DNS record. And don't panic, as it might take a bit for the dust to settle. Traefik for example might fail to obtain the lets encrypt certificate for a while.", app.Domain)
Owner

I like this but I think the DNS record part should be in the main --help description. I wouldn't mention Traefik in the case in the future that other proxies are used (I think Caddy is already being used...) and this is out-of-date / potentially confusing.

I like this but I think the DNS record part should be in the main `--help` description. I wouldn't mention Traefik in the case in the future that other proxies are used (I think Caddy is already being used...) and this is out-of-date / potentially confusing.
decentral1se marked this conversation as resolved
@ -0,0 +176,4 @@
fmt.Println()
fmt.Println("And don't forget to update you DNS record. And don't panic, as it might take a bit for the dust to settle. Traefik for example might fail to obtain the lets encrypt certificate for a while.", app.Domain)
fmt.Println()
fmt.Println("If anything goes wrong, you can always move the app config file to the original server and deploy it there again. There was no data removed on the old server")
Owner

I would add this really centrally to the main --help output instead so people know beforehand!

I would add this really centrally to the main `--help` output instead so people know beforehand!
decentral1se marked this conversation as resolved
@ -0,0 +262,4 @@
targetContainer, err := containerPkg.GetContainer(context.Background(), cl, f, true)
if err != nil {
log.Error(err)
continue
Owner

This code used to be in a for loop and now isn't? continue looks suspect here.

This code used to be in a for loop and now isn't? `continue` looks suspect here.
decentral1se marked this conversation as resolved
@ -0,0 +276,4 @@
}
log.Debugf("extracting secret %s", secretName)
out, err := exec.Command("ssh", app.Server, "-tt", fmt.Sprintf("sudo cat /var/lib/docker/containers/%s/mounts/secrets/%s", targetContainer.ID, secretID)).Output()
Owner

I think it should mention that tar / cat are required on the target server in the main --help output. You might also consider doing a check up-front at the start of the command execution? It will be slower but less painful if things are missing...

I think it should mention that `tar` / `cat` are required on the target server in the main `--help` output. You might also consider doing a check up-front at the start of the command execution? It will be slower but less painful if things are missing...
decentral1se marked this conversation as resolved
@ -143,0 +162,4 @@
{"DOMAIN", domain},
{"RECIPE", app.Recipe.Name},
{"OLD SERVER", server},
{"New SERVER", newServer},
Owner

NEW

`NEW`
decentral1se marked this conversation as resolved
@ -77,3 +77,3 @@
}
log.Info("polling undeploy status")
log.Debug("polling undeploy status")
Owner

That seems like a somewhat significant "off-topic" change to the undeploy screen for this PR. I have no strong feelings about it but we regularly see reported issues when the output of deploy/undeploy changes. If you are happy to deal with that, then I'm fine with it 🙃

That seems like a somewhat significant "off-topic" change to the undeploy screen for this PR. I have no strong feelings about it but we regularly see reported issues when the output of `deploy`/`undeploy` changes. If you are happy to deal with that, then I'm fine with it 🙃
decentral1se marked this conversation as resolved
Owner

Regarding testing I will try to come up with something, but we will probably need a second server

Hmmmm that sounds a bit wasteful and also awkward. The integration test suite is already quite overwhelming to get into running for the everyday hacker and this would complicate things. Is there a way to test the underlying logic in the unit tests? Is there a way to make this logic more testable? I did think of docker:dind to mock a remote server but that could be quite chaotic...

> Regarding testing I will try to come up with something, but we will probably need a second server Hmmmm that sounds a bit wasteful and also awkward. The integration test suite is already quite overwhelming to get into running for the everyday hacker and this would complicate things. Is there a way to test the underlying logic in the unit tests? Is there a way to make this logic more testable? I did think of `docker:dind` to mock a remote server but that could be quite chaotic...
decentral1se added this to the Abra v0.11.x project 2025-08-25 10:56:21 +00:00
decentral1se moved this to In Progress in Abra v0.11.x on 2025-08-25 10:56:36 +00:00
p4u1 was assigned by decentral1se 2025-08-27 14:01:03 +00:00
p4u1 was unassigned by decentral1se 2025-08-27 14:01:10 +00:00
p4u1 was assigned by decentral1se 2025-08-27 14:01:36 +00:00
Author
Member

I will be away until end of September, if anyone wants to pick this up feel free. Otherwise I will finish it when I'm back :)

I will be away until end of September, if anyone wants to pick this up feel free. Otherwise I will finish it when I'm back :)
decentral1se removed this from the Abra v0.11.x project 2025-08-28 09:33:39 +00:00
decentral1se added this to the Abra "next" project 2025-08-29 11:02:27 +00:00
Owner
https://git.coopcloud.tech/toolshed/abra/pulls/636
decentral1se closed this pull request 2025-09-01 05:44:37 +00:00
decentral1se modified the project from Abra "next" to Abra v0.11.x 2025-09-04 08:48:04 +00:00
decentral1se moved this to Done in Abra v0.11.x on 2025-09-04 08:48:16 +00:00

Pull request closed

Sign in to join this conversation.
No description provided.