Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: add spec annotation flag #1046

Merged
merged 1 commit into from
Apr 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apis/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1751,6 +1751,11 @@ definitions:
x-nullable: true
additionalProperties:
type: "string"
SpecAnnotation:
description: "annotations send to runtime spec."
type: "object"
additionalProperties:
type: "string"

ContainerCreateResp:
description: "response returned by daemon when container create successfully"
Expand Down
5 changes: 5 additions & 0 deletions apis/types/container_config.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions cli/common_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,8 @@ func addCommonFlags(flagSet *pflag.FlagSet) *container {
// disk quota
flagSet.StringSliceVar(&c.diskQuota, "disk-quota", nil, "Set disk quota for container")

// additional runtime spec annotations
flagSet.StringSliceVar(&c.specAnnotation, "annotation", nil, "Additional annotation for runtime")

return c
}
47 changes: 35 additions & 12 deletions cli/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type container struct {
IntelRdtL3Cbm string
diskQuota []string
oomScoreAdj int64
specAnnotation []string

cgroupParent string

Expand Down Expand Up @@ -121,6 +122,11 @@ func (c *container) config() (*types.ContainerCreateConfig, error) {
return nil, err
}

specAnnotation, err := validateAnnotation(c.specAnnotation)
if err != nil {
return nil, err
}

var networkMode string
if len(c.networks) == 0 {
networkMode = "bridge"
Expand Down Expand Up @@ -194,18 +200,19 @@ func (c *container) config() (*types.ContainerCreateConfig, error) {
}
config := &types.ContainerCreateConfig{
ContainerConfig: types.ContainerConfig{
Tty: c.tty,
Env: c.env,
Entrypoint: strings.Fields(c.entrypoint),
WorkingDir: c.workdir,
User: c.user,
Hostname: strfmt.Hostname(c.hostname),
Labels: labels,
Rich: c.rich,
RichMode: c.richMode,
InitScript: c.initScript,
ExposedPorts: ports,
DiskQuota: diskQuota,
Tty: c.tty,
Env: c.env,
Entrypoint: strings.Fields(c.entrypoint),
WorkingDir: c.workdir,
User: c.user,
Hostname: strfmt.Hostname(c.hostname),
Labels: labels,
Rich: c.rich,
RichMode: c.richMode,
InitScript: c.initScript,
ExposedPorts: ports,
DiskQuota: diskQuota,
SpecAnnotation: specAnnotation,
},

HostConfig: &types.HostConfig{
Expand Down Expand Up @@ -461,3 +468,19 @@ func validateOOMScore(score int64) error {

return nil
}

// validateAnnotation validates runtime annotations format.
func validateAnnotation(annotations []string) (map[string]string, error) {
specAnnotation := make(map[string]string)

for _, annotation := range annotations {
splits := strings.Split(annotation, "=")
if len(splits) != 2 || splits[0] == "" || splits[1] == "" {
return nil, fmt.Errorf("invalid format for spec annotation: %s, correct format should be key=value, neither should be nil", annotation)
}

specAnnotation[splits[0]] = splits[1]
}

return specAnnotation, nil
}
12 changes: 12 additions & 0 deletions daemon/mgr/spec_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package mgr
import (
"context"
"strconv"

"github.com/sirupsen/logrus"
)

// setupAnnotations extracts other related options from HostConfig and locate them in spec's annotations which will be dealt by vendored runc.
Expand All @@ -25,5 +27,15 @@ func setupAnnotations(ctx context.Context, meta *ContainerMeta, spec *SpecWrappe

s.Annotations["__schedule_latency_switch"] = strconv.FormatInt(r.ScheLatSwitch, 10)

// add additional spec annotations
annotations := meta.Config.SpecAnnotation
for k, v := range annotations {
if _, exist := s.Annotations[k]; exist {
logrus.Warnf("Duplicate spec annotation: %s=%s", k, v)
continue
}
s.Annotations[k] = v
}

return nil
}
24 changes: 24 additions & 0 deletions test/cli_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package main

import (
"encoding/json"
"fmt"
"runtime"
"strings"

"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/test/command"
"github.com/alibaba/pouch/test/environment"
"github.com/alibaba/pouch/test/util"

"github.com/go-check/check"
"github.com/gotestyourself/gotestyourself/icmd"
Expand Down Expand Up @@ -410,3 +412,25 @@ func (suite *PouchCreateSuite) TestCreateWithOOMOption(c *check.C) {
c.Assert(result.HostConfig.OomScoreAdj, check.Equals, int64(100))
c.Assert(*result.HostConfig.OomKillDisable, check.Equals, true)
}

// TestCreateWithAnnotation tests creating container with annotation.
func (suite *PouchCreateSuite) TestCreateWithAnnotation(c *check.C) {
cname := "TestCreateWithAnnotation"
command.PouchRun("create", "--annotation", "a=b", "--annotation", "foo=bar", "--name", cname, busyboxImage).Stdout()

output := command.PouchRun("inspect", cname).Stdout()
result := &types.ContainerJSON{}
if err := json.Unmarshal([]byte(output), result); err != nil {
c.Errorf("failed to decode inspect output: %v", err)
}

// kv in map not in order.
var annotationSlice []string
for k, v := range result.Config.SpecAnnotation {
annotationSlice = append(annotationSlice, fmt.Sprintf("%s=%s", k, v))
}
annotationStr := strings.Join(annotationSlice, " ")

c.Assert(util.PartialEqual(annotationStr, "a=b"), check.IsNil)
c.Assert(util.PartialEqual(annotationStr, "foo=bar"), check.IsNil)
}
23 changes: 23 additions & 0 deletions test/cli_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/test/command"
"github.com/alibaba/pouch/test/environment"
"github.com/alibaba/pouch/test/util"

"github.com/go-check/check"
"github.com/gotestyourself/gotestyourself/icmd"
Expand Down Expand Up @@ -789,3 +790,25 @@ func (suite *PouchRunSuite) TestRunWithDiskQuota(c *check.C) {

c.Assert(found, check.Equals, true)
}

// TestRunWithAnnotation is to verify the valid running container with annotation, and verify SpecAnnotation filed has been in inspect output.
func (suite *PouchRunSuite) TestRunWithAnnotation(c *check.C) {
cname := "TestRunWithAnnotation"
command.PouchRun("run", "-d", "--annotation", "a=b", "--annotation", "foo=bar", "--name", cname, busyboxImage).Stdout()

output := command.PouchRun("inspect", cname).Stdout()
result := &types.ContainerJSON{}
if err := json.Unmarshal([]byte(output), result); err != nil {
c.Errorf("failed to decode inspect output: %v", err)
}

// kv in map not in order.
var annotationSlice []string
for k, v := range result.Config.SpecAnnotation {
annotationSlice = append(annotationSlice, fmt.Sprintf("%s=%s", k, v))
}
annotationStr := strings.Join(annotationSlice, " ")

c.Assert(util.PartialEqual(annotationStr, "a=b"), check.IsNil)
c.Assert(util.PartialEqual(annotationStr, "foo=bar"), check.IsNil)
}
11 changes: 11 additions & 0 deletions test/cli_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,14 @@ func (suite *PouchStartSuite) TestStartWithPrivilege(c *check.C) {

command.PouchRun("start", name).Assert(c, icmd.Success)
}

// TestStartWithAnnotation starts a container with annotation.
func (suite *PouchStartSuite) TestStartWithAnnotation(c *check.C) {
name := "start-annotation"

res := command.PouchRun("create", "--name", name, "--annotation", "a=b", busyboxImage)
res.Assert(c, icmd.Success)
defer DelContainerForceMultyTime(c, name)

command.PouchRun("start", name).Assert(c, icmd.Success)
Copy link
Collaborator

@allencloud allencloud Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think after start, we need to check these annotation as well with pouch inspect.

In addition , I think we need to inspect after create, and then start

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been done in test/cli_run_test.go

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I do not think they are the same. They have called the same API, but in cli side, they called different code. Double check would be nicer to guarantee the code quality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow the previous logic, inspect test in doing in cli_run_test.go, not in start test, as I agree with you, this also related all test case in test/cli_start_test.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! I miss create test, this things should be done there too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. @allencloud

}