From 27758a9a7b2e56d9c797bbee0790ce6db45a43ae Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Wed, 28 Jul 2021 15:17:34 -0700 Subject: [PATCH] Implement port forwarding for Docker deployer (#6303) * Implement port forwarding for Docker deployer * schemas * linters * use PortManager as Accessor --- docs/content/en/schemas/v2beta20.json | 12 +- integration/diagnose_test.go | 2 +- .../examples/react-reload-docker/README.md | 17 ++ .../examples/react-reload-docker/app/.babelrc | 6 + .../react-reload-docker/app/.dockerignore | 2 + .../react-reload-docker/app/.gitignore | 2 + .../react-reload-docker/app/Dockerfile | 10 ++ .../react-reload-docker/app/package.json | 26 +++ .../app/src/components/HelloWorld.js | 8 + .../react-reload-docker/app/src/index.html | 11 ++ .../react-reload-docker/app/src/main.js | 5 + .../app/src/styles/HelloWorld.css | 6 + .../react-reload-docker/app/webpack.config.js | 27 +++ .../react-reload-docker/k8s/deployment.yaml | 29 ++++ .../react-reload-docker/skaffold.yaml | 16 ++ pkg/skaffold/deploy/docker/deploy.go | 89 ++++++---- pkg/skaffold/deploy/docker/port.go | 154 ++++++++++++++++++ pkg/skaffold/deploy/docker/port_test.go | 89 ++++++++++ pkg/skaffold/deploy/resource/deployment.go | 2 +- pkg/skaffold/docker/image.go | 18 +- pkg/skaffold/docker/logger/formatter_test.go | 2 +- pkg/skaffold/docker/tracker/tracker.go | 45 +++-- pkg/skaffold/docker/tracker/tracker_test.go | 83 ++-------- pkg/skaffold/schema/latest/v1/config.go | 11 +- .../schema/validation/samples_test.go | 2 +- pkg/skaffold/schema/validation/validation.go | 1 + 26 files changed, 538 insertions(+), 137 deletions(-) create mode 100644 integration/examples/react-reload-docker/README.md create mode 100644 integration/examples/react-reload-docker/app/.babelrc create mode 100644 integration/examples/react-reload-docker/app/.dockerignore create mode 100644 integration/examples/react-reload-docker/app/.gitignore create mode 100644 integration/examples/react-reload-docker/app/Dockerfile create mode 100644 integration/examples/react-reload-docker/app/package.json create mode 100644 integration/examples/react-reload-docker/app/src/components/HelloWorld.js create mode 100644 integration/examples/react-reload-docker/app/src/index.html create mode 100644 integration/examples/react-reload-docker/app/src/main.js create mode 100644 integration/examples/react-reload-docker/app/src/styles/HelloWorld.css create mode 100644 integration/examples/react-reload-docker/app/webpack.config.js create mode 100644 integration/examples/react-reload-docker/k8s/deployment.yaml create mode 100644 integration/examples/react-reload-docker/skaffold.yaml create mode 100644 pkg/skaffold/deploy/docker/port.go create mode 100644 pkg/skaffold/deploy/docker/port_test.go diff --git a/docs/content/en/schemas/v2beta20.json b/docs/content/en/schemas/v2beta20.json index 99efa80560f..7463346d5d3 100755 --- a/docs/content/en/schemas/v2beta20.json +++ b/docs/content/en/schemas/v2beta20.json @@ -2789,8 +2789,8 @@ }, "namespace": { "type": "string", - "description": "namespace of the resource to port forward.", - "x-intellij-html-description": "namespace of the resource to port forward." + "description": "namespace of the resource to port forward. Does not apply to local containers.", + "x-intellij-html-description": "namespace of the resource to port forward. Does not apply to local containers." }, "port": { "anyOf": [ @@ -2806,13 +2806,13 @@ }, "resourceName": { "type": "string", - "description": "name of the Kubernetes resource to port forward.", - "x-intellij-html-description": "name of the Kubernetes resource to port forward." + "description": "name of the Kubernetes resource or local container to port forward.", + "x-intellij-html-description": "name of the Kubernetes resource or local container to port forward." }, "resourceType": { "type": "string", - "description": "Kubernetes type that should be port forwarded. Acceptable resource types include: `Service`, `Pod` and Controller resource type that has a pod spec: `ReplicaSet`, `ReplicationController`, `Deployment`, `StatefulSet`, `DaemonSet`, `Job`, `CronJob`.", - "x-intellij-html-description": "Kubernetes type that should be port forwarded. Acceptable resource types include: Service, Pod and Controller resource type that has a pod spec: ReplicaSet, ReplicationController, Deployment, StatefulSet, DaemonSet, Job, CronJob." + "description": "resource type that should be port forwarded. Acceptable resource types include kubernetes types: `Service`, `Pod` and Controller resource type that has a pod spec: `ReplicaSet`, `ReplicationController`, `Deployment`, `StatefulSet`, `DaemonSet`, `Job`, `CronJob`. Standalone `Container` is also valid for Docker deployments.", + "x-intellij-html-description": "resource type that should be port forwarded. Acceptable resource types include kubernetes types: Service, Pod and Controller resource type that has a pod spec: ReplicaSet, ReplicationController, Deployment, StatefulSet, DaemonSet, Job, CronJob. Standalone Container is also valid for Docker deployments." } }, "preferredOrder": [ diff --git a/integration/diagnose_test.go b/integration/diagnose_test.go index d9a35dca8c1..111de74bd45 100644 --- a/integration/diagnose_test.go +++ b/integration/diagnose_test.go @@ -61,7 +61,7 @@ func folders(root string) ([]string, error) { for _, f := range files { // TODO(nkubala): remove once yaml is unhidden - if f.Mode().IsDir() && f.Name() != "docker-deploy" { + if f.Mode().IsDir() && f.Name() != "docker-deploy" && f.Name() != "react-reload-docker" { folders = append(folders, f.Name()) } } diff --git a/integration/examples/react-reload-docker/README.md b/integration/examples/react-reload-docker/README.md new file mode 100644 index 00000000000..44c3cb5bea0 --- /dev/null +++ b/integration/examples/react-reload-docker/README.md @@ -0,0 +1,17 @@ +### Example: React app with hot-reload + +Simple React app demonstrating the file synchronization mode in conjunction with webpack hot module reload. + +#### Init + +```bash +skaffold dev +``` + +#### Workflow + +* Make some changes to `HelloWorld.js`: + * The file will be synchronized to the cluster + * `webpack` will perform hot module reloading +* Make some changes to `package.json`: + * The full build/push/deploy process will be triggered, fetching dependencies from `npm` diff --git a/integration/examples/react-reload-docker/app/.babelrc b/integration/examples/react-reload-docker/app/.babelrc new file mode 100644 index 00000000000..fbd6da0e13a --- /dev/null +++ b/integration/examples/react-reload-docker/app/.babelrc @@ -0,0 +1,6 @@ +{ + "presets": [ + "@babel/preset-env", + "@babel/preset-react" + ] +} \ No newline at end of file diff --git a/integration/examples/react-reload-docker/app/.dockerignore b/integration/examples/react-reload-docker/app/.dockerignore new file mode 100644 index 00000000000..1bd722694bd --- /dev/null +++ b/integration/examples/react-reload-docker/app/.dockerignore @@ -0,0 +1,2 @@ +node_modules +*.swp diff --git a/integration/examples/react-reload-docker/app/.gitignore b/integration/examples/react-reload-docker/app/.gitignore new file mode 100644 index 00000000000..4ef133f11f3 --- /dev/null +++ b/integration/examples/react-reload-docker/app/.gitignore @@ -0,0 +1,2 @@ +/node_modules/ +package-lock.json diff --git a/integration/examples/react-reload-docker/app/Dockerfile b/integration/examples/react-reload-docker/app/Dockerfile new file mode 100644 index 00000000000..7b09d9c7f61 --- /dev/null +++ b/integration/examples/react-reload-docker/app/Dockerfile @@ -0,0 +1,10 @@ +FROM node:14.9-alpine + +WORKDIR /app +EXPOSE 8080 +CMD ["npm", "run", "dev"] + +COPY package* ./ +# examples don't use package-lock.json to minimize updates +RUN npm install --no-package-lock +COPY . . diff --git a/integration/examples/react-reload-docker/app/package.json b/integration/examples/react-reload-docker/app/package.json new file mode 100644 index 00000000000..3c39d7c79bd --- /dev/null +++ b/integration/examples/react-reload-docker/app/package.json @@ -0,0 +1,26 @@ +{ + "name": "react-reload", + "version": "1.0.0", + "description": "A React demo application for skaffold", + "main": "index.js", + "scripts": { + "dev": "webpack-dev-server --mode development --hot" + }, + "devDependencies": { + "@babel/core": "^7.11.6", + "@babel/preset-env": "^7.11.5", + "@babel/preset-react": "^7.10.4", + "babel-loader": "^8.1.0", + "css-loader": "^2.1.1", + "html-webpack-plugin": "^3.2.0", + "style-loader": "^0.23.1", + "webpack": "^4.44.1", + "webpack-cli": "^3.3.12", + "webpack-dev-server": "^3.11.0" + }, + "dependencies": { + "react": "^16.13.1", + "react-dom": "^16.13.1" + }, + "browserslist": "> 2%, not dead" +} diff --git a/integration/examples/react-reload-docker/app/src/components/HelloWorld.js b/integration/examples/react-reload-docker/app/src/components/HelloWorld.js new file mode 100644 index 00000000000..e26cecbd0d7 --- /dev/null +++ b/integration/examples/react-reload-docker/app/src/components/HelloWorld.js @@ -0,0 +1,8 @@ +import React from 'react'; +import '../styles/HelloWorld.css'; + +export const HelloWorld = () => ( +
+

Hello world!

+
+); diff --git a/integration/examples/react-reload-docker/app/src/index.html b/integration/examples/react-reload-docker/app/src/index.html new file mode 100644 index 00000000000..bc97a86dc10 --- /dev/null +++ b/integration/examples/react-reload-docker/app/src/index.html @@ -0,0 +1,11 @@ + + + + + + React demo app for skaffold + + +
+ + diff --git a/integration/examples/react-reload-docker/app/src/main.js b/integration/examples/react-reload-docker/app/src/main.js new file mode 100644 index 00000000000..1166c63dbe7 --- /dev/null +++ b/integration/examples/react-reload-docker/app/src/main.js @@ -0,0 +1,5 @@ +import React from 'react'; +import ReactDOM from 'react-dom'; +import { HelloWorld } from './components/HelloWorld.js'; + +ReactDOM.render( < HelloWorld/>, document.getElementById( 'root' ) ); diff --git a/integration/examples/react-reload-docker/app/src/styles/HelloWorld.css b/integration/examples/react-reload-docker/app/src/styles/HelloWorld.css new file mode 100644 index 00000000000..c6b88d966a2 --- /dev/null +++ b/integration/examples/react-reload-docker/app/src/styles/HelloWorld.css @@ -0,0 +1,6 @@ +h1 { + color: #27aedb; + text-align: center; + margin-top: 40vh; + font-size: 120pt; +} diff --git a/integration/examples/react-reload-docker/app/webpack.config.js b/integration/examples/react-reload-docker/app/webpack.config.js new file mode 100644 index 00000000000..0a41d4d1b84 --- /dev/null +++ b/integration/examples/react-reload-docker/app/webpack.config.js @@ -0,0 +1,27 @@ +const path = require( 'path' ); +const HtmlWebpackPlugin = require( 'html-webpack-plugin' ); + +module.exports = { + entry: './src/main.js', + output: { + path: path.join( __dirname, '/dist' ), + filename: 'main.js' + }, + devServer:{ + host: '0.0.0.0' + }, + module: { + rules: [ + { + test: /\.js$/, + exclude: /node_modules/, + use: [ 'babel-loader' ] + }, + { + test: /\.css$/, + use: [ 'style-loader', 'css-loader' ] + } + ] + }, + plugins: [ new HtmlWebpackPlugin( { template: './src/index.html' } ) ] +}; diff --git a/integration/examples/react-reload-docker/k8s/deployment.yaml b/integration/examples/react-reload-docker/k8s/deployment.yaml new file mode 100644 index 00000000000..16661e7643d --- /dev/null +++ b/integration/examples/react-reload-docker/k8s/deployment.yaml @@ -0,0 +1,29 @@ +apiVersion: v1 +kind: Service +metadata: + name: node +spec: + ports: + - port: 8080 + type: LoadBalancer + selector: + app: node +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: node +spec: + selector: + matchLabels: + app: node + template: + metadata: + labels: + app: node + spec: + containers: + - name: react + image: react-reload-docker + ports: + - containerPort: 8080 diff --git a/integration/examples/react-reload-docker/skaffold.yaml b/integration/examples/react-reload-docker/skaffold.yaml new file mode 100644 index 00000000000..29b07491451 --- /dev/null +++ b/integration/examples/react-reload-docker/skaffold.yaml @@ -0,0 +1,16 @@ +apiVersion: skaffold/v2beta20 +kind: Config +build: + local: + push: false + artifacts: + - image: react-reload-docker + context: app +deploy: + docker: + images: [react-reload-docker] +portForward: +- resourceType: Container + resourceName: react-reload-docker + port: 8080 + localPort: 9000 diff --git a/pkg/skaffold/deploy/docker/deploy.go b/pkg/skaffold/deploy/docker/deploy.go index 4e75c993bb1..61892d16e46 100644 --- a/pkg/skaffold/deploy/docker/deploy.go +++ b/pkg/skaffold/deploy/docker/deploy.go @@ -41,17 +41,18 @@ import ( ) type Deployer struct { - accessor access.Accessor debugger debug.Debugger logger log.Logger monitor status.Monitor syncer pkgsync.Syncer - cfg *v1.DockerDeploy - tracker *tracker.ContainerTracker - client dockerutil.LocalDaemon - network string - once sync.Once + cfg *v1.DockerDeploy + tracker *tracker.ContainerTracker + portManager *PortManager // functions as Accessor + client dockerutil.LocalDaemon + network string + resources []*v1.PortForwardResource + once sync.Once } func NewDeployer(cfg dockerutil.Config, labeller *label.DefaultLabeller, d *v1.DockerDeploy, resources []*v1.PortForwardResource) (*Deployer, error) { @@ -67,16 +68,17 @@ func NewDeployer(cfg dockerutil.Config, labeller *label.DefaultLabeller, d *v1.D } return &Deployer{ - cfg: d, - client: client, - network: fmt.Sprintf("skaffold-network-%s", uuid.New().String()), + cfg: d, + client: client, + network: fmt.Sprintf("skaffold-network-%s", uuid.New().String()), + resources: resources, // TODO(nkubala): implement components - tracker: tracker, - accessor: &access.NoopAccessor{}, - debugger: &debug.NoopDebugger{}, - logger: l, - monitor: &status.NoopMonitor{}, - syncer: &pkgsync.NoopSyncer{}, + tracker: tracker, + portManager: NewPortManager(), // fulfills Accessor interface + debugger: &debug.NoopDebugger{}, + logger: l, + monitor: &status.NoopMonitor{}, + syncer: &pkgsync.NoopSyncer{}, }, nil } @@ -84,10 +86,10 @@ func (d *Deployer) TrackBuildArtifacts(artifacts []graph.Artifact) { d.logger.RegisterArtifacts(artifacts) } -// TrackContainerFromBuild adds an artifact and its newly-associated container id +// TrackContainerFromBuild adds an artifact and its newly-associated container // to the container tracker. -func (d *Deployer) TrackContainerFromBuild(build graph.Artifact, id string) { - d.tracker.Add(build, id) +func (d *Deployer) TrackContainerFromBuild(artifact graph.Artifact, container tracker.Container) { + d.tracker.Add(artifact, container) } // Deploy deploys built artifacts by creating containers in the local docker daemon @@ -119,40 +121,69 @@ func (d *Deployer) deploy(ctx context.Context, out io.Writer, b graph.Artifact) logrus.Warnf("skipping deploy for image %s since it was not built by Skaffold", b.ImageName) return nil } - if containerID := d.tracker.DeployedContainerForImage(b.ImageName); containerID != "" { - logrus.Debugf("removing old container %s for image %s", containerID, b.ImageName) - if err := d.client.Delete(ctx, out, containerID); err != nil { - return fmt.Errorf("failed to remove old container %s for image %s: %w", containerID, b.ImageName, err) + if container, found := d.tracker.ContainerForImage(b.ImageName); found { + logrus.Debugf("removing old container %s for image %s", container.ID, b.ImageName) + if err := d.client.Delete(ctx, out, container.ID); err != nil { + return fmt.Errorf("failed to remove old container %s for image %s: %w", container.ID, b.ImageName, err) } + d.portManager.relinquishPorts(container.Name) } if d.cfg.UseCompose { // TODO(nkubala): implement return fmt.Errorf("docker compose not yet supported by skaffold") } + + ports, bindings, err := d.portManager.getPorts(b.ImageName, d.resources) + if err != nil { + return err + } + + containerName := d.getContainerName(ctx, b.ImageName) + opts := dockerutil.ContainerCreateOpts{ - Name: b.ImageName, - Image: b.Tag, - Network: d.network, + Name: containerName, + Image: b.Tag, + Network: d.network, + Ports: ports, + Bindings: bindings, } id, err := d.client.Run(ctx, out, opts) if err != nil { return errors.Wrap(err, "creating container in local docker") } - d.TrackContainerFromBuild(b, id) + d.TrackContainerFromBuild(b, tracker.Container{Name: containerName, ID: id}) return nil } +func (d *Deployer) getContainerName(ctx context.Context, name string) string { + currentName := name + counter := 1 + for { + if !d.client.ContainerExists(ctx, currentName) { + break + } + currentName = fmt.Sprintf("%s-%d", name, counter) + counter++ + } + + if currentName != name { + logrus.Debugf("container %s already present in local daemon: using %s instead", name, currentName) + } + return currentName +} + func (d *Deployer) Dependencies() ([]string, error) { // noop since there is no deploy config return nil, nil } func (d *Deployer) Cleanup(ctx context.Context, out io.Writer) error { - for _, id := range d.tracker.DeployedContainers() { - if err := d.client.Delete(ctx, out, id); err != nil { + for _, container := range d.tracker.DeployedContainers() { + if err := d.client.Delete(ctx, out, container.ID); err != nil { // TODO(nkubala): replace with actionable error return errors.Wrap(err, "cleaning up deployed container") } + d.portManager.relinquishPorts(container.Name) } err := d.client.NetworkRemove(ctx, d.network) @@ -164,7 +195,7 @@ func (d *Deployer) Render(context.Context, io.Writer, []graph.Artifact, bool, st } func (d *Deployer) GetAccessor() access.Accessor { - return d.accessor + return d.portManager } func (d *Deployer) GetDebugger() debug.Debugger { diff --git a/pkg/skaffold/deploy/docker/port.go b/pkg/skaffold/deploy/docker/port.go new file mode 100644 index 00000000000..ce17040552b --- /dev/null +++ b/pkg/skaffold/deploy/docker/port.go @@ -0,0 +1,154 @@ +/* +Copyright 2021 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package docker + +import ( + "context" + "fmt" + "io" + "strings" + "sync" + + "github.com/docker/go-connections/nat" + "github.com/sirupsen/logrus" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" + eventV2 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event/v2" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output" + v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + schemautil "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" +) + +type containerPortForwardEntry struct { + container string + resourceName string + resourceAddress string + localPort int32 + remotePort schemautil.IntOrString +} + +type PortManager struct { + containerPorts map[string][]int // maps containers to the ports they have allocated + portSet util.PortSet + entries []containerPortForwardEntry // reference shared with DockerForwarder so output is issued in the correct phase of the dev loop + lock sync.Mutex +} + +func NewPortManager() *PortManager { + return &PortManager{ + containerPorts: make(map[string][]int), + portSet: util.PortSet{}, + } +} + +func (pm *PortManager) Start(_ context.Context, out io.Writer) error { + pm.lock.Lock() + defer pm.lock.Unlock() + pm.containerPortForwardEvents(out) + return nil +} + +func (pm *PortManager) Stop() { + pm.lock.Lock() + defer pm.lock.Unlock() + pm.entries = nil +} + +// getPorts converts PortForwardResources into docker.PortSet/PortMap objects. +// These are passed to ContainerCreate on Deploy to expose container ports on the host. +// It also returns a list of containerPortForwardEntry, to be passed to the event handler +func (pm *PortManager) getPorts(containerName string, pf []*v1.PortForwardResource) (nat.PortSet, nat.PortMap, error) { + pm.lock.Lock() + defer pm.lock.Unlock() + s := make(nat.PortSet) + m := make(nat.PortMap) + var entries []containerPortForwardEntry + var ports []int + for _, p := range pf { + if strings.ToLower(string(p.Type)) != "container" { + logrus.Debugf("skipping non-container port forward resource in Docker deploy: %s\n", p.Name) + continue + } + localPort := util.GetAvailablePort(p.Address, p.LocalPort, &pm.portSet) + ports = append(ports, localPort) + port, err := nat.NewPort("tcp", p.Port.String()) + if err != nil { + return nil, nil, err + } + s[port] = struct{}{} + m[port] = []nat.PortBinding{ + {HostIP: p.Address, HostPort: fmt.Sprintf("%d", localPort)}, + } + entries = append(entries, containerPortForwardEntry{ + container: containerName, + resourceName: p.Name, + resourceAddress: p.Address, + localPort: int32(localPort), + remotePort: p.Port, + }) + } + pm.containerPorts[containerName] = ports + pm.entries = append(pm.entries, entries...) + return s, m, nil +} + +func (pm *PortManager) relinquishPorts(containerName string) { + pm.lock.Lock() + defer pm.lock.Unlock() + ports := pm.containerPorts[containerName] + for _, port := range ports { + pm.portSet.Delete(port) + } + pm.containerPorts[containerName] = nil +} + +func (pm *PortManager) containerPortForwardEvents(out io.Writer) { + for _, entry := range pm.entries { + event.PortForwarded( + entry.localPort, + entry.remotePort, + "", // no pod name + entry.container, // container name + "", // no namespace + "", // no port name + "container", + entry.resourceName, + entry.resourceAddress, + ) + + eventV2.PortForwarded( + entry.localPort, + entry.remotePort, + "", // no pod name + entry.container, // container name + "", // no namespace + "", // no port name + "container", + entry.resourceName, + entry.resourceAddress, + ) + + output.Green.Fprintln(out, + fmt.Sprintf("[%s] Forwarding container port %s -> local port http://%s:%d", + entry.container, + entry.remotePort.String(), + entry.resourceAddress, + entry.localPort, + )) + } +} diff --git a/pkg/skaffold/deploy/docker/port_test.go b/pkg/skaffold/deploy/docker/port_test.go new file mode 100644 index 00000000000..4fb8b3028ab --- /dev/null +++ b/pkg/skaffold/deploy/docker/port_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2021 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package docker + +import ( + "testing" + + v1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestGetPorts(t *testing.T) { + tests := []struct { + name string + resources map[int]*v1.PortForwardResource // we map local port to resources for ease of testing + }{ + { + name: "one port, one resource", + resources: map[int]*v1.PortForwardResource{ + 9000: { + Port: util.IntOrString{ + IntVal: 9000, + StrVal: "9000", + }, + Address: "127.0.0.1", + LocalPort: 9000, + }, + }, + }, + { + name: "two ports, two resources", + resources: map[int]*v1.PortForwardResource{ + 1234: { + Port: util.IntOrString{ + IntVal: 20, + StrVal: "20", + }, + Address: "192.168.999.999", + LocalPort: 1234, + }, + 4321: { + Port: util.IntOrString{ + IntVal: 8080, + StrVal: "8080", + }, + Address: "localhost", + LocalPort: 4321, + }, + }, + }, + } + + for _, test := range tests { + testutil.Run(t, test.name, func(t *testutil.T) { + pm := NewPortManager() + s, m, err := pm.getPorts(test.name, collectResources(test.resources)) + for port := range s { // the PortSet contains the local ports, so we grab the bindings keyed off these + bindings := m[port] + t.CheckDeepEqual(len(bindings), 1) // we always have a 1-1 mapping of resource to binding + t.CheckError(false, err) // shouldn't error, unless GetAvailablePort is broken + t.CheckDeepEqual(bindings[0].HostIP, test.resources[port.Int()].Address) + t.CheckDeepEqual(bindings[0].HostPort, test.resources[port.Int()].Port.StrVal) + } + }) + } +} + +func collectResources(resourceMap map[int]*v1.PortForwardResource) []*v1.PortForwardResource { + var resources []*v1.PortForwardResource + for _, r := range resourceMap { + resources = append(resources, r) + } + return resources +} diff --git a/pkg/skaffold/deploy/resource/deployment.go b/pkg/skaffold/deploy/resource/deployment.go index d571df7e70e..28bae52fbb6 100644 --- a/pkg/skaffold/deploy/resource/deployment.go +++ b/pkg/skaffold/deploy/resource/deployment.go @@ -140,7 +140,7 @@ func (d *Deployment) CheckStatus(ctx context.Context, cfg kubectl.Config) { d.UpdateStatus(ae) if err := d.fetchPods(ctx); err != nil { - logrus.Debugf("pod statuses could be fetched this time due to %s", err) + logrus.Debugf("pod statuses could not be fetched this time due to %s", err) } } diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index 0f9886742e0..90e64dc446b 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -37,6 +37,7 @@ import ( "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" + "github.com/docker/go-connections/nat" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/sirupsen/logrus" @@ -66,6 +67,8 @@ type ContainerCreateOpts struct { Network string VolumesFrom []string Wait bool + Ports nat.PortSet + Bindings nat.PortMap } // LocalDaemon talks to a local Docker API. @@ -75,6 +78,7 @@ type LocalDaemon interface { ServerVersion(ctx context.Context) (types.Version, error) ConfigFile(ctx context.Context, image string) (*v1.ConfigFile, error) ContainerLogs(ctx context.Context, w *io.PipeWriter, id string) error + ContainerExists(ctx context.Context, name string) bool Build(ctx context.Context, out io.Writer, workspace string, artifact string, a *latestV1.DockerArtifact, opts BuildOptions) (string, error) Push(ctx context.Context, out io.Writer, ref string) (string, error) Pull(ctx context.Context, out io.Writer, ref string) error @@ -166,6 +170,11 @@ func (l *localDaemon) ContainerLogs(ctx context.Context, w *io.PipeWriter, id st } } +func (l *localDaemon) ContainerExists(ctx context.Context, name string) bool { + _, err := l.apiClient.ContainerInspect(ctx, name) + return err == nil +} + // Delete stops, removes, and prunes a running container func (l *localDaemon) Delete(ctx context.Context, out io.Writer, id string) error { if err := l.apiClient.ContainerStop(ctx, id, nil); err != nil { @@ -184,12 +193,14 @@ func (l *localDaemon) Delete(ctx context.Context, out io.Writer, id string) erro // Run creates a container from a given image reference, and returns then container ID. func (l *localDaemon) Run(ctx context.Context, out io.Writer, opts ContainerCreateOpts) (string, error) { cfg := &container.Config{ - Image: opts.Image, + Image: opts.Image, + ExposedPorts: opts.Ports, } hCfg := &container.HostConfig{ - NetworkMode: container.NetworkMode(opts.Network), - VolumesFrom: opts.VolumesFrom, + NetworkMode: container.NetworkMode(opts.Network), + VolumesFrom: opts.VolumesFrom, + PortBindings: opts.Bindings, } c, err := l.apiClient.ContainerCreate(ctx, cfg, hCfg, nil, nil, opts.Name) if err != nil { @@ -552,6 +563,7 @@ func (l *localDaemon) ImageList(ctx context.Context, ref string) ([]types.ImageS Filters: filters.NewArgs(filters.Arg("reference", ref)), }) } + func (l *localDaemon) DiskUsage(ctx context.Context) (uint64, error) { usage, err := l.apiClient.DiskUsage(ctx) if err != nil { diff --git a/pkg/skaffold/docker/logger/formatter_test.go b/pkg/skaffold/docker/logger/formatter_test.go index 62bee3a06c6..64dab1667e0 100644 --- a/pkg/skaffold/docker/logger/formatter_test.go +++ b/pkg/skaffold/docker/logger/formatter_test.go @@ -77,7 +77,7 @@ func TestPrintLogLineFormatted(t *testing.T) { groups = 5 ) ct := tracker.NewContainerTracker() - ct.Add(graph.Artifact{ImageName: "image", Tag: "image:tag"}, "id") + ct.Add(graph.Artifact{ImageName: "image", Tag: "image:tag"}, tracker.Container{ID: "id"}) f := NewDockerLogFormatter(&mockColorPicker{}, ct, func() bool { return false }, "id") diff --git a/pkg/skaffold/docker/tracker/tracker.go b/pkg/skaffold/docker/tracker/tracker.go index 93d3ae96f14..46dc7bb6c2a 100644 --- a/pkg/skaffold/docker/tracker/tracker.go +++ b/pkg/skaffold/docker/tracker/tracker.go @@ -17,26 +17,29 @@ limitations under the License. package tracker import ( - "sort" "sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph" ) +type Container struct { + Name string + ID string +} + type ContainerTracker struct { sync.RWMutex - deployedContainers map[string]string // image name -> container id + deployedContainers map[string]Container // artifact image name -> container + containerToArtifact map[string]graph.Artifact // container id -> artifact (for colorpicker) containers map[string]bool // set of tracked container ids - containerToArtifact map[string]graph.Artifact // container id -> graph.Artifact notifier chan string } // NewContainerTracker creates a new ContainerTracker. func NewContainerTracker() *ContainerTracker { return &ContainerTracker{ - deployedContainers: make(map[string]string), - containers: make(map[string]bool), containerToArtifact: make(map[string]graph.Artifact), + deployedContainers: make(map[string]Container), notifier: make(chan string, 1), } } @@ -48,32 +51,25 @@ func (t *ContainerTracker) Notifier() chan string { return t.notifier } -// ImageForContainer maps a container id to the image tag it was created from. -// Used by the ColorPicker to maintain consistency between images and colors. func (t *ContainerTracker) ArtifactForContainer(id string) graph.Artifact { + t.Lock() + defer t.Unlock() return t.containerToArtifact[id] } -// DeployedContainerForImage returns a the ID of a deployed container created from +// ContainerIdForImage returns the deployed container created from // the provided image, if it exists. -func (t *ContainerTracker) DeployedContainerForImage(image string) string { +func (t *ContainerTracker) ContainerForImage(image string) (Container, bool) { t.Lock() defer t.Unlock() - if id, found := t.deployedContainers[image]; found { - return id - } - return "" + c, found := t.deployedContainers[image] + return c, found } // DeployedContainers returns the list of all containers deployed // during this run. -func (t *ContainerTracker) DeployedContainers() []string { - var containers []string - for _, id := range t.deployedContainers { - containers = append(containers, id) - } - sort.Strings(containers) - return containers +func (t *ContainerTracker) DeployedContainers() map[string]Container { + return t.deployedContainers } // Reset resets the entire tracker when a new dev cycle starts. @@ -84,13 +80,12 @@ func (t *ContainerTracker) Reset() { } // Add adds a container to the list. -func (t *ContainerTracker) Add(artifact graph.Artifact, id string) { +func (t *ContainerTracker) Add(artifact graph.Artifact, c Container) { t.Lock() defer t.Unlock() - t.containers[id] = true - t.deployedContainers[artifact.ImageName] = id - t.containerToArtifact[id] = artifact + t.deployedContainers[artifact.ImageName] = c + t.containerToArtifact[c.ID] = artifact go func() { - t.notifier <- id + t.notifier <- c.ID }() } diff --git a/pkg/skaffold/docker/tracker/tracker_test.go b/pkg/skaffold/docker/tracker/tracker_test.go index ddbf68ae259..ce0edce7a2d 100644 --- a/pkg/skaffold/docker/tracker/tracker_test.go +++ b/pkg/skaffold/docker/tracker/tracker_test.go @@ -71,9 +71,22 @@ func TestDeployedContainers(t *testing.T) { testutil.Run(t, test.name, func(t *testutil.T) { tracker := NewContainerTracker() for _, pair := range test.containers { - tracker.Add(pair.artifact, pair.id) + tracker.Add(pair.artifact, Container{ID: pair.id}) + } + deployedContainers := tracker.DeployedContainers() + + // ensure each container exists in the returned map + for _, c := range test.expectedContainers { + found := false + for _, container := range deployedContainers { + if container.ID == c { + found = true + } + } + if !found { + t.Fail() + } } - t.CheckDeepEqual(test.expectedContainers, tracker.DeployedContainers()) }) } } @@ -131,70 +144,10 @@ func TestDeployedContainerForImage(t *testing.T) { testutil.Run(t, test.name, func(t *testutil.T) { tracker := NewContainerTracker() for _, pair := range test.containers { - tracker.Add(pair.artifact, pair.id) - } - t.CheckDeepEqual(test.expected, tracker.DeployedContainerForImage(test.target)) - }) - } -} - -func TestArtifactForContainer(t *testing.T) { - tests := []struct { - name string - containers []ArtifactIDPair - target string - expected graph.Artifact - }{ - { - name: "one container", - containers: []ArtifactIDPair{{artifact: graph.Artifact{ImageName: "image1"}, id: "deadbeef"}}, - target: "deadbeef", - expected: graph.Artifact{ImageName: "image1"}, - }, - { - name: "two containers, retrieve the second", - containers: []ArtifactIDPair{ - {artifact: graph.Artifact{ImageName: "image1"}, id: "deadbeef"}, - {artifact: graph.Artifact{ImageName: "image2"}, id: "foobar"}, - }, - target: "foobar", - expected: graph.Artifact{ImageName: "image2"}, - }, - { - name: "adding the same artifact overwrites previous ID", - containers: []ArtifactIDPair{ - {artifact: graph.Artifact{ImageName: "image1"}, id: "this will be ignored"}, - {artifact: graph.Artifact{ImageName: "image1"}, id: "deadbeef"}, - {artifact: graph.Artifact{ImageName: "image2"}, id: "foobar"}, - }, - target: "deadbeef", - expected: graph.Artifact{ImageName: "image1"}, - }, - { - name: "tags are updated on artifact", - containers: []ArtifactIDPair{ - {artifact: graph.Artifact{ImageName: "image1", Tag: "image1:tag1"}, id: "deadbeef"}, - {artifact: graph.Artifact{ImageName: "image1", Tag: "image1:tag2"}, id: "deadbeef"}, - {artifact: graph.Artifact{ImageName: "image2"}, id: "foobar"}, - }, - target: "deadbeef", - expected: graph.Artifact{ImageName: "image1", Tag: "image1:tag2"}, - }, - { - name: "untracked id returns empty artifact", - containers: []ArtifactIDPair{{artifact: graph.Artifact{ImageName: "image1"}, id: "deadbeef"}}, - target: "bogus", - expected: graph.Artifact{}, - }, - } - - for _, test := range tests { - testutil.Run(t, test.name, func(t *testutil.T) { - tracker := NewContainerTracker() - for _, pair := range test.containers { - tracker.Add(pair.artifact, pair.id) + tracker.Add(pair.artifact, Container{ID: pair.id}) } - t.CheckDeepEqual(test.expected, tracker.ArtifactForContainer(test.target)) + container, _ := tracker.ContainerForImage(test.target) + t.CheckDeepEqual(test.expected, container.ID) }) } } diff --git a/pkg/skaffold/schema/latest/v1/config.go b/pkg/skaffold/schema/latest/v1/config.go index 859233c080b..0e431a29602 100644 --- a/pkg/skaffold/schema/latest/v1/config.go +++ b/pkg/skaffold/schema/latest/v1/config.go @@ -124,14 +124,15 @@ type ResourceType string // PortForwardResource describes a resource to port forward. type PortForwardResource struct { - // Type is the Kubernetes type that should be port forwarded. - // Acceptable resource types include: `Service`, `Pod` and Controller resource type that has a pod spec: `ReplicaSet`, `ReplicationController`, `Deployment`, `StatefulSet`, `DaemonSet`, `Job`, `CronJob`. + // Type is the resource type that should be port forwarded. + // Acceptable resource types include kubernetes types: `Service`, `Pod` and Controller resource type that has a pod spec: `ReplicaSet`, `ReplicationController`, `Deployment`, `StatefulSet`, `DaemonSet`, `Job`, `CronJob`. + // Standalone `Container` is also valid for Docker deployments. Type ResourceType `yaml:"resourceType,omitempty"` - // Name is the name of the Kubernetes resource to port forward. + // Name is the name of the Kubernetes resource or local container to port forward. Name string `yaml:"resourceName,omitempty"` - // Namespace is the namespace of the resource to port forward. + // Namespace is the namespace of the resource to port forward. Does not apply to local containers. Namespace string `yaml:"namespace,omitempty"` // Port is the resource port that will be forwarded. @@ -523,7 +524,7 @@ type DeployConfig struct { // time for hybrid workflows. type DeployType struct { // DockerDeploy *alpha* uses the `docker` CLI to create application containers in Docker. - DockerDeploy *DockerDeploy `yaml:"-,omitempty"` + DockerDeploy *DockerDeploy `yaml:",omitempty"` // HelmDeploy *beta* uses the `helm` CLI to apply the charts to the cluster. HelmDeploy *HelmDeploy `yaml:"helm,omitempty"` diff --git a/pkg/skaffold/schema/validation/samples_test.go b/pkg/skaffold/schema/validation/samples_test.go index 4b8621eec24..60eec113afd 100644 --- a/pkg/skaffold/schema/validation/samples_test.go +++ b/pkg/skaffold/schema/validation/samples_test.go @@ -38,7 +38,7 @@ const ( ) var ( - ignoredExamples = []string{"docker-deploy"} + ignoredExamples = []string{"docker-deploy", "react-reload-docker"} ignoredSamples = []string{"structureTest.yaml", "build.sh", "globalConfig.yaml", "Dockerfile.app", "Dockerfile.base"} ) diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go index c942b43be15..9b0ec696cf4 100644 --- a/pkg/skaffold/schema/validation/validation.go +++ b/pkg/skaffold/schema/validation/validation.go @@ -482,6 +482,7 @@ func validateSyncRules(artifacts []*latestV1.Artifact) []error { func validatePortForwardResources(pfrs []*latestV1.PortForwardResource) []error { var errs []error validResourceTypes := map[string]struct{}{ + "container": {}, "pod": {}, "deployment": {}, "service": {},