Implement port forwarding for Docker deployer (GoogleContainerTools#6303

)

* Implement port forwarding for Docker deployer

* schemas

* linters

* use PortManager as Accessor

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: <code>Service</code>, <code>Pod</code> and Controller resource type that has a pod spec: <code>ReplicaSet</code>, <code>ReplicationController</code>, <code>Deployment</code>, <code>StatefulSet</code>, <code>DaemonSet</code>, <code>Job</code>, <code>CronJob</code>."
+          "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: <code>Service</code>, <code>Pod</code> and Controller resource type that has a pod spec: <code>ReplicaSet</code>, <code>ReplicationController</code>, <code>Deployment</code>, <code>StatefulSet</code>, <code>DaemonSet</code>, <code>Job</code>, <code>CronJob</code>. Standalone <code>Container</code> is also valid for Docker deployments."
         }
       },
       "preferredOrder": [

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())
 		}
 	}

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`

integration/examples/react-reload-docker/app/.babelrc

@@ -0,0 +1,6 @@
+{
+    "presets": [
+        "@babel/preset-env",
+        "@babel/preset-react"
+    ]
+}

integration/examples/react-reload-docker/app/.dockerignore

@@ -0,0 +1,2 @@
+node_modules
+*.swp

integration/examples/react-reload-docker/app/.gitignore

@@ -0,0 +1,2 @@
+/node_modules/
+package-lock.json

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 . .

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"
+}

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 = () => (
+	<div>
+		<h1>Hello world!</h1>
+	</div>
+);

integration/examples/react-reload-docker/app/src/index.html

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1.0">
+    <title>React demo app for skaffold</title>
+</head>
+<body>
+    <div id="root"/>
+</body>
+</html>

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' ) );

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;
+}

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' } ) ]
+};

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

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

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,27 +68,28 @@ 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
 }
 
 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 {