From e1cbea53cdbc5ae7133a18230f5a3fb534111838 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Fri, 25 Oct 2019 16:07:36 +0200 Subject: [PATCH 1/3] Add --label flag when running an application This will add the given labels to all the containers inside the application (except for the invocation image). The labels are passed over by file, this is to be able to easely add new parameters in the future and limit the explosion of parameters we have in our bundle Signed-off-by: Djordje Lukic --- cmd/cnab-run/install.go | 26 ++++++++++++++++++++++++++ internal/commands/image/list.go | 2 +- internal/commands/image/list_test.go | 6 ------ internal/commands/parameters.go | 18 ++++++++++++++++++ internal/commands/run.go | 11 +++++++---- internal/names.go | 4 ++++ internal/packager/cnab.go | 23 +++++++++++++++++++++++ 7 files changed, 79 insertions(+), 11 deletions(-) diff --git a/cmd/cnab-run/install.go b/cmd/cnab-run/install.go index 4556a6b9f..5dec88344 100644 --- a/cmd/cnab-run/install.go +++ b/cmd/cnab-run/install.go @@ -51,7 +51,11 @@ func installAction(instanceName string) error { if err != nil { return err } + if err = addLabels(rendered); err != nil { + return err + } addAppLabels(rendered, instanceName) + if err := os.Chdir(app.Path); err != nil { return err } @@ -87,6 +91,28 @@ func getBundleImageMap() (map[string]bundle.Image, error) { return result, nil } +func addLabels(rendered *composetypes.Config) error { + args, err := ioutil.ReadFile(internal.DockerArgsPath) + if err != nil { + return err + } + a := packager.DockerAppArgs{} + err = json.Unmarshal(args, &a) + if err != nil { + return err + } + for k, v := range a.Labels { + for i, service := range rendered.Services { + if service.Labels == nil { + service.Labels = map[string]string{} + } + service.Labels[k] = v + rendered.Services[i] = service + } + } + return nil +} + func addAppLabels(rendered *composetypes.Config, instanceName string) { for i, service := range rendered.Services { if service.Labels == nil { diff --git a/internal/commands/image/list.go b/internal/commands/image/list.go index c6725e1ac..e4feb0ef8 100644 --- a/internal/commands/image/list.go +++ b/internal/commands/image/list.go @@ -107,7 +107,7 @@ func printImageIDs(dockerCli command.Cli, refs []pkg) error { } fmt.Fprintln(&buf, stringid.TruncateID(id.String())) } - fmt.Fprintf(dockerCli.Out(), buf.String()) + fmt.Fprint(dockerCli.Out(), buf.String()) return nil } diff --git a/internal/commands/image/list_test.go b/internal/commands/image/list_test.go index 7193ef952..de4996820 100644 --- a/internal/commands/image/list_test.go +++ b/internal/commands/image/list_test.go @@ -14,12 +14,6 @@ import ( "github.com/docker/distribution/reference" ) -type mockRef string - -func (ref mockRef) String() string { - return string(ref) -} - type bundleStoreStubForListCmd struct { refMap map[reference.Reference]*bundle.Bundle // in order to keep the reference in the same order between tests diff --git a/internal/commands/parameters.go b/internal/commands/parameters.go index fa4d63b7f..f4559fdea 100644 --- a/internal/commands/parameters.go +++ b/internal/commands/parameters.go @@ -1,6 +1,7 @@ package commands import ( + "encoding/json" "fmt" "io" "os" @@ -8,6 +9,7 @@ import ( "github.com/deislabs/cnab-go/bundle" "github.com/docker/app/internal" + "github.com/docker/app/internal/packager" "github.com/docker/app/internal/store" "github.com/docker/app/types/parameters" cliopts "github.com/docker/cli/opts" @@ -45,6 +47,22 @@ func withCommandLineParameters(overrides []string) mergeBundleOpt { } } +func withLabels(labels []string) mergeBundleOpt { + return func(c *mergeBundleConfig) error { + l := packager.DockerAppArgs{ + Labels: cliopts.ConvertKVStringsToMap(labels), + } + out, err := json.Marshal(l) + if err != nil { + return err + } + if _, ok := c.bundle.Parameters[internal.ParameterArgs]; ok { + c.params[internal.ParameterArgs] = string(out) + } + return nil + } +} + func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt { return func(c *mergeBundleConfig) error { if _, ok := c.bundle.Definitions[internal.ParameterShareRegistryCredsName]; ok { diff --git a/internal/commands/run.go b/internal/commands/run.go index e4c55325e..508efcc61 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -25,6 +25,7 @@ type runOptions struct { kubeNamespace string stackName string cnabBundle string + labels []string } const longDescription = `Run an App from an App image.` @@ -59,10 +60,11 @@ func runCmd(dockerCli command.Cli) *cobra.Command { } opts.parametersOptions.addFlags(cmd.Flags()) opts.credentialOptions.addFlags(cmd.Flags()) - cmd.Flags().StringVar(&opts.orchestrator, "orchestrator", "", "Orchestrator to run on (swarm, kubernetes)") - cmd.Flags().StringVar(&opts.kubeNamespace, "namespace", "default", "Kubernetes namespace in which to run the App") - cmd.Flags().StringVar(&opts.stackName, "name", "", "Name of the running App") - cmd.Flags().StringVar(&opts.cnabBundle, "cnab-bundle-json", "", "Run a CNAB bundle instead of a Docker App image") + cmd.Flags().StringVar(&opts.orchestrator, "orchestrator", "", "Orchestrator to install on (swarm, kubernetes)") + cmd.Flags().StringVar(&opts.kubeNamespace, "namespace", "default", "Kubernetes namespace to install into") + cmd.Flags().StringVar(&opts.stackName, "name", "", "Assign a name to the installation") + cmd.Flags().StringVar(&opts.cnabBundle, "cnab-bundle-json", "", "Run a CNAB bundle instead of a Docker App") + cmd.Flags().StringArrayVar(&opts.labels, "label", nil, "Label to add to services") return cmd } @@ -130,6 +132,7 @@ func runBundle(dockerCli command.Cli, bndl *bundle.Bundle, opts runOptions, ref if err := mergeBundleParameters(installation, withFileParameters(opts.parametersFiles), withCommandLineParameters(opts.overrides), + withLabels(opts.labels), withOrchestratorParameters(opts.orchestrator, opts.kubeNamespace), withSendRegistryAuth(opts.sendRegistryAuth), ); err != nil { diff --git a/internal/names.go b/internal/names.go index 8ede3e56c..96a23d995 100644 --- a/internal/names.go +++ b/internal/names.go @@ -52,6 +52,8 @@ const ( ParameterRenderFormatName = Namespace + "render-format" // ParameterInspectFormatName is the name of the parameter containing the inspect format ParameterInspectFormatName = Namespace + "inspect-format" + // ParameterArgs is the name of the parameter containing labels to be applied to service containers + ParameterArgs = Namespace + "args" // ParameterShareRegistryCredsName is the name of the parameter which indicates if credentials should be shared ParameterShareRegistryCredsName = Namespace + "share-registry-creds" @@ -68,6 +70,8 @@ const ( // the inspect output format. DockerInspectFormatEnvVar = "DOCKER_INSPECT_FORMAT" + DockerArgsPath = "/cnab/app/args.json" + // CustomDockerAppName is the custom variable set by Docker App to // save custom informations CustomDockerAppName = "com.docker.app" diff --git a/internal/packager/cnab.go b/internal/packager/cnab.go index 5f131cbef..1f1c0efe2 100644 --- a/internal/packager/cnab.go +++ b/internal/packager/cnab.go @@ -23,11 +23,24 @@ type DockerAppCustom struct { Payload json.RawMessage `json:"payload,omitempty"` } +// DockerAppArgs represent the object passed to the invocation image +// by Docker App. +type DockerAppArgs struct { + // Labels are the labels to add to containers on run + Labels map[string]string `json:"labels,omitempty"` +} + // ToCNAB creates a CNAB bundle from an app package func ToCNAB(app *types.App, invocationImageName string) (*bundle.Bundle, error) { mapping := ExtractCNABParameterMapping(app.Parameters()) flatParameters := app.Parameters().Flatten() definitions := definition.Definitions{ + internal.ParameterArgs: { + Type: "string", + Default: "", + Title: "Labels", + Description: "Labels to apply to service containers", + }, internal.ParameterOrchestratorName: { Type: "string", Enum: []interface{}{ @@ -73,6 +86,16 @@ func ToCNAB(app *types.App, invocationImageName string) (*bundle.Bundle, error) }, } parameters := map[string]bundle.Parameter{ + internal.ParameterArgs: { + Destination: &bundle.Location{ + Path: internal.DockerArgsPath, + }, + ApplyTo: []string{ + "install", + "upgrade", + }, + Definition: internal.ParameterArgs, + }, internal.ParameterOrchestratorName: { Destination: &bundle.Location{ EnvironmentVariable: internal.DockerStackOrchestratorEnvVar, From 248d5ab21b66dbabccea4fcaf212fc635072b6c7 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Fri, 25 Oct 2019 17:03:54 +0200 Subject: [PATCH 2/3] Add e2e test for service container labels on run Signed-off-by: Djordje Lukic --- cmd/cnab-run/install.go | 5 ++-- e2e/commands_test.go | 25 +++++++++++++++++++ internal/commands/parameters.go | 5 ++++ internal/packager/cnab.go | 4 +-- internal/packager/testdata/bundle-json.golden | 16 ++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/cmd/cnab-run/install.go b/cmd/cnab-run/install.go index 5dec88344..3b5d96579 100644 --- a/cmd/cnab-run/install.go +++ b/cmd/cnab-run/install.go @@ -96,9 +96,8 @@ func addLabels(rendered *composetypes.Config) error { if err != nil { return err } - a := packager.DockerAppArgs{} - err = json.Unmarshal(args, &a) - if err != nil { + var a packager.DockerAppArgs + if err := json.Unmarshal(args, &a); err != nil { return err } for k, v := range a.Labels { diff --git a/e2e/commands_test.go b/e2e/commands_test.go index 57eb1f406..002e6510a 100644 --- a/e2e/commands_test.go +++ b/e2e/commands_test.go @@ -193,6 +193,31 @@ func TestRunOnlyOne(t *testing.T) { }) } +func TestRunWithLabels(t *testing.T) { + runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) { + cmd := info.configuredCmd + + contextPath := filepath.Join("testdata", "simple") + cmd.Command = dockerCli.Command("app", "build", "--tag", "myapp", contextPath) + icmd.RunCmd(cmd).Assert(t, icmd.Success) + + cmd.Command = dockerCli.Command("app", "run", "myapp", "--name", "myapp", "--label", "label.key=labelValue") + icmd.RunCmd(cmd).Assert(t, icmd.Success) + + services := []string{ + "myapp_db", "myapp_web", "myapp_api", + } + for _, service := range services { + fmt.Printf("%q", service) + cmd.Command = dockerCli.Command("inspect", service) + icmd.RunCmd(cmd).Assert(t, icmd.Expected{ + ExitCode: 0, + Out: `"label.key": "labelValue"`, + }) + } + }) +} + func TestDockerAppLifecycle(t *testing.T) { t.Run("withBindMounts", func(t *testing.T) { testDockerAppLifecycle(t, true) diff --git a/internal/commands/parameters.go b/internal/commands/parameters.go index f4559fdea..9c053de6f 100644 --- a/internal/commands/parameters.go +++ b/internal/commands/parameters.go @@ -49,6 +49,11 @@ func withCommandLineParameters(overrides []string) mergeBundleOpt { func withLabels(labels []string) mergeBundleOpt { return func(c *mergeBundleConfig) error { + for _, l := range labels { + if strings.HasPrefix(l, internal.Namespace) { + return errors.Errorf("labels cannot start with %q", internal.Namespace) + } + } l := packager.DockerAppArgs{ Labels: cliopts.ConvertKVStringsToMap(labels), } diff --git a/internal/packager/cnab.go b/internal/packager/cnab.go index 1f1c0efe2..27760cbba 100644 --- a/internal/packager/cnab.go +++ b/internal/packager/cnab.go @@ -38,8 +38,8 @@ func ToCNAB(app *types.App, invocationImageName string) (*bundle.Bundle, error) internal.ParameterArgs: { Type: "string", Default: "", - Title: "Labels", - Description: "Labels to apply to service containers", + Title: "Arguments", + Description: "Arguments that are passed by file to the invocation image", }, internal.ParameterOrchestratorName: { Type: "string", diff --git a/internal/packager/testdata/bundle-json.golden b/internal/packager/testdata/bundle-json.golden index d070f5049..c3ee233cd 100644 --- a/internal/packager/testdata/bundle-json.golden +++ b/internal/packager/testdata/bundle-json.golden @@ -51,6 +51,16 @@ "io.cnab.status": {} }, "parameters": { + "com.docker.app.args": { + "definition": "com.docker.app.args", + "applyTo": [ + "install", + "upgrade" + ], + "destination": { + "path": "/cnab/app/args.json" + } + }, "com.docker.app.inspect-format": { "definition": "com.docker.app.inspect-format", "applyTo": [ @@ -115,6 +125,12 @@ } }, "definitions": { + "com.docker.app.args": { + "default": "", + "description": "Arguments that are passed by file to the invocation image", + "title": "Arguments", + "type": "string" + }, "com.docker.app.inspect-format": { "default": "json", "description": "Output format for the inspect command", From 903f3c0f436197836a08a8de0372b57b4d1380b2 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Thu, 31 Oct 2019 11:21:28 +0100 Subject: [PATCH 3/3] Add unit test for adding labels Signed-off-by: Djordje Lukic --- e2e/commands_test.go | 1 - internal/commands/parameters_test.go | 41 ++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/e2e/commands_test.go b/e2e/commands_test.go index 002e6510a..652d4c18d 100644 --- a/e2e/commands_test.go +++ b/e2e/commands_test.go @@ -208,7 +208,6 @@ func TestRunWithLabels(t *testing.T) { "myapp_db", "myapp_web", "myapp_api", } for _, service := range services { - fmt.Printf("%q", service) cmd.Command = dockerCli.Command("inspect", service) icmd.RunCmd(cmd).Assert(t, icmd.Expected{ ExitCode: 0, diff --git a/internal/commands/parameters_test.go b/internal/commands/parameters_test.go index ccb1d91df..5c1f748ed 100644 --- a/internal/commands/parameters_test.go +++ b/internal/commands/parameters_test.go @@ -2,6 +2,8 @@ package commands import ( "bytes" + "encoding/json" + "fmt" "strings" "testing" @@ -9,6 +11,7 @@ import ( "github.com/deislabs/cnab-go/bundle/definition" "github.com/deislabs/cnab-go/claim" "github.com/docker/app/internal" + "github.com/docker/app/internal/packager" "github.com/docker/app/internal/store" "gotest.tools/assert" "gotest.tools/assert/cmp" @@ -264,3 +267,41 @@ func TestMergeBundleParameters(t *testing.T) { assert.ErrorContains(t, err, "invalid value for parameter") }) } + +func TestLabels(t *testing.T) { + expected := packager.DockerAppArgs{ + Labels: map[string]string{ + "label": "value", + }, + } + expectedStr, err := json.Marshal(expected) + assert.NilError(t, err) + + labels := []string{ + "label=value", + } + op := withLabels(labels) + + config := &mergeBundleConfig{ + bundle: &bundle.Bundle{ + Parameters: map[string]bundle.Parameter{ + internal.ParameterArgs: {}, + }, + }, + params: map[string]string{}, + } + err = op(config) + assert.NilError(t, err) + fmt.Println(config.params) + l := config.params[internal.ParameterArgs] + assert.Equal(t, l, string(expectedStr)) +} + +func TestInvalidLabels(t *testing.T) { + labels := []string{ + "com.docker.app.label=value", + } + op := withLabels(labels) + err := op(&mergeBundleConfig{}) + assert.ErrorContains(t, err, fmt.Sprintf("labels cannot start with %q", internal.Namespace)) +}