Skip to content

Commit

Permalink
fix(container): allow setting nodePort=true on container modules
Browse files Browse the repository at this point in the history
This allows the cluster to automatically pick a nodePort, which is
often preferable to hard-coding it.

Closes #1057
  • Loading branch information
edvald committed Aug 1, 2019
1 parent 6e37318 commit 8d75188
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 15 deletions.
2 changes: 1 addition & 1 deletion docs/reference/module-types/container.md
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ services:

[services](#services) > [ports](#services[].ports[]) > nodePort

Set this to expose the service on the specified port on the host node (may not be supported by all providers).
Set this to expose the service on the specified port on the host node (may not be supported by all providers). Set to `true` to have the cluster pick a port automatically, which is most often advisable if the cluster is shared by multiple users.

| Type | Required |
| -------- | -------- |
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/module-types/maven-container.md
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ services:

[services](#services) > [ports](#services[].ports[]) > nodePort

Set this to expose the service on the specified port on the host node (may not be supported by all providers).
Set this to expose the service on the specified port on the host node (may not be supported by all providers). Set to `true` to have the cluster pick a port automatically, which is most often advisable if the cluster is shared by multiple users.

| Type | Required |
| -------- | -------- |
Expand Down
9 changes: 6 additions & 3 deletions garden-service/src/plugins/container/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface ServicePortSpec {
// Defaults to containerPort
servicePort: number
hostPort?: number
nodePort?: number | null
nodePort?: number | true
}

export interface ServiceVolumeSpec {
Expand Down Expand Up @@ -247,9 +247,12 @@ export const portSchema = joi.object()
hostPort: joi.number()
.meta({ deprecated: true }),
nodePort: joi.number()
.allow(true)
.description(deline`
Set this to expose the service on the specified port on the host node
(may not be supported by all providers).`),
Set this to expose the service on the specified port on the host node (may not be supported by all providers).
Set to \`true\` to have the cluster pick a port automatically, which is most often advisable if the cluster is
shared by multiple users.
`),
})

const volumeSchema = joi.object()
Expand Down
23 changes: 14 additions & 9 deletions garden-service/src/plugins/kubernetes/container/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { V1ServicePort } from "@kubernetes/client-node"
import { ContainerService } from "../../container/config"

export async function createServiceResources(service: ContainerService, namespace: string) {
const services: any = []

const addService = (name: string, type: string, servicePorts: any[]) => {
const addService = (name: string, type: string, servicePorts: V1ServicePort[]) => {
services.push({
apiVersion: "v1",
kind: "Service",
Expand Down Expand Up @@ -43,17 +44,21 @@ export async function createServiceResources(service: ContainerService, namespac
}

// optionally add a NodePort service for externally open ports, if applicable
// TODO: explore nicer ways to do this
const exposedPorts = ports.filter(portSpec => portSpec.nodePort)

if (exposedPorts.length > 0) {
const nodePorts = exposedPorts.map(portSpec => ({
// TODO: do the parsing and defaults when loading the yaml
name: portSpec.name,
protocol: portSpec.protocol,
port: portSpec.containerPort,
nodePort: portSpec.nodePort,
}))
const nodePorts = exposedPorts.map(portSpec => {
const port: V1ServicePort = {
name: portSpec.name,
protocol: portSpec.protocol,
port: portSpec.servicePort,
targetPort: portSpec.containerPort,
}
if (portSpec.nodePort !== true) {
port.nodePort = portSpec.nodePort
}
return port
})

addService(service.name + "-nodeport", "NodePort", nodePorts)
}
Expand Down
120 changes: 119 additions & 1 deletion garden-service/test/unit/src/plugins/kubernetes/container/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { gardenPlugin } from "../../../../../../src/plugins/container/container"
import { resolve } from "path"
import { Garden } from "../../../../../../src/garden"
import { expect } from "chai"
import { ContainerService } from "../../../../../../src/plugins/container/config"

describe("createServiceResources", () => {
const projectRoot = resolve(dataDir, "test-project-container")
Expand Down Expand Up @@ -48,7 +49,7 @@ describe("createServiceResources", () => {

it("should add annotations if configured", async () => {
const graph = await garden.getConfigGraph()
const service = await graph.getService("service-a")
const service: ContainerService = await graph.getService("service-a")

service.spec.annotations = { my: "annotation" }

Expand Down Expand Up @@ -82,4 +83,121 @@ describe("createServiceResources", () => {
},
])
})

it("should create a NodePort service if a nodePort is specified", async () => {
const graph = await garden.getConfigGraph()
const service: ContainerService = await graph.getService("service-a")

service.spec.ports[0].nodePort = 12345

const resources = await createServiceResources(service, "my-namespace")

expect(resources).to.eql([
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "service-a",
namespace: "my-namespace",
annotations: {},
},
spec: {
ports: [
{
name: "http",
protocol: "TCP",
targetPort: 8080,
port: 8080,
},
],
selector: {
service: "service-a",
},
type: "ClusterIP",
},
},
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "service-a-nodeport",
namespace: "my-namespace",
annotations: {},
},
spec: {
ports: [
{
name: "http",
protocol: "TCP",
targetPort: 8080,
port: 8080,
nodePort: 12345,
},
],
selector: {
service: "service-a",
},
type: "NodePort",
},
},
])
})

it("should create a NodePort service without nodePort set if nodePort is specified as true", async () => {
const graph = await garden.getConfigGraph()
const service: ContainerService = await graph.getService("service-a")

service.spec.ports[0].nodePort = true

const resources = await createServiceResources(service, "my-namespace")

expect(resources).to.eql([
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "service-a",
namespace: "my-namespace",
annotations: {},
},
spec: {
ports: [
{
name: "http",
protocol: "TCP",
targetPort: 8080,
port: 8080,
},
],
selector: {
service: "service-a",
},
type: "ClusterIP",
},
},
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "service-a-nodeport",
namespace: "my-namespace",
annotations: {},
},
spec: {
ports: [
{
name: "http",
protocol: "TCP",
targetPort: 8080,
port: 8080,
},
],
selector: {
service: "service-a",
},
type: "NodePort",
},
},
])
})
})

0 comments on commit 8d75188

Please sign in to comment.