From 099b64935b446a65b36f442cdfee6e581a95421a Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 10 Sep 2020 14:17:49 +0200 Subject: [PATCH] Minor fixes Co-authored-by: Chris Crone Signed-off-by: Guillaume Tardif --- aci/cloud.go | 2 +- aci/compose.go | 12 +++---- aci/containers.go | 20 +++++------ aci/volumes.go | 34 ++++++++----------- api/client/client.go | 3 +- cli/cmd/volume/acivolume.go | 3 +- cli/cmd/volume/{acilist.go => list.go} | 8 ++--- .../volume/{acilist_test.go => list_test.go} | 0 cli/main.go | 17 ++++------ ecs/backend.go | 4 +-- ecs/local/backend.go | 7 ++-- 11 files changed, 47 insertions(+), 63 deletions(-) rename cli/cmd/volume/{acilist.go => list.go} (86%) rename cli/cmd/volume/{acilist_test.go => list_test.go} (100%) diff --git a/aci/cloud.go b/aci/cloud.go index 0dee380f9..ff761800a 100644 --- a/aci/cloud.go +++ b/aci/cloud.go @@ -31,7 +31,7 @@ type aciCloudService struct { func (cs *aciCloudService) Login(ctx context.Context, params interface{}) error { opts, ok := params.(LoginParams) if !ok { - return errors.New("Could not read azure LoginParams struct from generic parameter") + return errors.New("could not read Azure LoginParams struct from generic parameter") } if opts.ClientID != "" { return cs.loginService.LoginServicePrincipal(opts.ClientID, opts.ClientSecret, opts.TenantID) diff --git a/aci/compose.go b/aci/compose.go index d7fd4e321..fe6a3e1eb 100644 --- a/aci/compose.go +++ b/aci/compose.go @@ -37,7 +37,7 @@ type aciComposeService struct { } func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) error { - logrus.Debugf("Up on project with name %q\n", project.Name) + logrus.Debugf("Up on project with name %q", project.Name) groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, *project) addTag(&groupDefinition, composeContainerTag) @@ -48,7 +48,7 @@ func (cs *aciComposeService) Up(ctx context.Context, project *types.Project) err } func (cs *aciComposeService) Down(ctx context.Context, project string) error { - logrus.Debugf("Down on project with name %q\n", project) + logrus.Debugf("Down on project with name %q", project) cg, err := deleteACIContainerGroup(ctx, cs.ctx, project) if err != nil { @@ -69,11 +69,11 @@ func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose. group, err := groupsClient.Get(ctx, cs.ctx.ResourceGroup, project) if err != nil { - return []compose.ServiceStatus{}, err + return nil, err } - if group.Containers == nil || len(*group.Containers) < 1 { - return []compose.ServiceStatus{}, fmt.Errorf("no containers found in ACI container group %s", project) + if group.Containers == nil || len(*group.Containers) == 0 { + return nil, fmt.Errorf("no containers found in ACI container group %s", project) } res := []compose.ServiceStatus{} @@ -89,7 +89,7 @@ func (cs *aciComposeService) Ps(ctx context.Context, project string) ([]compose. func (cs *aciComposeService) List(ctx context.Context, project string) ([]compose.Stack, error) { containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) if err != nil { - return []compose.Stack{}, err + return nil, err } stacks := []compose.Stack{} diff --git a/aci/containers.go b/aci/containers.go index afc48ec3d..61c50b656 100644 --- a/aci/containers.go +++ b/aci/containers.go @@ -43,12 +43,12 @@ type aciContainerService struct { func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers.Container, error) { containerGroups, err := getACIContainerGroups(ctx, cs.ctx.SubscriptionID, cs.ctx.ResourceGroup) if err != nil { - return []containers.Container{}, err + return nil, err } var res []containers.Container for _, group := range containerGroups { - if group.Containers == nil || len(*group.Containers) < 1 { - return []containers.Container{}, fmt.Errorf("no containers found in ACI container group %s", *group.Name) + if group.Containers == nil || len(*group.Containers) == 0 { + return nil, fmt.Errorf("no containers found in ACI container group %s", *group.Name) } for _, container := range *group.Containers { @@ -64,7 +64,7 @@ func (cs *aciContainerService) List(ctx context.Context, all bool) ([]containers func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerConfig) error { if strings.Contains(r.ID, composeContainerSeparator) { - return errors.New(fmt.Sprintf("invalid container name. ACI container name cannot include %q", composeContainerSeparator)) + return fmt.Errorf("invalid container name. ACI container name cannot include %q", composeContainerSeparator) } project, err := convert.ContainerToComposeProject(r) @@ -72,7 +72,7 @@ func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerCo return err } - logrus.Debugf("Running container %q with name %q\n", r.Image, r.ID) + logrus.Debugf("Running container %q with name %q", r.Image, r.ID) groupDefinition, err := convert.ToContainerGroup(ctx, cs.ctx, project) if err != nil { return err @@ -86,7 +86,7 @@ func (cs *aciContainerService) Start(ctx context.Context, containerID string) er groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot start specified service %q from compose application %q, you can update and restart the entire compose app with docker compose up --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } containerGroupsClient, err := login.NewContainerGroupsClient(cs.ctx.SubscriptionID) @@ -110,12 +110,12 @@ func (cs *aciContainerService) Start(ctx context.Context, containerID string) er func (cs *aciContainerService) Stop(ctx context.Context, containerID string, timeout *uint32) error { if timeout != nil && *timeout != uint32(0) { - return errors.Errorf("ACI integration does not support setting a timeout to stop a container before killing it.") + return fmt.Errorf("the ACI integration does not support setting a timeout to stop a container before killing it") } groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot stop service %q from compose application %q, you can stop the entire compose app with docker stop %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } return stopACIContainerGroup(ctx, cs.ctx, groupName) } @@ -124,7 +124,7 @@ func (cs *aciContainerService) Kill(ctx context.Context, containerID string, _ s groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot kill service %q from compose application %q, you can kill the entire compose app with docker kill %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } return stopACIContainerGroup(ctx, cs.ctx, groupName) // As ACI doesn't have a kill command, we are using the stop implementation instead } @@ -187,7 +187,7 @@ func (cs *aciContainerService) Delete(ctx context.Context, containerID string, r groupName, containerName := getGroupAndContainerName(containerID) if groupName != containerID { msg := "cannot delete service %q from compose application %q, you can delete the entire compose app with docker compose down --project-name %s" - return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName)) + return fmt.Errorf(msg, containerName, groupName, groupName) } if !request.Force { diff --git a/aci/volumes.go b/aci/volumes.go index 3a43d68af..250f0ffa5 100644 --- a/aci/volumes.go +++ b/aci/volumes.go @@ -19,22 +19,19 @@ package aci import ( "context" "fmt" + "net/http" "strings" - "github.com/docker/compose-cli/progress" - - "github.com/Azure/go-autorest/autorest/to" - - "github.com/docker/compose-cli/aci/login" - - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" - - "github.com/docker/compose-cli/api/volumes" - "github.com/docker/compose-cli/errdefs" - "github.com/pkg/errors" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" + "github.com/Azure/go-autorest/autorest/to" + + "github.com/docker/compose-cli/aci/login" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" + "github.com/docker/compose-cli/progress" ) type aciVolumeService struct { @@ -75,7 +72,7 @@ func (cs *aciVolumeService) List(ctx context.Context) ([]volumes.Volume, error) return fileShares, nil } -//VolumeCreateOptions options to create a new ACI volume +// VolumeCreateOptions options to create a new ACI volume type VolumeCreateOptions struct { Account string Fileshare string @@ -84,7 +81,7 @@ type VolumeCreateOptions struct { func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (volumes.Volume, error) { opts, ok := options.(VolumeCreateOptions) if !ok { - return volumes.Volume{}, errors.New("Could not read azure VolumeCreateOptions struct from generic parameter") + return volumes.Volume{}, errors.New("could not read Azure VolumeCreateOptions struct from generic parameter") } w := progress.ContextWriter(ctx) w.Event(event(opts.Account, progress.Working, "Validating")) @@ -96,7 +93,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo if err == nil { w.Event(event(opts.Account, progress.Done, "Use existing")) } else { - if account.StatusCode != 404 { + if account.StatusCode != http.StatusNotFound { return volumes.Volume{}, err } result, err := accountClient.CheckNameAvailability(ctx, storage.AccountCheckNameAvailabilityParameters{ @@ -118,8 +115,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } - err = future.WaitForCompletionRef(ctx, accountClient.Client) - if err != nil { + if err := future.WaitForCompletionRef(ctx, accountClient.Client); err != nil { w.Event(errorEvent(opts.Account)) return volumes.Volume{}, err } @@ -141,7 +137,7 @@ func (cs *aciVolumeService) Create(ctx context.Context, options interface{}) (vo w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, errors.Wrapf(errdefs.ErrAlreadyExists, "Azure fileshare %q already exists", opts.Fileshare) } - if fileShare.StatusCode != 404 { + if fileShare.StatusCode != http.StatusNotFound { w.Event(errorEvent(opts.Fileshare)) return volumes.Volume{}, err } @@ -199,7 +195,7 @@ func (cs *aciVolumeService) Delete(ctx context.Context, id string, options inter if err == nil { if _, ok := account.Tags[dockerVolumeTag]; ok { result, err := storageAccountsClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount) - if result.StatusCode == 204 { + if result.StatusCode == http.StatusNoContent { return errors.Wrapf(errdefs.ErrNotFound, "storage account %s does not exist", storageAccount) } return err @@ -209,7 +205,7 @@ func (cs *aciVolumeService) Delete(ctx context.Context, id string, options inter result, err := fileShareClient.Delete(ctx, cs.aciContext.ResourceGroup, storageAccount, fileshare) if result.StatusCode == 204 { - return errors.Wrapf(errdefs.ErrNotFound, "fileshare %s does not exist", fileshare) + return errors.Wrapf(errdefs.ErrNotFound, "fileshare %q", fileshare) } return err } diff --git a/api/client/client.go b/api/client/client.go index 632dc3256..fc7c1c277 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -19,11 +19,10 @@ package client import ( "context" - "github.com/docker/compose-cli/api/volumes" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" diff --git a/cli/cmd/volume/acivolume.go b/cli/cmd/volume/acivolume.go index a4e4dd6d6..d419b8e1a 100644 --- a/cli/cmd/volume/acivolume.go +++ b/cli/cmd/volume/acivolume.go @@ -58,8 +58,7 @@ func createVolume() *cobra.Command { return err } err = progress.Run(ctx, func(ctx context.Context) error { - _, err := c.VolumeService().Create(ctx, aciOpts) - if err != nil { + if _, err := c.VolumeService().Create(ctx, aciOpts); err != nil { return err } return nil diff --git a/cli/cmd/volume/acilist.go b/cli/cmd/volume/list.go similarity index 86% rename from cli/cmd/volume/acilist.go rename to cli/cmd/volume/list.go index 7261f3427..bca775960 100644 --- a/cli/cmd/volume/acilist.go +++ b/cli/cmd/volume/list.go @@ -32,7 +32,7 @@ import ( func listVolume() *cobra.Command { cmd := &cobra.Command{ Use: "ls", - Short: "list Azure file shares usable as ACI volumes.", + Short: "list available volumes in context.", Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { c, err := client.New(cmd.Context()) @@ -53,14 +53,14 @@ func listVolume() *cobra.Command { func printList(out io.Writer, volumes []volumes.Volume) { printSection(out, func(w io.Writer) { for _, vol := range volumes { - fmt.Fprintf(w, "%s\t%s\n", vol.ID, vol.Description) // nolint:errcheck + _, _ = fmt.Fprintf(w, "%s\t%s\n", vol.ID, vol.Description) } }, "ID", "DESCRIPTION") } func printSection(out io.Writer, printer func(io.Writer), headers ...string) { w := tabwriter.NewWriter(out, 20, 1, 3, ' ', 0) - fmt.Fprintln(w, strings.Join(headers, "\t")) // nolint:errcheck + _, _ = fmt.Fprintln(w, strings.Join(headers, "\t")) printer(w) - w.Flush() // nolint:errcheck + _ = w.Flush() } diff --git a/cli/cmd/volume/acilist_test.go b/cli/cmd/volume/list_test.go similarity index 100% rename from cli/cmd/volume/acilist_test.go rename to cli/cmd/volume/list_test.go diff --git a/cli/main.go b/cli/main.go index db88150eb..a4eadd1bb 100644 --- a/cli/main.go +++ b/cli/main.go @@ -27,26 +27,16 @@ import ( "syscall" "time" - volume "github.com/docker/compose-cli/cli/cmd/volume" - "github.com/docker/compose-cli/cli/cmd/compose" - "github.com/docker/compose-cli/cli/cmd/logout" - + volume "github.com/docker/compose-cli/cli/cmd/volume" "github.com/docker/compose-cli/errdefs" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/spf13/cobra" // Backend registrations _ "github.com/docker/compose-cli/aci" - _ "github.com/docker/compose-cli/ecs" - _ "github.com/docker/compose-cli/ecs/local" - _ "github.com/docker/compose-cli/example" - _ "github.com/docker/compose-cli/local" - "github.com/docker/compose-cli/metrics" - "github.com/docker/compose-cli/cli/cmd" contextcmd "github.com/docker/compose-cli/cli/cmd/context" "github.com/docker/compose-cli/cli/cmd/login" @@ -56,6 +46,11 @@ import ( "github.com/docker/compose-cli/config" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/store" + _ "github.com/docker/compose-cli/ecs" + _ "github.com/docker/compose-cli/ecs/local" + _ "github.com/docker/compose-cli/example" + _ "github.com/docker/compose-cli/local" + "github.com/docker/compose-cli/metrics" ) var ( diff --git a/ecs/backend.go b/ecs/backend.go index a615cbeb4..8cc0a392c 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -19,14 +19,12 @@ package ecs import ( "context" - "github.com/docker/compose-cli/api/volumes" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/cloud" diff --git a/ecs/local/backend.go b/ecs/local/backend.go index cec7c104f..c62ed640c 100644 --- a/ecs/local/backend.go +++ b/ecs/local/backend.go @@ -19,17 +19,14 @@ package local import ( "context" - "github.com/docker/compose-cli/api/volumes" - - "github.com/docker/docker/client" - "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" - + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/context/store" + "github.com/docker/docker/client" ) const backendType = store.EcsLocalSimulationContextType