Skip to content

Commit

Permalink
feat(k8s): port-forward to Deployments and DaemonSets
Browse files Browse the repository at this point in the history
Previously we only created port forwards for Service resources. We
still prefer those when applicable but also create port forwards
directly to Deployment and DaemonSet container ports.
  • Loading branch information
edvald committed Jul 15, 2021
1 parent 37f1f76 commit 8c2b747
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 15 deletions.
70 changes: 55 additions & 15 deletions core/src/plugins/kubernetes/port-forward.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ChildProcess } from "child_process"

import getPort = require("get-port")
const AsyncLock = require("async-lock")
import { V1Service } from "@kubernetes/client-node"
import { V1ContainerPort, V1Deployment, V1PodTemplate, V1Service } from "@kubernetes/client-node"

import { GetPortForwardParams, GetPortForwardResult } from "../../types/plugin/service/getPortForward"
import { KubernetesProvider, KubernetesPluginContext } from "./config"
Expand All @@ -20,7 +20,7 @@ import { PluginContext } from "../../plugin-context"
import { kubectl } from "./kubectl"
import { KubernetesResource } from "./types"
import { ForwardablePort, GardenService } from "../../types/service"
import { isBuiltIn } from "./util"
import { isBuiltIn, matchSelector } from "./util"
import { LogEntry } from "../../logger/log-entry"
import { RuntimeError } from "../../exceptions"
import execa = require("execa")
Expand Down Expand Up @@ -203,7 +203,7 @@ export async function getPortForwardHandler({
}

function getTargetResource(service: GardenService, targetName?: string) {
return `Service/${targetName || service.name}`
return targetName || `Service/${service.name}`
}

/**
Expand All @@ -212,19 +212,59 @@ function getTargetResource(service: GardenService, targetName?: string) {
export function getForwardablePorts(resources: KubernetesResource[]) {
const ports: ForwardablePort[] = []

for (const resource of resources) {
if (isBuiltIn(resource) && resource.kind === "Service") {
const service = resource as V1Service

for (const portSpec of service.spec!.ports || []) {
ports.push({
name: portSpec.name,
// TODO: not sure if/how possible but it would be good to deduce the protocol somehow
protocol: "TCP",
targetName: service.metadata!.name,
targetPort: portSpec.port,
})
// Start by getting ports defined by Service resources
const services = resources.filter((r) => isBuiltIn(r) && r.kind === "Service") as V1Service[]

for (const service of services) {
for (const portSpec of service.spec!.ports || []) {
ports.push({
name: portSpec.name,
// TODO: not sure if/how possible but it would be good to deduce the protocol somehow
protocol: "TCP",
targetName: "Service/" + service.metadata!.name,
targetPort: portSpec.port,
})
}
}

// Then find ports defined by Deployments and DaemonSets (omitting ports that Service resources already point to).
const workloads = resources.filter(
(r) => (isBuiltIn(r) && r.kind === "Deployment") || r.kind === "DaemonSet"
) as V1Deployment[]

const matchesService = (podTemplate: V1PodTemplate, portSpec: V1ContainerPort) => {
for (const service of services) {
if (!matchSelector(service.spec?.selector || {}, podTemplate.metadata?.labels || {})) {
continue
}

for (const servicePort of service.spec?.ports || []) {
const serviceTargetPort = (servicePort.targetPort as any) as number

if (serviceTargetPort && serviceTargetPort === portSpec.containerPort) {
return true
}
}
}
return false
}

for (const workload of workloads) {
const podTemplate = workload.spec!.template
const containers = podTemplate.spec?.containers || []
const portSpecs = containers.flatMap((c) => c.ports || [])

for (const portSpec of portSpecs) {
if (matchesService(podTemplate, portSpec)) {
continue
}

ports.push({
name: portSpec.name,
protocol: "TCP",
targetName: `${workload.kind!}/${workload.metadata!.name}`,
targetPort: portSpec.containerPort,
})
}
}

Expand Down
12 changes: 12 additions & 0 deletions core/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { getChartPath, renderHelmTemplateString } from "./helm/common"
import { HotReloadableResource } from "./hot-reload/hot-reload"
import { ProviderMap } from "../../config/provider"
import { PodRunner } from "./run"
import { isSubset } from "../../util/is-subset"

export const skopeoImage = "gardendev/skopeo:1.41.0-2"

Expand Down Expand Up @@ -482,6 +483,17 @@ export function getSelectorString(labels: { [key: string]: string }) {
return selectorString.trimEnd().slice(0, -1)
}

/**
* Returns true if the provided matchLabels selector matches the given labels. Use to e.g. match the selector on a
* Service with Pod templates from a Deployment.
*
* @param selector The selector on the Service, or the `matchLabels` part of a Deployment spec selector
* @param labels The workload labels to match agains
*/
export function matchSelector(selector: { [key: string]: string }, labels: { [key: string]: string }) {
return Object.keys(selector).length > 0 && isSubset(labels, selector)
}

/**
* Returns the `serviceResource` spec on the module. If the module has a base module, the two resource specs
* are merged using a JSON Merge Patch (RFC 7396).
Expand Down
150 changes: 150 additions & 0 deletions core/test/unit/src/plugins/kubernetes/port-forward.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* Copyright (C) 2018-2021 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { expect } from "chai"
import { getForwardablePorts } from "../../../../../src/plugins/kubernetes/port-forward"

describe("getForwardablePorts", () => {
it("returns all ports for Service resources", () => {
const ports = getForwardablePorts([
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "foo",
},
spec: {
ports: [{ port: 12345 }],
},
},
])

expect(ports).to.eql([
{
name: undefined,
protocol: "TCP",
targetName: "Service/foo",
targetPort: 12345,
},
])
})

it("returns all ports for Deployment resources", () => {
const ports = getForwardablePorts([
{
apiVersion: "apps/v1",
kind: "Deployment",
metadata: {
name: "foo",
},
spec: {
template: {
spec: {
containers: [
{
ports: [{ containerPort: 12345 }],
},
],
},
},
},
},
])

expect(ports).to.eql([
{
name: undefined,
protocol: "TCP",
targetName: "Deployment/foo",
targetPort: 12345,
},
])
})

it("returns all ports for DaemonSet resources", () => {
const ports = getForwardablePorts([
{
apiVersion: "apps/v1",
kind: "DaemonSet",
metadata: {
name: "foo",
},
spec: {
template: {
spec: {
containers: [
{
ports: [{ containerPort: 12345 }],
},
],
},
},
},
},
])

expect(ports).to.eql([
{
name: undefined,
protocol: "TCP",
targetName: "DaemonSet/foo",
targetPort: 12345,
},
])
})

it("omits a Deployment port that is already pointed to by a Service resource", () => {
const ports = getForwardablePorts([
{
apiVersion: "v1",
kind: "Service",
metadata: {
name: "foo",
},
spec: {
selector: {
app: "foo",
},
ports: [{ port: 12345, targetPort: 12346 }],
},
},
{
apiVersion: "apps/v1",
kind: "Deployment",
metadata: {
name: "foo",
},
spec: {
template: {
metadata: {
labels: {
app: "foo",
},
},
spec: {
containers: [
{
ports: [{ containerPort: 12346 }],
},
],
},
},
},
},
])

expect(ports).to.eql([
{
name: undefined,
protocol: "TCP",
targetName: "Service/foo",
targetPort: 12345,
},
])
})
})
28 changes: 28 additions & 0 deletions core/test/unit/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
getStaticLabelsFromPod,
getSelectorString,
makePodName,
matchSelector,
} from "../../../../../src/plugins/kubernetes/util"
import { KubernetesServerResource } from "../../../../../src/plugins/kubernetes/types"
import { V1Pod } from "@kubernetes/client-node"
Expand Down Expand Up @@ -334,3 +335,30 @@ describe("makePodName", () => {
expect(name.slice(0, -7)).to.equal("test-some-module-with-a-really-unnecessarily-long-name-r")
})
})

describe("matchSelector", () => {
it("should return false if selector is empty", () => {
const matched = matchSelector({}, { foo: "bar" })
expect(matched).to.be.false
})

it("should return false if selector contains key missing from labels", () => {
const matched = matchSelector({ foo: "bar" }, { nope: "nyet" })
expect(matched).to.be.false
})

it("should return false if selector contains value mismatched with a label", () => {
const matched = matchSelector({ foo: "bar" }, { foo: "nyet" })
expect(matched).to.be.false
})

it("should return true if selector matches labels exactly", () => {
const matched = matchSelector({ foo: "bar" }, { foo: "bar" })
expect(matched).to.be.true
})

it("should return true if selector is a subset of labels", () => {
const matched = matchSelector({ foo: "bar" }, { foo: "bar", something: "else" })
expect(matched).to.be.true
})
})

0 comments on commit 8c2b747

Please sign in to comment.