Adding server prune and undeploy prune #278

						
				@@ -14,0 +18,4 @@

				var pruneFlag = &cli.BoolFlag{

					Name:        "prune, p",

					Destination: &prune,

					Usage:       "Prunes unused containers, networks, and dangling images for stack",

decentral1se commented

2023-02-15 09:07:35 +00:00

stack 👉 an app

`stack` 👉 `an app`

codegod100 marked this conversation as resolved

cli/app/undeploy.go Outdated

						
				@@ -57,5 +67,39 @@ volumes as eligiblef or pruning once undeployed.

						return nil

					},

					After: func(c *cli.Context) error {

decentral1se commented

2023-02-15 09:10:49 +00:00

Hmmm, I'm not sure we should be putting this in After as it has some not obvious runtime behaviour (see https://pkg.go.dev/github.com/urfave/cli/v2#AfterFunc) and also the --prune is part of the normal functioning of the sub-command, not a thing to happen after execution? Perhaps abstracting to a function and calling from the main Action?

Hmmm, I'm not sure we should be putting this in `After` as it has some not obvious runtime behaviour (see https://pkg.go.dev/github.com/urfave/cli/v2#AfterFunc) and also the `--prune` is part of the normal functioning of the sub-command, not a thing to happen after execution? Perhaps abstracting to a function and calling from the main `Action`?

codegod100 commented

2023-02-15 09:27:34 +00:00

this part? it is run even if Action() panics
The issue I was trying to tackle is that pruning while docker container is still running is kinda pointless. when we run abra app undeploy It takes clock time for the container to spin down. maybe we put an arbitrary wait in there? I hate doing that but not sure another way since we are done talking to docker by the time the actual spindown happens.

this part? `it is run even if Action() panics` The issue I was trying to tackle is that pruning while docker container is still running is kinda pointless. when we run `abra app undeploy` It takes clock time for the container to spin down. maybe we put an arbitrary wait in there? I hate doing that but not sure another way since we are done talking to docker by the time the actual spindown happens.

decentral1se commented

2023-02-15 14:10:23 +00:00

Aha, I see. Could you just poll every second for the stack until it goes away? Then you can drop into the pruning logic after and still keep it all in the main Action code path.

Aha, I see. Could you just poll every second for the stack until it goes away? Then you can drop into the pruning logic after and still keep it all in the main `Action` code path.

codegod100 marked this conversation as resolved

cli/app/undeploy.go Outdated

						
				@@ -60,0 +79,4 @@

						}

						ctx := context.Background()

						pruneFilters := filters.NewArgs()

decentral1se commented

2023-02-15 09:12:55 +00:00

Could this be tucked into app.Filters? You might need to add a flag to specify the * logic? Would be then:

fs, err := app.Filters(false, false)
if err != nil {
    logrus.Fatal(err)
}

Could potentially learn from the new runtime package options praxis 🤔 For another day...

Could this be tucked into `app.Filters`? You might need to add a flag to specify the `*` logic? Would be then: ``` fs, err := app.Filters(false, false) if err != nil { logrus.Fatal(err) } ``` Could potentially learn from the new `runtime` package options praxis 🤔 For another day...

codegod100 commented

2023-02-17 07:47:49 +00:00

I'm kinda confused what you mean here. What are those false,false args being passed to Filters()?

I'm kinda confused what you mean here. What are those `false,false` args being passed to `Filters()`?

decentral1se commented

2023-02-17 08:16:27 +00:00

Yeh it's kinda a mess not of our making, i'll copy in the comments:

// Filters retrieves exact app filters for querying the container runtime. Due
// to upstream issues, filtering works different depending on what you're
// querying. So, for example, secrets don't work with regex! The caller needs
// to implement their own validation that the right secrets are matched. In
// order to handle these cases, we provide the `appendServiceNames` /
// `exactMatch` modifiers.

Filters goal was to hide away the weirdness. But I see now again that the function arguments are also confusing. Anyway, feel free to skip this for now and I can loop back after for a re-factor.

Yeh it's kinda a mess not of our making, i'll copy in the comments: ``` // Filters retrieves exact app filters for querying the container runtime. Due // to upstream issues, filtering works different depending on what you're // querying. So, for example, secrets don't work with regex! The caller needs // to implement their own validation that the right secrets are matched. In // order to handle these cases, we provide the `appendServiceNames` / // `exactMatch` modifiers. ``` `Filters` goal was to hide away the weirdness. But I see now again that the function arguments are also confusing. Anyway, feel free to skip this for now and I can loop back after for a re-factor.

codegod100 marked this conversation as resolved

cli/app/undeploy.go Outdated

						
				@@ -60,0 +84,4 @@

						pruneFilters.Add("label", stackSearch)

						cr, err := cl.ContainersPrune(ctx, pruneFilters)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:13:49 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/app/undeploy.go Outdated

						
				@@ -60,0 +90,4 @@

						nr, err := cl.NetworksPrune(ctx, pruneFilters)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:13:55 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/app/undeploy.go Outdated

						
				@@ -60,0 +96,4 @@

						ir, err := cl.ImagesPrune(ctx, pruneFilters)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:14:01 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/server/prune.go Outdated

						
				@@ -0,0 +30,4 @@

					Name:    "prune",

					Aliases: []string{"p"},

					Usage:   "Prune a managed server; Runs a docker system prune",

					Description: `

decentral1se commented

2023-02-15 09:14:47 +00:00

Could be a "..." oneliner after all?

Could be a `"..."` oneliner after all?

codegod100 marked this conversation as resolved

cli/server/prune.go

						
				@@ -0,0 +47,4 @@

						cl, err := client.New(serverName)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:15:01 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/server/prune.go

						
				@@ -0,0 +52,4 @@

						ctx := context.Background()

						cr, err := cl.ContainersPrune(ctx, args)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:15:09 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/server/prune.go

						
				@@ -0,0 +58,4 @@

						nr, err := cl.NetworksPrune(ctx, args)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:15:18 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/server/prune.go

						
				@@ -0,0 +68,4 @@

						}

						ir, err := cl.ImagesPrune(ctx, pruneFilters)

						if err != nil {

							return err

decentral1se commented

2023-02-15 09:15:29 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

cli/server/prune.go

						
				@@ -0,0 +75,4 @@

						if volunesFilter {

							vr, err := cl.VolumesPrune(ctx, args)

							if err != nil {

								return err

decentral1se commented

2023-02-15 09:15:36 +00:00

logrus.Fatal(err)

`logrus.Fatal(err)`

codegod100 marked this conversation as resolved

codegod100 added 1 commit 2023-02-17 08:24:45 +00:00

review cleanups

continuous-integration/drone/pr Build is passing

Details

0d8191bc3e

codegod100 commented

2023-02-17 08:30:00 +00:00

all changes fixed

decentral1se approved these changes 2023-02-17 08:53:34 +00:00

decentral1se merged commit b14ec0cda4 into main

2023-02-17 08:53:46 +00:00

decentral1se referenced this pull request from toolshed/organising

2023-02-17 09:49:26 +00:00

Refactor app.Filters to re-use options approach #419

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: toolshed/abra#278

Allow edits from maintainers