From b25a6b4bd6c30f23de7d45cae2a06f6bb7d657cc Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 7 May 2020 04:58:04 +0200 Subject: [PATCH 1/7] Add volumes to run command Signed-off-by: Ulysses Souza --- azure/backend.go | 16 ++++-- azure/convert/volume.go | 110 ++++++++++++++++++++++++++++++++++++++++ cli/cmd/run/opts.go | 54 ++++++++++++++++++++ cli/cmd/run/run.go | 6 +-- cli/options/run/opts.go | 1 + containers/api.go | 4 +- 6 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 azure/convert/volume.go create mode 100644 cli/cmd/run/opts.go diff --git a/azure/backend.go b/azure/backend.go index 40f112599..e86c989bd 100644 --- a/azure/backend.go +++ b/azure/backend.go @@ -155,17 +155,25 @@ func (cs *aciContainerService) Run(ctx context.Context, r containers.ContainerCo Published: p.HostPort, }) } + + projectVolumes, serviceConfigVolumes, err := convert.GetRunVolumes(r.Volumes) + if err != nil { + return err + } + project := compose.Project{ Name: r.ID, Config: types.Config{ Services: []types.ServiceConfig{ { - Name: singleContainerName, - Image: r.Image, - Ports: ports, - Labels: r.Labels, + Name: singleContainerName, + Image: r.Image, + Ports: ports, + Labels: r.Labels, + Volumes: serviceConfigVolumes, }, }, + Volumes: projectVolumes, }, } diff --git a/azure/convert/volume.go b/azure/convert/volume.go new file mode 100644 index 000000000..1a6598f61 --- /dev/null +++ b/azure/convert/volume.go @@ -0,0 +1,110 @@ +package convert + +import ( + "errors" + "fmt" + "net/url" + "path/filepath" + "strings" + + "github.com/compose-spec/compose-go/types" +) + +// GetRunVolumes return volume configurations for a project and a single service +// this is meant to be used as a compose project of a single service +func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.ServiceVolumeConfig, error) { + var serviceConfigVolumes []types.ServiceVolumeConfig + projectVolumes := make(map[string]types.VolumeConfig, len(volumes)) + for i, v := range volumes { + var vi volumeInput + err := vi.parse(fmt.Sprintf("volume-%d", i), v) + if err != nil { + return nil, nil, err + } + projectVolumes[vi.name] = types.VolumeConfig{ + Name: vi.name, + Driver: azureFileDriverName, + DriverOpts: map[string]string{ + volumeDriveroptsAccountNameKey: vi.username, + volumeDriveroptsAccountKeyKey: vi.key, + volumeDriveroptsShareNameKey: vi.share, + }, + } + sv := types.ServiceVolumeConfig{ + Type: azureFileDriverName, + Source: vi.name, + Target: vi.target, + } + serviceConfigVolumes = append(serviceConfigVolumes, sv) + } + + return projectVolumes, serviceConfigVolumes, nil +} + +type volumeInput struct { + name string + username string + key string + share string + target string +} + +func scapeKeySlashes(rawURL string) (string, error) { + urlSplit := strings.Split(rawURL, "@") + if len(urlSplit) < 1 { + return "", errors.New("invalid url format " + rawURL) + } + userPasswd := strings.ReplaceAll(urlSplit[0], "/", "_") + scaped := userPasswd + rawURL[strings.Index(rawURL, "@"):] + + return scaped, nil +} + +func unscapeKey(passwd string) string { + return strings.ReplaceAll(passwd, "_", "/") +} + +// Removes the second ':' that separates the source from target +func volumeURL(pathURL string) (*url.URL, error) { + scapedURL, err := scapeKeySlashes(pathURL) + if err != nil { + return nil, err + } + pathURL = "//" + scapedURL + + count := strings.Count(pathURL, ":") + if count > 2 { + return nil, fmt.Errorf("unable to parse volume mount %q", pathURL) + } + if count == 2 { + tokens := strings.Split(pathURL, ":") + pathURL = fmt.Sprintf("%s:%s%s", tokens[0], tokens[1], tokens[2]) + } + return url.Parse(pathURL) +} + +func (v *volumeInput) parse(name string, s string) error { + volumeURL, err := volumeURL(s) + if err != nil { + return fmt.Errorf("volume specification %q could not be parsed %q", s, err) + } + v.username = volumeURL.User.Username() + if v.username == "" { + return fmt.Errorf("volume specification %q does not include a storage username", v) + } + passwd, ok := volumeURL.User.Password() + if !ok || passwd == "" { + return fmt.Errorf("volume specification %q does not include a storage key", v) + } + v.key = unscapeKey(passwd) + v.share = volumeURL.Host + if v.share == "" { + return fmt.Errorf("volume specification %q does not include a storage file share", v) + } + v.name = name + v.target = volumeURL.Path + if v.target == "" { + v.target = filepath.Join("/run/volumes/", v.share) + } + return nil +} diff --git a/cli/cmd/run/opts.go b/cli/cmd/run/opts.go new file mode 100644 index 000000000..b1dcb86e9 --- /dev/null +++ b/cli/cmd/run/opts.go @@ -0,0 +1,54 @@ +package run + +import ( + "fmt" + "strconv" + "strings" + + "github.com/docker/api/containers" +) + +type runOpts struct { + name string + publish []string + volumes []string +} + +func toPorts(ports []string) ([]containers.Port, error) { + var result []containers.Port + + for _, port := range ports { + parts := strings.Split(port, ":") + if len(parts) != 2 { + return nil, fmt.Errorf("unable to parse ports %q", port) + } + source, err := strconv.Atoi(parts[0]) + if err != nil { + return nil, err + } + destination, err := strconv.Atoi(parts[1]) + if err != nil { + return nil, err + } + + result = append(result, containers.Port{ + HostPort: uint32(source), + ContainerPort: uint32(destination), + }) + } + return result, nil +} + +func (r *runOpts) toContainerConfig(image string) (containers.ContainerConfig, error) { + publish, err := toPorts(r.publish) + if err != nil { + return containers.ContainerConfig{}, err + } + + return containers.ContainerConfig{ + ID: r.name, + Image: image, + Ports: publish, + Volumes: r.volumes, + }, nil +} diff --git a/cli/cmd/run/run.go b/cli/cmd/run/run.go index 369b4ebd6..5b448f875 100644 --- a/cli/cmd/run/run.go +++ b/cli/cmd/run/run.go @@ -54,6 +54,7 @@ func Command() *cobra.Command { cmd.Flags().StringArrayVarP(&opts.Publish, "publish", "p", []string{}, "Publish a container's port(s). [HOST_PORT:]CONTAINER_PORT") cmd.Flags().StringVar(&opts.Name, "name", getRandomName(), "Assign a name to the container") cmd.Flags().StringArrayVarP(&opts.Labels, "label", "l", []string{}, "Set meta data on a container") + cmd.Flags().StringArrayVarP(&opts.Volumes, "volume", "v", []string{}, "Volume. Ex: user:key@my_share:/absolute/path/to/target") return cmd } @@ -64,18 +65,17 @@ func runRun(ctx context.Context, image string, opts run.Opts) error { return err } - project, err := opts.ToContainerConfig(image) + containerConfig, err := opts.ToContainerConfig(image) if err != nil { return err } - if err = c.ContainerService().Run(ctx, project); err != nil { + if err = c.ContainerService().Run(ctx, containerConfig); err != nil { return err } fmt.Println(opts.Name) return nil - } func getRandomName() string { diff --git a/cli/options/run/opts.go b/cli/options/run/opts.go index 73ce79830..168a4c63c 100644 --- a/cli/options/run/opts.go +++ b/cli/options/run/opts.go @@ -15,6 +15,7 @@ type Opts struct { Name string Publish []string Labels []string + Volumes []string } // ToContainerConfig convert run options to a container configuration diff --git a/containers/api.go b/containers/api.go index 741adee85..3f345ddd0 100644 --- a/containers/api.go +++ b/containers/api.go @@ -26,7 +26,7 @@ type Port struct { HostPort uint32 // ContainerPort is the port number inside the container ContainerPort uint32 - /// Protocol is the protocol of the port mapping + // Protocol is the protocol of the port mapping Protocol string // HostIP is the host ip to use HostIP string @@ -42,6 +42,8 @@ type ContainerConfig struct { Ports []Port // Labels set labels to the container Labels map[string]string + // Volumes to be mounted + Volumes []string } // LogsRequest contains configuration about a log request From d28e5fd7421234b9f5c741939379b0e4dd5c6b06 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 11:29:48 +0200 Subject: [PATCH 2/7] Add e2e-aci tests for volumes Signed-off-by: Ulysses Souza --- azure/aci.go | 14 +++-- azure/storage/storage.go | 76 +++++++++++++++++++++++ azure/storage/storage_test.go | 67 ++++++++++++++++++++ cli/options/run/opts.go | 9 +-- go.mod | 3 + go.sum | 16 +++++ tests/aci-e2e/e2e-aci.go | 112 +++++++++++++++++++++++++++++----- 7 files changed, 273 insertions(+), 24 deletions(-) create mode 100644 azure/storage/storage.go create mode 100644 azure/storage/storage_test.go diff --git a/azure/aci.go b/azure/aci.go index 79907d3b1..d9fd89732 100644 --- a/azure/aci.go +++ b/azure/aci.go @@ -257,11 +257,14 @@ func getContainerClient(subscriptionID string) (containerinstance.ContainerClien return containerClient, nil } -func getSubscriptionsClient() subscription.SubscriptionsClient { +func getSubscriptionsClient() (subscription.SubscriptionsClient, error) { subc := subscription.NewSubscriptionsClient() - authorizer, _ := login.NewAuthorizerFromLogin() + authorizer, err := login.NewAuthorizerFromLogin() + if err != nil { + return subscription.SubscriptionsClient{}, err + } subc.Authorizer = authorizer - return subc + return subc, nil } // GetGroupsClient ... @@ -274,7 +277,10 @@ func GetGroupsClient(subscriptionID string) resources.GroupsClient { // GetSubscriptionID ... func GetSubscriptionID(ctx context.Context) (string, error) { - c := getSubscriptionsClient() + c, err := getSubscriptionsClient() + if err != nil { + return "", err + } res, err := c.List(ctx) if err != nil { return "", err diff --git a/azure/storage/storage.go b/azure/storage/storage.go new file mode 100644 index 000000000..8d616db95 --- /dev/null +++ b/azure/storage/storage.go @@ -0,0 +1,76 @@ +package storage + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/to" + + "github.com/docker/api/azure/login" + "github.com/docker/api/context/store" +) + +// CreateStorageAccount creates a new storage account. +func CreateStorageAccount(ctx context.Context, aciContext store.AciContext, accountName string) (storage.Account, error) { + storageAccountsClient := getStorageAccountsClient(aciContext) + result, err := storageAccountsClient.CheckNameAvailability( + ctx, + storage.AccountCheckNameAvailabilityParameters{ + Name: to.StringPtr(accountName), + Type: to.StringPtr("Microsoft.Storage/storageAccounts"), + }) + + if err != nil { + return storage.Account{}, err + } + if !*result.NameAvailable { + return storage.Account{}, err + } + + future, err := storageAccountsClient.Create( + ctx, + aciContext.ResourceGroup, + accountName, + storage.AccountCreateParameters{ + Sku: &storage.Sku{ + Name: storage.StandardLRS, + }, + Location: to.StringPtr(aciContext.Location), + AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{}}) + if err != nil { + return storage.Account{}, err + } + err = future.WaitForCompletionRef(ctx, storageAccountsClient.Client) + if err != nil { + return storage.Account{}, err + } + return future.Result(storageAccountsClient) +} + +// DeleteStorageAccount deletes a given storage account +func DeleteStorageAccount(ctx context.Context, aciContext store.AciContext, accountName string) (autorest.Response, error) { + storageAccountsClient := getStorageAccountsClient(aciContext) + response, err := storageAccountsClient.Delete(ctx, aciContext.ResourceGroup, accountName) + if err != nil { + return autorest.Response{}, err + } + return response, err +} + +// ListKeys lists the storage account keys +func ListKeys(ctx context.Context, aciContext store.AciContext, accountName string) (storage.AccountListKeysResult, error) { + storageAccountsClient := getStorageAccountsClient(aciContext) + keys, err := storageAccountsClient.ListKeys(ctx, aciContext.ResourceGroup, accountName) + if err != nil { + return storage.AccountListKeysResult{}, err + } + return keys, nil +} + +func getStorageAccountsClient(aciContext store.AciContext) storage.AccountsClient { + storageAccountsClient := storage.NewAccountsClient(aciContext.SubscriptionID) + autho, _ := login.NewAuthorizerFromLogin() + storageAccountsClient.Authorizer = autho + return storageAccountsClient +} diff --git a/azure/storage/storage_test.go b/azure/storage/storage_test.go new file mode 100644 index 000000000..3cfa43476 --- /dev/null +++ b/azure/storage/storage_test.go @@ -0,0 +1,67 @@ +package storage + +import ( + "context" + "fmt" + "net/url" + "testing" + + "github.com/Azure/azure-storage-file-go/azfile" + . "github.com/onsi/gomega" + + "github.com/docker/api/azure" + "github.com/docker/api/context/store" +) + +const ( + resourceGroupName = "rgulyssessouza" + location = "westeurope" + + testAccountName = "dockertestaccountname" + testShareName = "dockertestsharename" + testContent = "test content!" +) + +func TestGetContainerName(t *testing.T) { + RegisterTestingT(t) + + subscriptionID, err := azure.GetSubscriptionID(context.TODO()) + Expect(err).To(BeNil()) + aciContext := store.AciContext{ + SubscriptionID: subscriptionID, + Location: location, + ResourceGroup: resourceGroupName, + } + + storageAccount, err := CreateStorageAccount(context.TODO(), aciContext, testAccountName) + Expect(err).To(BeNil()) + Expect(*storageAccount.Name).To(Equal(testAccountName)) + + list, err := ListKeys(context.TODO(), aciContext, *storageAccount.Name) + Expect(err).To(BeNil()) + + firstKey := *(*list.Keys)[0].Value + + // Create a ShareURL object that wraps a soon-to-be-created share's URL and a default pipeline. + u, _ := url.Parse(fmt.Sprintf("https://%s.file.core.windows.net/%s", testAccountName, testShareName)) + credential, err := azfile.NewSharedKeyCredential(testAccountName, firstKey) + Expect(err).To(BeNil()) + + shareURL := azfile.NewShareURL(*u, azfile.NewPipeline(credential, azfile.PipelineOptions{})) + _, err = shareURL.Create(context.TODO(), azfile.Metadata{}, 0) + Expect(err).To(BeNil()) + + fURL, err := url.Parse(u.String() + "/testfile") + Expect(err).To(BeNil()) + fileURL := azfile.NewFileURL(*fURL, azfile.NewPipeline(credential, azfile.PipelineOptions{})) + err = azfile.UploadBufferToAzureFile(context.TODO(), []byte(testContent), fileURL, azfile.UploadToAzureFileOptions{}) + Expect(err).To(BeNil()) + + b := make([]byte, len(testContent)) + _, err = azfile.DownloadAzureFileToBuffer(context.TODO(), fileURL, b, azfile.DownloadFromAzureFileOptions{}) + Expect(err).To(BeNil()) + Expect(string(b)).To(Equal(testContent)) + + _, err = DeleteStorageAccount(context.TODO(), aciContext, testAccountName) + Expect(err).To(BeNil()) +} diff --git a/cli/options/run/opts.go b/cli/options/run/opts.go index 168a4c63c..af3bd9918 100644 --- a/cli/options/run/opts.go +++ b/cli/options/run/opts.go @@ -31,10 +31,11 @@ func (r *Opts) ToContainerConfig(image string) (containers.ContainerConfig, erro } return containers.ContainerConfig{ - ID: r.Name, - Image: image, - Ports: publish, - Labels: labels, + ID: r.Name, + Image: image, + Ports: publish, + Labels: labels, + Volumes: r.Volumes, }, nil } diff --git a/go.mod b/go.mod index 49230442c..88bf18b6a 100644 --- a/go.mod +++ b/go.mod @@ -3,10 +3,13 @@ module github.com/docker/api go 1.14 require ( + github.com/Azure/azure-pipeline-go v0.2.1 github.com/Azure/azure-sdk-for-go v42.0.0+incompatible + github.com/Azure/azure-storage-file-go v0.7.0 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 // indirect github.com/Azure/go-autorest/autorest v0.10.0 github.com/Azure/go-autorest/autorest/adal v0.8.2 + github.com/Azure/go-autorest/autorest/azure/auth v0.4.2 github.com/Azure/go-autorest/autorest/azure/cli v0.3.1 github.com/Azure/go-autorest/autorest/date v0.2.0 github.com/Azure/go-autorest/autorest/to v0.3.0 diff --git a/go.sum b/go.sum index 3c88bba00..98e43449f 100644 --- a/go.sum +++ b/go.sum @@ -1,15 +1,26 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= +github.com/Azure/azure-pipeline-go v0.2.1 h1:OLBdZJ3yvOn2MezlWvbrBMTEUQC72zAftRZOMdj5HYo= +github.com/Azure/azure-pipeline-go v0.2.1/go.mod h1:UGSo8XybXnIGZ3epmeBw7Jdz+HiUVpqIlpz/HKHylF4= github.com/Azure/azure-sdk-for-go v42.0.0+incompatible h1:yz6sFf5bHZ+gEOQVuK5JhPqTTAmv+OvSLSaqgzqaCwY= github.com/Azure/azure-sdk-for-go v42.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= +github.com/Azure/azure-storage-file-go v0.7.0 h1:yWoV0MYwzmoSgWACcVkdPolvAULFPNamcQLpIvS/Et4= +github.com/Azure/azure-storage-file-go v0.7.0/go.mod h1:3w3mufGcMjcOJ3w+4Gs+5wsSgkT7xDwWWqMMIrXtW4c= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= +github.com/Azure/go-autorest v1.1.1 h1:4G9tVCqooRY3vDTB2bA1Z01PlSALtnUbji0AfzthUSs= +github.com/Azure/go-autorest v14.1.0+incompatible h1:qROrS0rWxAXGfFdNOI33we8553d7T8v78jXf/8tjLBM= github.com/Azure/go-autorest/autorest v0.9.0/go.mod h1:xyHB1BMZT0cuDHU7I0+g046+BFDTQ8rEZB0s4Yfa6bI= +github.com/Azure/go-autorest/autorest v0.9.3/go.mod h1:GsRuLYvwzLjjjRoWEIyMUaYq8GNUx2nRB378IPt/1p0= github.com/Azure/go-autorest/autorest v0.10.0 h1:mvdtztBqcL8se7MdrUweNieTNi4kfNG6GOJuurQJpuY= github.com/Azure/go-autorest/autorest v0.10.0/go.mod h1:/FALq9T/kS7b5J5qsQ+RSTUdAmGFqi0vUdVNNx8q630= +github.com/Azure/go-autorest/autorest v0.10.1 h1:uaB8A32IZU9YKs9v50+/LWIWTDHJk2vlGzbfd7FfESI= github.com/Azure/go-autorest/autorest/adal v0.5.0/go.mod h1:8Z9fGy2MpX0PvDjB1pEgQTmVqjGhiHBW7RJJEciWzS0= github.com/Azure/go-autorest/autorest/adal v0.8.0/go.mod h1:Z6vX6WXXuyieHAXwMj0S6HY6e6wcHn37qQMBQlvY3lc= +github.com/Azure/go-autorest/autorest/adal v0.8.1/go.mod h1:ZjhuQClTqx435SRJ2iMlOxPYt3d2C/T/7TiQCVZSn3Q= github.com/Azure/go-autorest/autorest/adal v0.8.2 h1:O1X4oexUxnZCaEUGsvMnr8ZGj8HI37tNezwY4npRqA0= github.com/Azure/go-autorest/autorest/adal v0.8.2/go.mod h1:ZjhuQClTqx435SRJ2iMlOxPYt3d2C/T/7TiQCVZSn3Q= +github.com/Azure/go-autorest/autorest/azure/auth v0.4.2 h1:iM6UAvjR97ZIeR93qTcwpKNMpV+/FTWjwEbuPD495Tk= +github.com/Azure/go-autorest/autorest/azure/auth v0.4.2/go.mod h1:90gmfKdlmKgfjUpnCEpOJzsUEjrWDSLwHIG73tSXddM= github.com/Azure/go-autorest/autorest/azure/cli v0.3.1 h1:LXl088ZQlP0SBppGFsRZonW6hSvwgL5gRByMbvUbx8U= github.com/Azure/go-autorest/autorest/azure/cli v0.3.1/go.mod h1:ZG5p860J94/0kI9mNJVoIoLgXcirM2gF5i2kWloofxw= github.com/Azure/go-autorest/autorest/date v0.1.0/go.mod h1:plvfp3oPSKwf2DNjlBjWF/7vwR+cUD/ELuzDCXwHUVA= @@ -151,9 +162,12 @@ github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFB github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= +github.com/mattn/go-ieproxy v0.0.0-20190610004146-91bb50d98149 h1:HfxbT6/JcvIljmERptWhwa8XzP7H3T+Z2N26gTsaDaA= +github.com/mattn/go-ieproxy v0.0.0-20190610004146-91bb50d98149/go.mod h1:31jz6HNzdxOmlERGGEc4v/dMssOfmp2p5bT/okiKFFc= github.com/mattn/go-shellwords v1.0.10 h1:Y7Xqm8piKOO3v10Thp7Z36h4FYFjt5xB//6XvOrs2Gw= github.com/mattn/go-shellwords v1.0.10/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= @@ -270,6 +284,7 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200425230154-ff2c4b7c35a0 h1:Jcxah/M+oLZ/R4/z5RzfPzGbPXnVDPkEDtf2JnuxN+U= golang.org/x/net v0.0.0-20200425230154-ff2c4b7c35a0/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs= @@ -288,6 +303,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200122134326-e047566fdf82/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884= diff --git a/tests/aci-e2e/e2e-aci.go b/tests/aci-e2e/e2e-aci.go index 179549bf8..5c59c9f8c 100644 --- a/tests/aci-e2e/e2e-aci.go +++ b/tests/aci-e2e/e2e-aci.go @@ -2,14 +2,22 @@ package main import ( "context" + "fmt" "log" + "net/url" "strings" "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" "github.com/Azure/go-autorest/autorest/to" + + azure_storage "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/storage/mgmt/storage" + "github.com/Azure/azure-storage-file-go/azfile" + . "github.com/onsi/gomega" "github.com/docker/api/azure" + "github.com/docker/api/azure/storage" + "github.com/docker/api/context/store" . "github.com/docker/api/tests/framework" ) @@ -44,9 +52,11 @@ func main() { Expect(output).To(ContainSubstring("default *")) }) + var subscriptionID string It("creates a new aci context for tests", func() { setupTestResourceGroup(resourceGroupName) - subscriptionID, err := azure.GetSubscriptionID(context.TODO()) + var err error + subscriptionID, err = azure.GetSubscriptionID(context.TODO()) Expect(err).To(BeNil()) NewDockerCommand("context", "create", contextName, "aci", "--aci-subscription-id", subscriptionID, "--aci-resource-group", resourceGroupName, "--aci-location", location).ExecOrDie() @@ -68,21 +78,7 @@ func main() { }) It("runs nginx on port 80", func() { - output := NewDockerCommand("run", "nginx", "-p", "80:80", "--name", testContainerName).ExecOrDie() - Expect(output).To(Equal(testContainerName + "\n")) - output = NewDockerCommand("ps").ExecOrDie() - lines := Lines(output) - Expect(len(lines)).To(Equal(2)) - - containerFields := Columns(lines[1]) - Expect(containerFields[1]).To(Equal("nginx")) - Expect(containerFields[2]).To(Equal("Running")) - exposedIP := containerFields[3] - Expect(exposedIP).To(ContainSubstring(":80->80/tcp")) - - url := strings.ReplaceAll(exposedIP, "->80/tcp", "") - output = NewCommand("curl", url).ExecOrDie() - Expect(output).To(ContainSubstring("Welcome to nginx!")) + runTest(subscriptionID) }) It("removes container nginx", func() { @@ -136,6 +132,90 @@ func main() { }) } +const ( + testStorageAccountName = "dockertestaccountname" + testShareName = "dockertestsharename" + testFileContent = "Volume mounted with success!" + testFileName = "index.html" +) + +func createStorageAccount(aciContext store.AciContext, accountName string) azure_storage.Account { + storageAccount, err := storage.CreateStorageAccount(context.TODO(), aciContext, accountName) + Expect(err).To(BeNil()) + Expect(*storageAccount.Name).To(Equal(accountName)) + return storageAccount +} + +func getStorageKeys(aciContext store.AciContext, storageAccountName string) []azure_storage.AccountKey { + list, err := storage.ListKeys(context.TODO(), aciContext, storageAccountName) + Expect(err).To(BeNil()) + Expect(list.Keys).ToNot(BeNil()) + Expect(len(*list.Keys)).To(BeNumerically(">", 0)) + + return *list.Keys +} + +func deleteStorageAccount(aciContext store.AciContext) { + _, err := storage.DeleteStorageAccount(context.TODO(), aciContext, testStorageAccountName) + Expect(err).To(BeNil()) +} + +func createFileShare(key, shareName string) (azfile.SharedKeyCredential, url.URL) { + // Create a ShareURL object that wraps a soon-to-be-created share's URL and a default pipeline. + u, _ := url.Parse(fmt.Sprintf("https://%s.file.core.windows.net/%s", testStorageAccountName, shareName)) + credential, err := azfile.NewSharedKeyCredential(testStorageAccountName, key) + Expect(err).To(BeNil()) + + shareURL := azfile.NewShareURL(*u, azfile.NewPipeline(credential, azfile.PipelineOptions{})) + _, err = shareURL.Create(context.TODO(), azfile.Metadata{}, 0) + Expect(err).To(BeNil()) + + return *credential, *u +} + +func uploadFile(credential azfile.SharedKeyCredential, baseURL, fileName, fileContent string) { + fURL, err := url.Parse(baseURL + "/" + fileName) + Expect(err).To(BeNil()) + fileURL := azfile.NewFileURL(*fURL, azfile.NewPipeline(&credential, azfile.PipelineOptions{})) + err = azfile.UploadBufferToAzureFile(context.TODO(), []byte(testFileContent), fileURL, azfile.UploadToAzureFileOptions{}) + Expect(err).To(BeNil()) +} + +func runTest(subscriptionID string) { + aciContext := store.AciContext{ + SubscriptionID: subscriptionID, + Location: location, + ResourceGroup: resourceGroupName, + } + createStorageAccount(aciContext, testStorageAccountName) + defer deleteStorageAccount(aciContext) + keys := getStorageKeys(aciContext, testStorageAccountName) + firstKey := *keys[0].Value + credential, u := createFileShare(firstKey, testShareName) + uploadFile(credential, u.String(), testFileName, testFileContent) + + mountTarget := "/usr/share/nginx/html" + output := NewDockerCommand("run", "nginx", + "-v", fmt.Sprintf("%s:%s@%s:%s", + testStorageAccountName, firstKey, testShareName, mountTarget), + "-p", "80:80", + "--name", testContainerName).ExecOrDie() + Expect(output).To(Equal(testContainerName + "\n")) + output = NewDockerCommand("ps").ExecOrDie() + lines := Lines(output) + Expect(len(lines)).To(Equal(2)) + + containerFields := Columns(lines[1]) + Expect(containerFields[1]).To(Equal("nginx")) + Expect(containerFields[2]).To(Equal("Running")) + exposedIP := containerFields[3] + Expect(exposedIP).To(ContainSubstring(":80->80/tcp")) + + publishedURL := strings.ReplaceAll(exposedIP, "->80/tcp", "") + output = NewCommand("curl", publishedURL).ExecOrDie() + Expect(output).To(ContainSubstring(testFileContent)) +} + func setupTestResourceGroup(groupName string) { log.Println("Creating resource group " + resourceGroupName) ctx := context.TODO() From bdd987f246b72c86fd0520d9cd3a45200fead802 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 15:26:59 +0200 Subject: [PATCH 3/7] Refactor placement and method name Signed-off-by: Ulysses Souza --- azure/convert/volume.go | 14 ++--- azure/storage/storage_test.go | 67 --------------------- tests/aci-e2e/e2e-aci.go | 2 +- {azure => tests/aci-e2e}/storage/storage.go | 0 4 files changed, 8 insertions(+), 75 deletions(-) delete mode 100644 azure/storage/storage_test.go rename {azure => tests/aci-e2e}/storage/storage.go (100%) diff --git a/azure/convert/volume.go b/azure/convert/volume.go index 1a6598f61..0368aa374 100644 --- a/azure/convert/volume.go +++ b/azure/convert/volume.go @@ -49,7 +49,7 @@ type volumeInput struct { target string } -func scapeKeySlashes(rawURL string) (string, error) { +func escapeKeySlashes(rawURL string) (string, error) { urlSplit := strings.Split(rawURL, "@") if len(urlSplit) < 1 { return "", errors.New("invalid url format " + rawURL) @@ -60,13 +60,13 @@ func scapeKeySlashes(rawURL string) (string, error) { return scaped, nil } -func unscapeKey(passwd string) string { - return strings.ReplaceAll(passwd, "_", "/") +func unescapeKey(key string) string { + return strings.ReplaceAll(key, "_", "/") } // Removes the second ':' that separates the source from target func volumeURL(pathURL string) (*url.URL, error) { - scapedURL, err := scapeKeySlashes(pathURL) + scapedURL, err := escapeKeySlashes(pathURL) if err != nil { return nil, err } @@ -92,11 +92,11 @@ func (v *volumeInput) parse(name string, s string) error { if v.username == "" { return fmt.Errorf("volume specification %q does not include a storage username", v) } - passwd, ok := volumeURL.User.Password() - if !ok || passwd == "" { + key, ok := volumeURL.User.Password() + if !ok || key == "" { return fmt.Errorf("volume specification %q does not include a storage key", v) } - v.key = unscapeKey(passwd) + v.key = unescapeKey(key) v.share = volumeURL.Host if v.share == "" { return fmt.Errorf("volume specification %q does not include a storage file share", v) diff --git a/azure/storage/storage_test.go b/azure/storage/storage_test.go deleted file mode 100644 index 3cfa43476..000000000 --- a/azure/storage/storage_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package storage - -import ( - "context" - "fmt" - "net/url" - "testing" - - "github.com/Azure/azure-storage-file-go/azfile" - . "github.com/onsi/gomega" - - "github.com/docker/api/azure" - "github.com/docker/api/context/store" -) - -const ( - resourceGroupName = "rgulyssessouza" - location = "westeurope" - - testAccountName = "dockertestaccountname" - testShareName = "dockertestsharename" - testContent = "test content!" -) - -func TestGetContainerName(t *testing.T) { - RegisterTestingT(t) - - subscriptionID, err := azure.GetSubscriptionID(context.TODO()) - Expect(err).To(BeNil()) - aciContext := store.AciContext{ - SubscriptionID: subscriptionID, - Location: location, - ResourceGroup: resourceGroupName, - } - - storageAccount, err := CreateStorageAccount(context.TODO(), aciContext, testAccountName) - Expect(err).To(BeNil()) - Expect(*storageAccount.Name).To(Equal(testAccountName)) - - list, err := ListKeys(context.TODO(), aciContext, *storageAccount.Name) - Expect(err).To(BeNil()) - - firstKey := *(*list.Keys)[0].Value - - // Create a ShareURL object that wraps a soon-to-be-created share's URL and a default pipeline. - u, _ := url.Parse(fmt.Sprintf("https://%s.file.core.windows.net/%s", testAccountName, testShareName)) - credential, err := azfile.NewSharedKeyCredential(testAccountName, firstKey) - Expect(err).To(BeNil()) - - shareURL := azfile.NewShareURL(*u, azfile.NewPipeline(credential, azfile.PipelineOptions{})) - _, err = shareURL.Create(context.TODO(), azfile.Metadata{}, 0) - Expect(err).To(BeNil()) - - fURL, err := url.Parse(u.String() + "/testfile") - Expect(err).To(BeNil()) - fileURL := azfile.NewFileURL(*fURL, azfile.NewPipeline(credential, azfile.PipelineOptions{})) - err = azfile.UploadBufferToAzureFile(context.TODO(), []byte(testContent), fileURL, azfile.UploadToAzureFileOptions{}) - Expect(err).To(BeNil()) - - b := make([]byte, len(testContent)) - _, err = azfile.DownloadAzureFileToBuffer(context.TODO(), fileURL, b, azfile.DownloadFromAzureFileOptions{}) - Expect(err).To(BeNil()) - Expect(string(b)).To(Equal(testContent)) - - _, err = DeleteStorageAccount(context.TODO(), aciContext, testAccountName) - Expect(err).To(BeNil()) -} diff --git a/tests/aci-e2e/e2e-aci.go b/tests/aci-e2e/e2e-aci.go index 5c59c9f8c..cec3edfba 100644 --- a/tests/aci-e2e/e2e-aci.go +++ b/tests/aci-e2e/e2e-aci.go @@ -16,8 +16,8 @@ import ( . "github.com/onsi/gomega" "github.com/docker/api/azure" - "github.com/docker/api/azure/storage" "github.com/docker/api/context/store" + "github.com/docker/api/tests/aci-e2e/storage" . "github.com/docker/api/tests/framework" ) diff --git a/azure/storage/storage.go b/tests/aci-e2e/storage/storage.go similarity index 100% rename from azure/storage/storage.go rename to tests/aci-e2e/storage/storage.go From b68c019e93c524d488548fa331db2663fd697ddb Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 15:50:57 +0200 Subject: [PATCH 4/7] Implement ErrParsingFailed error Signed-off-by: Ulysses Souza --- azure/aci.go | 7 ++++--- azure/convert/volume.go | 15 +++++++++------ errdefs/errors.go | 2 ++ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/azure/aci.go b/azure/aci.go index d9fd89732..9d2cf58fd 100644 --- a/azure/aci.go +++ b/azure/aci.go @@ -9,8 +9,6 @@ import ( "strings" "time" - "github.com/docker/api/azure/login" - "github.com/Azure/azure-sdk-for-go/profiles/2019-03-01/resources/mgmt/resources" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance" @@ -21,6 +19,9 @@ import ( "github.com/gobwas/ws/wsutil" "github.com/pkg/errors" + "github.com/docker/api/azure/login" + "github.com/docker/api/errdefs" + "github.com/docker/api/context/store" ) @@ -261,7 +262,7 @@ func getSubscriptionsClient() (subscription.SubscriptionsClient, error) { subc := subscription.NewSubscriptionsClient() authorizer, err := login.NewAuthorizerFromLogin() if err != nil { - return subscription.SubscriptionsClient{}, err + return subscription.SubscriptionsClient{}, errors.Wrap(errdefs.ErrLoginFailed, err.Error()) } subc.Authorizer = authorizer return subc, nil diff --git a/azure/convert/volume.go b/azure/convert/volume.go index 0368aa374..24d630531 100644 --- a/azure/convert/volume.go +++ b/azure/convert/volume.go @@ -1,13 +1,16 @@ package convert import ( - "errors" "fmt" "net/url" "path/filepath" "strings" + "github.com/pkg/errors" + "github.com/compose-spec/compose-go/types" + + "github.com/docker/api/errdefs" ) // GetRunVolumes return volume configurations for a project and a single service @@ -74,7 +77,7 @@ func volumeURL(pathURL string) (*url.URL, error) { count := strings.Count(pathURL, ":") if count > 2 { - return nil, fmt.Errorf("unable to parse volume mount %q", pathURL) + return nil, errors.Wrap(errdefs.ErrParsingFailed, fmt.Sprintf("unable to parse volume mount %q", pathURL)) } if count == 2 { tokens := strings.Split(pathURL, ":") @@ -86,20 +89,20 @@ func volumeURL(pathURL string) (*url.URL, error) { func (v *volumeInput) parse(name string, s string) error { volumeURL, err := volumeURL(s) if err != nil { - return fmt.Errorf("volume specification %q could not be parsed %q", s, err) + return errors.Wrap(errdefs.ErrParsingFailed, fmt.Sprintf("volume specification %q could not be parsed %q", s, err)) } v.username = volumeURL.User.Username() if v.username == "" { - return fmt.Errorf("volume specification %q does not include a storage username", v) + return errors.Wrap(errdefs.ErrParsingFailed, fmt.Sprintf("volume specification %q does not include a storage username", v)) } key, ok := volumeURL.User.Password() if !ok || key == "" { - return fmt.Errorf("volume specification %q does not include a storage key", v) + return errors.Wrap(errdefs.ErrParsingFailed, fmt.Sprintf("volume specification %q does not include a storage key", v)) } v.key = unescapeKey(key) v.share = volumeURL.Host if v.share == "" { - return fmt.Errorf("volume specification %q does not include a storage file share", v) + return errors.Wrap(errdefs.ErrParsingFailed, fmt.Sprintf("volume specification %q does not include a storage file share", v)) } v.name = name v.target = volumeURL.Path diff --git a/errdefs/errors.go b/errdefs/errors.go index be64d2114..6fae1e8f3 100644 --- a/errdefs/errors.go +++ b/errdefs/errors.go @@ -45,6 +45,8 @@ var ( // ErrNotImplemented is returned when a backend doesn't implement // an action ErrNotImplemented = errors.New("not implemented") + // ErrParsingFailed + ErrParsingFailed = errors.New("parsing failed") ) // IsNotFoundError returns true if the unwrapped error is ErrNotFound From 70bcdca2c27a39e2fb529d9567d9edfbfcc38c5b Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 19:45:03 +0200 Subject: [PATCH 5/7] Add tests to volume convertion Signed-off-by: Ulysses Souza --- azure/convert/volume.go | 10 ++- azure/convert/volume_test.go | 134 +++++++++++++++++++++++++++++++++++ errdefs/errors.go | 5 ++ 3 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 azure/convert/volume_test.go diff --git a/azure/convert/volume.go b/azure/convert/volume.go index 24d630531..0acafdf68 100644 --- a/azure/convert/volume.go +++ b/azure/convert/volume.go @@ -55,10 +55,16 @@ type volumeInput struct { func escapeKeySlashes(rawURL string) (string, error) { urlSplit := strings.Split(rawURL, "@") if len(urlSplit) < 1 { - return "", errors.New("invalid url format " + rawURL) + return "", errors.Wrap(errdefs.ErrParsingFailed, "invalid url format "+rawURL) } userPasswd := strings.ReplaceAll(urlSplit[0], "/", "_") - scaped := userPasswd + rawURL[strings.Index(rawURL, "@"):] + + atIndex := strings.Index(rawURL, "@") + if atIndex < 0 { + return "", errors.Wrap(errdefs.ErrParsingFailed, "no share specified in "+rawURL) + } + + scaped := userPasswd + rawURL[atIndex:] return scaped, nil } diff --git a/azure/convert/volume_test.go b/azure/convert/volume_test.go new file mode 100644 index 000000000..ff7adf39d --- /dev/null +++ b/azure/convert/volume_test.go @@ -0,0 +1,134 @@ +/* + Copyright (c) 2020 Docker Inc. + + Permission is hereby granted, free of charge, to any person + obtaining a copy of this software and associated documentation + files (the "Software"), to deal in the Software without + restriction, including without limitation the rights to use, copy, + modify, merge, publish, distribute, sublicense, and/or sell copies + of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be + included in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + EXPRESS OR IMPLIED, + INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + HOLDERS BE LIABLE FOR ANY CLAIM, + DAMAGES OR OTHER LIABILITY, + WHETHER IN AN ACTION OF CONTRACT, + TORT OR OTHERWISE, + ARISING FROM, OUT OF OR IN CONNECTION WITH + THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ + +package convert + +import ( + "testing" + + "github.com/compose-spec/compose-go/types" + "gotest.tools/assert" + + "github.com/docker/api/errdefs" +) + +const ( + storageAccountNameKey = "storage_account_name" + storageAccountKeyKey = "storage_account_key" + shareNameKey = "share_name" +) + +func TestGetRunVolumes(t *testing.T) { + volumeStrings := []string{ + "myuser1:mykey1@myshare1/my/path/to/target1", + "myuser2:mykey2@myshare2/my/path/to/target2", + "myuser3:mykey3@mydefaultsharename", // Use default placement at '/run/volumes/' + } + var goldenVolumeConfigs = map[string]types.VolumeConfig{ + "volume-0": { + Name: "volume-0", + Driver: "azure_file", + DriverOpts: map[string]string{ + storageAccountNameKey: "myuser1", + storageAccountKeyKey: "mykey1", + shareNameKey: "myshare1", + }, + }, + "volume-1": { + Name: "volume-1", + Driver: "azure_file", + DriverOpts: map[string]string{ + storageAccountNameKey: "myuser2", + storageAccountKeyKey: "mykey2", + shareNameKey: "myshare2", + }, + }, + "volume-2": { + Name: "volume-2", + Driver: "azure_file", + DriverOpts: map[string]string{ + storageAccountNameKey: "myuser3", + storageAccountKeyKey: "mykey3", + shareNameKey: "mydefaultsharename", + }, + }, + } + goldenServiceVolumeConfigs := []types.ServiceVolumeConfig{ + { + Type: "azure_file", + Source: "volume-0", + Target: "/my/path/to/target1", + }, + { + Type: "azure_file", + Source: "volume-1", + Target: "/my/path/to/target2", + }, + { + Type: "azure_file", + Source: "volume-2", + Target: "/run/volumes/mydefaultsharename", + }, + } + + volumeConfigs, serviceVolumeConfigs, err := GetRunVolumes(volumeStrings) + assert.NilError(t, err) + for k, v := range volumeConfigs { + assert.DeepEqual(t, goldenVolumeConfigs[k], v) + } + for i, v := range serviceVolumeConfigs { + assert.DeepEqual(t, goldenServiceVolumeConfigs[i], v) + } +} + +func TestGetRunVolumesMissingFileShare(t *testing.T) { + _, _, err := GetRunVolumes([]string{"myuser:mykey@"}) + assert.Equal(t, true, errdefs.IsErrParsingFailed(err)) + assert.ErrorContains(t, err, "does not include a storage file share") +} + +func TestGetRunVolumesMissingUser(t *testing.T) { + _, _, err := GetRunVolumes([]string{":mykey@myshare"}) + assert.Equal(t, true, errdefs.IsErrParsingFailed(err)) + assert.ErrorContains(t, err, "does not include a storage username") +} + +func TestGetRunVolumesMissingKey(t *testing.T) { + _, _, err := GetRunVolumes([]string{"userwithnokey:@myshare"}) + assert.Equal(t, true, errdefs.IsErrParsingFailed(err)) + assert.ErrorContains(t, err, "does not include a storage key") + + _, _, err = GetRunVolumes([]string{"userwithnokeytoo@myshare"}) + assert.Equal(t, true, errdefs.IsErrParsingFailed(err)) + assert.ErrorContains(t, err, "does not include a storage key") +} + +func TestGetRunVolumesNoShare(t *testing.T) { + _, _, err := GetRunVolumes([]string{"noshare"}) + assert.Equal(t, true, errdefs.IsErrParsingFailed(err)) + assert.ErrorContains(t, err, "no share specified") +} diff --git a/errdefs/errors.go b/errdefs/errors.go index 6fae1e8f3..436377e7f 100644 --- a/errdefs/errors.go +++ b/errdefs/errors.go @@ -73,3 +73,8 @@ func IsUnknownError(err error) bool { func IsErrNotImplemented(err error) bool { return errors.Is(err, ErrNotImplemented) } + +// IsErrParseFail returns true if the unwrapped error is ErrParsingFailed +func IsErrParsingFailed(err error) bool { + return errors.Is(err, ErrParsingFailed) +} From 762f462d8081e95ccd42532812f4f8e41e38775f Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 19:51:22 +0200 Subject: [PATCH 6/7] Get "run" test back to main function Signed-off-by: Ulysses Souza --- tests/aci-e2e/e2e-aci.go | 68 +++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/tests/aci-e2e/e2e-aci.go b/tests/aci-e2e/e2e-aci.go index cec3edfba..4097bf355 100644 --- a/tests/aci-e2e/e2e-aci.go +++ b/tests/aci-e2e/e2e-aci.go @@ -78,7 +78,38 @@ func main() { }) It("runs nginx on port 80", func() { - runTest(subscriptionID) + aciContext := store.AciContext{ + SubscriptionID: subscriptionID, + Location: location, + ResourceGroup: resourceGroupName, + } + createStorageAccount(aciContext, testStorageAccountName) + defer deleteStorageAccount(aciContext) + keys := getStorageKeys(aciContext, testStorageAccountName) + firstKey := *keys[0].Value + credential, u := createFileShare(firstKey, testShareName) + uploadFile(credential, u.String(), testFileName, testFileContent) + + mountTarget := "/usr/share/nginx/html" + output := NewDockerCommand("run", "nginx", + "-v", fmt.Sprintf("%s:%s@%s:%s", + testStorageAccountName, firstKey, testShareName, mountTarget), + "-p", "80:80", + "--name", testContainerName).ExecOrDie() + Expect(output).To(Equal(testContainerName + "\n")) + output = NewDockerCommand("ps").ExecOrDie() + lines := Lines(output) + Expect(len(lines)).To(Equal(2)) + + containerFields := Columns(lines[1]) + Expect(containerFields[1]).To(Equal("nginx")) + Expect(containerFields[2]).To(Equal("Running")) + exposedIP := containerFields[3] + Expect(exposedIP).To(ContainSubstring(":80->80/tcp")) + + publishedURL := strings.ReplaceAll(exposedIP, "->80/tcp", "") + output = NewCommand("curl", publishedURL).ExecOrDie() + Expect(output).To(ContainSubstring(testFileContent)) }) It("removes container nginx", func() { @@ -181,41 +212,6 @@ func uploadFile(credential azfile.SharedKeyCredential, baseURL, fileName, fileCo Expect(err).To(BeNil()) } -func runTest(subscriptionID string) { - aciContext := store.AciContext{ - SubscriptionID: subscriptionID, - Location: location, - ResourceGroup: resourceGroupName, - } - createStorageAccount(aciContext, testStorageAccountName) - defer deleteStorageAccount(aciContext) - keys := getStorageKeys(aciContext, testStorageAccountName) - firstKey := *keys[0].Value - credential, u := createFileShare(firstKey, testShareName) - uploadFile(credential, u.String(), testFileName, testFileContent) - - mountTarget := "/usr/share/nginx/html" - output := NewDockerCommand("run", "nginx", - "-v", fmt.Sprintf("%s:%s@%s:%s", - testStorageAccountName, firstKey, testShareName, mountTarget), - "-p", "80:80", - "--name", testContainerName).ExecOrDie() - Expect(output).To(Equal(testContainerName + "\n")) - output = NewDockerCommand("ps").ExecOrDie() - lines := Lines(output) - Expect(len(lines)).To(Equal(2)) - - containerFields := Columns(lines[1]) - Expect(containerFields[1]).To(Equal("nginx")) - Expect(containerFields[2]).To(Equal("Running")) - exposedIP := containerFields[3] - Expect(exposedIP).To(ContainSubstring(":80->80/tcp")) - - publishedURL := strings.ReplaceAll(exposedIP, "->80/tcp", "") - output = NewCommand("curl", publishedURL).ExecOrDie() - Expect(output).To(ContainSubstring(testFileContent)) -} - func setupTestResourceGroup(groupName string) { log.Println("Creating resource group " + resourceGroupName) ctx := context.TODO() From d2fece3311ca871c8f3de09d818dc64cbdf40e6e Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Tue, 19 May 2020 20:03:53 +0200 Subject: [PATCH 7/7] Fix linter problems Signed-off-by: Ulysses Souza --- cli/cmd/run/opts.go | 54 ---------------------------------------- errdefs/errors.go | 4 +-- tests/aci-e2e/e2e-aci.go | 2 +- 3 files changed, 3 insertions(+), 57 deletions(-) delete mode 100644 cli/cmd/run/opts.go diff --git a/cli/cmd/run/opts.go b/cli/cmd/run/opts.go deleted file mode 100644 index b1dcb86e9..000000000 --- a/cli/cmd/run/opts.go +++ /dev/null @@ -1,54 +0,0 @@ -package run - -import ( - "fmt" - "strconv" - "strings" - - "github.com/docker/api/containers" -) - -type runOpts struct { - name string - publish []string - volumes []string -} - -func toPorts(ports []string) ([]containers.Port, error) { - var result []containers.Port - - for _, port := range ports { - parts := strings.Split(port, ":") - if len(parts) != 2 { - return nil, fmt.Errorf("unable to parse ports %q", port) - } - source, err := strconv.Atoi(parts[0]) - if err != nil { - return nil, err - } - destination, err := strconv.Atoi(parts[1]) - if err != nil { - return nil, err - } - - result = append(result, containers.Port{ - HostPort: uint32(source), - ContainerPort: uint32(destination), - }) - } - return result, nil -} - -func (r *runOpts) toContainerConfig(image string) (containers.ContainerConfig, error) { - publish, err := toPorts(r.publish) - if err != nil { - return containers.ContainerConfig{}, err - } - - return containers.ContainerConfig{ - ID: r.name, - Image: image, - Ports: publish, - Volumes: r.volumes, - }, nil -} diff --git a/errdefs/errors.go b/errdefs/errors.go index 436377e7f..3a4e29843 100644 --- a/errdefs/errors.go +++ b/errdefs/errors.go @@ -45,7 +45,7 @@ var ( // ErrNotImplemented is returned when a backend doesn't implement // an action ErrNotImplemented = errors.New("not implemented") - // ErrParsingFailed + // ErrParsingFailed is returned when a string cannot be parsed ErrParsingFailed = errors.New("parsing failed") ) @@ -74,7 +74,7 @@ func IsErrNotImplemented(err error) bool { return errors.Is(err, ErrNotImplemented) } -// IsErrParseFail returns true if the unwrapped error is ErrParsingFailed +// IsErrParsingFailed returns true if the unwrapped error is ErrParsingFailed func IsErrParsingFailed(err error) bool { return errors.Is(err, ErrParsingFailed) } diff --git a/tests/aci-e2e/e2e-aci.go b/tests/aci-e2e/e2e-aci.go index 4097bf355..7eece0a7a 100644 --- a/tests/aci-e2e/e2e-aci.go +++ b/tests/aci-e2e/e2e-aci.go @@ -208,7 +208,7 @@ func uploadFile(credential azfile.SharedKeyCredential, baseURL, fileName, fileCo fURL, err := url.Parse(baseURL + "/" + fileName) Expect(err).To(BeNil()) fileURL := azfile.NewFileURL(*fURL, azfile.NewPipeline(&credential, azfile.PipelineOptions{})) - err = azfile.UploadBufferToAzureFile(context.TODO(), []byte(testFileContent), fileURL, azfile.UploadToAzureFileOptions{}) + err = azfile.UploadBufferToAzureFile(context.TODO(), []byte(fileContent), fileURL, azfile.UploadToAzureFileOptions{}) Expect(err).To(BeNil()) }