feat: initial commit for abra app volume ls/rm #51

Merged
roxxers merged 2 commits from knoflook/abra:secret-create into main 2021-08-18 18:29:14 +00:00
2 changed files with 121 additions and 0 deletions
Showing only changes of commit 5663659d23 - Show all commits

View File

@ -29,5 +29,6 @@ scaling apps up and spinning them down.
appRunCommand,
appRollbackCommand,
appSecretCommand,
appVolumeCommand,
},
}

120
cli/app/volume.go Normal file
View File

@ -0,0 +1,120 @@
package app
import (
"context"
"errors"
"fmt"
"coopcloud.tech/abra/cli/internal"
"coopcloud.tech/abra/client"
"coopcloud.tech/abra/config"
"github.com/AlecAivazis/survey/v2"
"github.com/docker/docker/api/types/filters"
"github.com/sirupsen/logrus"
"github.com/urfave/cli/v2"
)
var appVolumeListCommand = &cli.Command{
Name: "list",
Usage: "list volumes associated with an app",
Aliases: []string{"ls"},
Action: func(c *cli.Context) error {
appName := c.Args().First()
if appName == "" {
internal.ShowSubcommandHelpAndError(c, errors.New("No app name provided!"))
roxxers marked this conversation as resolved Outdated

Style: Errors should not start with capital. https://staticcheck.io/docs/checks#ST1005

Style: Errors should not start with capital. https://staticcheck.io/docs/checks#ST1005

I am also unsure of using this error vs just printing the help page without the error. Since not giving the first arg seems to do so without error for other commands. This is more a thing for future clean up tbf and me rambling.

I am also unsure of using this error vs just printing the help page without the error. Since not giving the first arg seems to do so without error for other commands. This is more a thing for future clean up tbf and me rambling.
}
appFiles, err := config.LoadAppFiles("")
if err != nil {
logrus.Fatal(err)
}
host := appFiles[appName].Server
knoflook marked this conversation as resolved Outdated

As discussed with my panic hour, this just isn't checking if the app itself exists leading to a weird error that is very confusing. Like "Context '' does not exist."

var host string
if app, ok := appFiles[appName]; ok {
	host = app.Server
}
if host == "" {
	logrus.Fatalf(`app "%s" does not exist`, appName)
}

Just tested this and it works great with this addition.

As discussed with my panic hour, this just isn't checking if the app itself exists leading to a weird error that is very confusing. Like "Context '' does not exist." ```go var host string if app, ok := appFiles[appName]; ok { host = app.Server } if host == "" { logrus.Fatalf(`app "%s" does not exist`, appName) } ``` Just tested this and it works great with this addition.

Just realised this should be simplified to

var host string
if app, ok := appFiles[appName]; ok {
	host = app.Server
} else {
	logrus.Fatalf(`app "%s" does not exist`, appName)
}
Just realised this should be simplified to ```go var host string if app, ok := appFiles[appName]; ok { host = app.Server } else { logrus.Fatalf(`app "%s" does not exist`, appName) } ```
ctx := context.Background()
cl, err := client.NewClientWithContext(host)
if err != nil {
logrus.Fatal(err)
}
fs := filters.NewArgs()
fs.Add("name", appName)
volumeListOKBody, err := cl.VolumeList(ctx, fs)
volumeList := volumeListOKBody.Volumes
if err != nil {
logrus.Fatal(err)
}
fmt.Println("DRIVER\t\t\tVOLUME NAME")
knoflook marked this conversation as resolved Outdated

Minimal: We use table creation in the formatter for the abra/cli package

Minimal: We use table creation in the formatter for the abra/cli package
for _, volume := range volumeList {
fmt.Printf("%s\t\t\t%s\n", volume.Driver, volume.Name)
}
return nil
},
}
var appVolumeRemoveCommand = &cli.Command{
Name: "remove",
Usage: "remove volume(s) associated with an app",
Aliases: []string{"rm", "delete"},
Flags: []cli.Flag{
internal.ForceFlag,
},
Action: func(c *cli.Context) error {
appName := c.Args().First()
if appName == "" {
internal.ShowSubcommandHelpAndError(c, errors.New("No app name provided!"))
}
appFiles, err := config.LoadAppFiles("")
if err != nil {
logrus.Fatal(err)
}
host := appFiles[appName].Server
knoflook marked this conversation as resolved Outdated

Same issue as before. Since you are doing this twice you could refactor some of the calls here into their own functions so both commands can use them?

Same issue as before. Since you are doing this twice you could refactor some of the calls here into their own functions so both commands can use them?
ctx := context.Background()
cl, err := client.NewClientWithContext(host)
if err != nil {
logrus.Fatal(err)
}
fs := filters.NewArgs()
fs.Add("name", appName)
volumeListOKBody, err := cl.VolumeList(ctx, fs)
volumeList := volumeListOKBody.Volumes
if err != nil {
logrus.Fatal(err)
}
var volumeNames []string
for _, volume := range volumeList {
volumeNames = append(volumeNames, volume.Name)
}
var volumesToRemove []string
if !internal.Force {
roxxers marked this conversation as resolved Outdated

This is great!!

This is great!!
volumesPrompt := &survey.MultiSelect{
Message: "Which volumes do you want to remove?",
Options: volumeNames,
Default: volumeNames,
}
if err := survey.AskOne(volumesPrompt, &volumesToRemove); err != nil {
logrus.Fatal(err)
}
} else {
volumesToRemove = volumeNames
}
for _, vol := range volumesToRemove {
err := cl.VolumeRemove(ctx, vol, internal.Force)
if err != nil {
logrus.Fatal(err)
}
logrus.Info(fmt.Sprintf("Volume %s removed.", vol))
}
return nil
},
}
var appVolumeCommand = &cli.Command{
Name: "volume",
Usage: "List or remove volumes associated with app",
ArgsUsage: "<command>",
Subcommands: []*cli.Command{
appVolumeListCommand,
appVolumeRemoveCommand,
},
}