Skip to content

Commit

Permalink
Support multiple port mappings on additional_containers (#361)
Browse files Browse the repository at this point in the history 
* Support multiple port mappings on additional_containers

* Executor: use new additional container's ports field
  • Loading branch information
@edigaryev
edigaryev authored Apr 6, 2021
1 parent 0116211 commit c59a412
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 49 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/antihax/optional v1.0.0
github.com/avast/retry-go v3.0.0+incompatible
github.com/certifi/gocertifi v0.0.0-20200922220541-2c3bb06c6054
github.com/cirruslabs/cirrus-ci-agent v1.37.0
github.com/cirruslabs/cirrus-ci-agent v1.39.0
github.com/cirruslabs/echelon v1.4.1
github.com/cirruslabs/go-java-glob v0.1.0
github.com/cirruslabs/podmanapi v0.1.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5P
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
github.com/cilium/ebpf v0.0.0-20200110133405-4032b1d8aae3/go.mod h1:MA5e5Lr8slmEg9bt0VpxxWqJlO4iwu3FBdHUzV7wQVg=
github.com/cilium/ebpf v0.0.0-20200507155900-a9f01edf17e3/go.mod h1:XT+cAw5wfvsodedcijoh1l9cf7v1x9FlFB/3VmF/O8s=
github.com/cirruslabs/cirrus-ci-agent v1.37.0 h1:C05EnW7eeMWH115JXHc6WswXhqorO3Tb7CNwe+wwGVY=
github.com/cirruslabs/cirrus-ci-agent v1.37.0/go.mod h1:T3amh8kB54wkmpWrQ/VXVl9osHXdt3ILS0cH5ZaYGF4=
github.com/cirruslabs/cirrus-ci-agent v1.39.0 h1:0ekH81DiZVBiZi5Ju8jlnMTm4bRIFk0ZuC8bQ236g7Q=
github.com/cirruslabs/cirrus-ci-agent v1.39.0/go.mod h1:T3amh8kB54wkmpWrQ/VXVl9osHXdt3ILS0cH5ZaYGF4=
github.com/cirruslabs/cirrus-ci-annotations v0.1.0/go.mod h1:xrmxzL58Pf4cSSQCmQEOPGQ3poeARxJdHneurUrqjZk=
github.com/cirruslabs/echelon v1.4.1 h1:7ij7cANbL1feOyds5sRtYLPBJwycFi3nvLKJI4vCOPs=
github.com/cirruslabs/echelon v1.4.1/go.mod h1:1jFBACMy3tzodXyTtNNLN9bw6UUU7Xpq9tYMRydehtY=
Expand Down
14 changes: 10 additions & 4 deletions internal/executor/instance/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ type Params struct {
WorkingDirectory string
}

// nolint:gocognit
func RunContainerizedAgent(ctx context.Context, config *runconfig.RunConfig, params *Params) error {
logger := config.Logger
backend := config.ContainerBackend
Expand Down Expand Up @@ -237,7 +238,9 @@ func RunContainerizedAgent(ctx context.Context, config *runconfig.RunConfig, par
if len(params.AdditionalContainers) > 0 {
var ports []string
for _, additionalContainer := range params.AdditionalContainers {
ports = append(ports, strconv.FormatUint(uint64(additionalContainer.ContainerPort), 10))
for _, portMapping := range additionalContainer.Ports {
ports = append(ports, strconv.FormatUint(uint64(portMapping.ContainerPort), 10))
}
}
commaDelimitedPorts := strings.Join(ports, ",")
input.Env["CIRRUS_PORTS_WAIT_FOR"] = commaDelimitedPorts
Expand Down Expand Up @@ -383,9 +386,12 @@ func runAdditionalContainer(
// would require fiddling with Netfilter, which results in unwanted complexity.
//
// So here we simply do our best effort and warn the user about potential problems.
if additionalContainer.HostPort != 0 {
logger.Warnf("port mappings are unsupported by the Cirrus CLI, please tell the application "+
"running in the additional container '%s' to use a different port", additionalContainer.Name)
for _, portMapping := range additionalContainer.Ports {
if portMapping.HostPort != 0 {
logger.Warnf("port mappings are unsupported by the Cirrus CLI, please tell the application "+
"running in the additional container '%s' to use a different port", additionalContainer.Name)
break
}
}

logger.Debugf("starting additional container %s", cont.ID)
Expand Down
76 changes: 56 additions & 20 deletions pkg/parser/instance/additionalcontainer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,35 @@ type AdditionalContainer struct {
parseable.DefaultParser
}

func parsePort(port string) (*api.PortMapping, error) {
// Support port mapping where a host port[1] is specified in addition to container port
// [1]: https://cirrus-ci.org/guide/writing-tasks/#additional-containers
const maxSplits = 2

portParts := strings.SplitN(port, ":", maxSplits)

if len(portParts) == maxSplits {
hostPort, err := strconv.ParseUint(portParts[0], 10, 32)
if err != nil {
return nil, err
}

containerPort, err := strconv.ParseUint(portParts[1], 10, 32)
if err != nil {
return nil, err
}

return &api.PortMapping{ContainerPort: uint32(containerPort), HostPort: uint32(hostPort)}, nil
}

containerPort, err := strconv.ParseUint(portParts[0], 10, 32)
if err != nil {
return nil, err
}

return &api.PortMapping{ContainerPort: uint32(containerPort)}, nil
}

// nolint:gocognit
func NewAdditionalContainer(mergedEnv map[string]string, boolevator *boolevator.Boolevator) *AdditionalContainer {
ac := &AdditionalContainer{
Expand Down Expand Up @@ -83,36 +112,35 @@ func NewAdditionalContainer(mergedEnv map[string]string, boolevator *boolevator.
return nil
})

ac.RequiredField(nameable.NewSimpleNameable("port"), schema.Port(), func(node *node.Node) error {
ac.OptionalField(nameable.NewSimpleNameable("port"), schema.Port(), func(node *node.Node) error {
port, err := node.GetExpandedStringValue(mergedEnv)
if err != nil {
return err
}

// Support port mapping where a host port[1] is specified in addition to container port
// [1]: https://cirrus-ci.org/guide/writing-tasks/#additional-containers
const maxSplits = 2
portMapping, err := parsePort(port)
if err != nil {
return node.ParserError("failed to parse port: %v", err)
}

portParts := strings.SplitN(port, ":", maxSplits)
ac.proto.Ports = append(ac.proto.Ports, portMapping)

if len(portParts) == maxSplits {
hostPort, err := strconv.ParseUint(portParts[0], 10, 32)
if err != nil {
return err
}
ac.proto.HostPort = uint32(hostPort)
return nil
})

containerPort, err := strconv.ParseUint(portParts[1], 10, 32)
if err != nil {
return err
}
ac.proto.ContainerPort = uint32(containerPort)
} else {
containerPort, err := strconv.ParseUint(portParts[0], 10, 32)
ac.OptionalField(nameable.NewSimpleNameable("ports"), schema.Ports(), func(node *node.Node) error {
ports, err := node.GetSliceOfExpandedStrings(mergedEnv)
if err != nil {
return err
}

for _, port := range ports {
portMapping, err := parsePort(port)
if err != nil {
return err
return node.ParserError("failed to parse port: %v", err)
}
ac.proto.ContainerPort = uint32(containerPort)

ac.proto.Ports = append(ac.proto.Ports, portMapping)
}

return nil
Expand Down Expand Up @@ -187,6 +215,14 @@ func (ac *AdditionalContainer) Parse(node *node.Node) (*api.AdditionalContainer,
return nil, err
}

// Once "port" field is deprecated we can mark "ports" field as required and remove this logic
if len(ac.proto.Ports) == 0 {
return nil, node.ParserError("should specify either \"port\" or \"ports\"")
}
if node.HasChild("port") && node.HasChild("ports") {
return nil, node.ParserError("please only use \"ports\" field")
}

// Resource defaults
if ac.proto.Cpu == 0 {
ac.proto.Cpu = defaultAdditionalCPU
Expand Down
8 changes: 8 additions & 0 deletions pkg/parser/schema/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ func Port() *schema.Schema {
}
}

func Ports() *schema.Schema {
result := ArrayOf(Port())

result.Description = "Ports exposed by the container."

return result
}

func ArrayOf(arrayItemSchema *schema.Schema) *schema.Schema {
return &schema.Schema{
Type: schema.PrimitiveTypes{schema.ArrayType},
Expand Down
100 changes: 90 additions & 10 deletions pkg/parser/testdata/cirrus.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2487,11 +2487,27 @@
}
],
"description": "Container readiness probe command."
},
"ports": {
"description": "Ports exposed by the container.",
"items": [
{
"anyOf": [
{
"type": "number"
},
{
"type": "string"
}
],
"description": "Port exposed by the container."
}
],
"type": "array"
}
},
"required": [
"image",
"port"
"image"
],
"type": "object"
}
Expand Down Expand Up @@ -2678,11 +2694,27 @@
}
],
"description": "Container readiness probe command."
},
"ports": {
"description": "Ports exposed by the container.",
"items": [
{
"anyOf": [
{
"type": "number"
},
{
"type": "string"
}
],
"description": "Port exposed by the container."
}
],
"type": "array"
}
},
"required": [
"image",
"port"
"image"
],
"type": "object"
}
Expand Down Expand Up @@ -2871,11 +2903,27 @@
}
],
"description": "Container readiness probe command."
},
"ports": {
"description": "Ports exposed by the container.",
"items": [
{
"anyOf": [
{
"type": "number"
},
{
"type": "string"
}
],
"description": "Port exposed by the container."
}
],
"type": "array"
}
},
"required": [
"image",
"port"
"image"
],
"type": "object"
}
Expand Down Expand Up @@ -3164,11 +3212,27 @@
}
],
"description": "Container readiness probe command."
},
"ports": {
"description": "Ports exposed by the container.",
"items": [
{
"anyOf": [
{
"type": "number"
},
{
"type": "string"
}
],
"description": "Port exposed by the container."
}
],
"type": "array"
}
},
"required": [
"image",
"port"
"image"
],
"type": "object"
}
Expand Down Expand Up @@ -3858,11 +3922,27 @@
}
],
"description": "Container readiness probe command."
},
"ports": {
"description": "Ports exposed by the container.",
"items": [
{
"anyOf": [
{
"type": "number"
},
{
"type": "string"
}
],
"description": "Port exposed by the container."
}
],
"type": "array"
}
},
"required": [
"image",
"port"
"image"
],
"type": "object"
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/parser/testdata/example-mysql.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@
"@type": "type.googleapis.com/org.cirruslabs.ci.services.cirruscigrpc.ContainerInstance",
"additionalContainers": [
{
"containerPort": 3306,
"cpu": 0.5,
"environment": {
"MYSQL_ROOT_PASSWORD": ""
},
"image": "mysql:latest",
"memory": 512,
"name": "mysql"
"name": "mysql",
"ports": [
{
"containerPort": 3306
}
]
}
],
"cpu": 2,
Expand Down
12 changes: 10 additions & 2 deletions pkg/parser/testdata/proto-instance.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
"image": "mysql:latest",
"memory": 1024,
"name": "mysql",
"containerPort": 3306
"ports": [
{
"containerPort": 3306
}
]
}
],
"cpu": 2.5,
Expand Down Expand Up @@ -59,7 +63,11 @@
"image": "mysql:latest",
"memory": 1024,
"name": "mysql",
"containerPort": 3306
"ports": [
{
"containerPort": 3306
}
]
}
],
"cpu": 2.5,
Expand Down
8 changes: 6 additions & 2 deletions pkg/parser/testdata/via-rpc/new-deduplicator-same-type.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@
"@type": "type.googleapis.com/org.cirruslabs.ci.services.cirruscigrpc.ContainerInstance",
"additionalContainers": [
{
"containerPort": 80,
"cpu": 0.5,
"image": "nginx:latest",
"memory": 20480,
"name": "nginx"
"name": "nginx",
"ports": [
{
"containerPort": 80
}
]
}
],
"cpu": 2,
Expand Down
Loading

0 comments on commit c59a412

Please sign in to comment.