Build versioned v2plugin from versioned binaries #1040

chrisplo · 2017-10-31T19:47:47Z

This enables build and caching locally or in a build machine the
v2plugin before creating test environment VMs and failing much faster
on any PR issues, which is good for CI turnaround.

v2plugin_rootfs (called by host-pluginfs-create) builds a docker
container runtime with a versioned archive of netplugin binaries, then
extracts the entire filesystem from the container, which is now left as
a versioned tarball.

The versioned archive of netplugin binaries used for v2plugin can now be
created in two ways:

run-build (dependency for demo-v2plugin) will compile then version an
archive with built assets (runs only on the VM)
tar target (locally or in VM)

host-pluginfs-create can be run in the VM or locally

Leaving the plugin rootfs as a tarball has a few advantages reasons:

docker in a VM had internal issues when creating the plugin when the
rootfs was unarchived on the host and exposed though a virtualbox
mount, at least on OS X
the rootfs is versioned

This adjusted host-plugin-release and demo-v2plugin to both use a new
target 'host-pluginfs-unpack'

Drive-by:

Fix Dockerfile by adding objdb which is needed by vendored packages
Fix config.template for the CONTIV_V2PLUGIN_NAME replacement

chrisplo · 2017-10-31T20:12:17Z

checking on a version naming bug during testing

chrisplo · 2017-11-01T00:01:33Z

~~#1042 helps to sort out version naming issues, will rebase after that lands but early comments welcome~~ done

unclejack · 2017-11-01T11:44:35Z

Makefile

@@ -129,7 +134,7 @@ update:
 # setting CONTIV_NODES=<number> while calling 'make demo' can be used to bring
 # up a cluster of <number> nodes. By default <number> = 1
 start:
-	CONTIV_DOCKER_VERSION="$${CONTIV_DOCKER_VERSION:-$(DEFAULT_DOCKER_VERSION)}" CONTIV_NODE_OS=${CONTIV_NODE_OS} vagrant up


Why are you making this change here? It seems unrelated to the purpose of the PR.

was reducing duplication (the build.sh defaults this to same value), but unrelated to PR, backing this out

unclejack · 2017-11-01T11:46:03Z

Makefile

@@ -6,7 +6,6 @@
 .PHONY: all all-CI build clean default unit-test release tar checks go-version gofmt-src \
 	golint-src govet-src run-build compile-with-docker

-DEFAULT_DOCKER_VERSION := 1.12.6


Please leave this alone.

will back this change out

unclejack

The Docker version related changes need to be removed. We need to be able to support specifying custom Docker version. We'll use this again soon.

unclejack · 2017-11-01T11:50:12Z

Makefile

@@ -213,7 +218,7 @@ ssh:
 	@vagrant ssh netplugin-node1 -c 'bash -lc "cd /opt/gopath/src/github.com/contiv/netplugin/ && bash"' || echo 'Please run "make demo"'

 ssh-build: start
-	vagrant ssh netplugin-node1 -c 'bash -lc "source /etc/profile.d/envvar.sh && cd /opt/gopath/src/github.com/contiv/netplugin && make run-build install-shell-completion"'


This change doesn't seem related to the goal of this PR. Can you split this in a separate cleanup commit instead, please?

unclejack · 2017-11-01T11:54:09Z

The permissions of some of the files have been changed. Are those changes intentional or accidental?

chrisplo · 2017-11-01T18:04:09Z

the jenkins file was unintentional, the bash script was intentional

~~I'm holding working on this till #1042 is resolved~~ 1042 merged

This enables build and docker image caching on a build machine for the v2plugin. The new target demo-v2plugin-from-local does everything it can locally before spawning the VM, failing much faster on any PR issues, which is good for CI turnaround. It runs a little faster than demo-v2plugin after base image layers are cached. v2plugin_rootfs (called by host-pluginfs-create) builds a docker container runtime with a versioned archive of netplugin binaries, then extracts the entire filesystem from the container, which is now left as a versioned tarball. The versioned archive of netplugin binaries used for v2plugin can now be created in two ways: * run-build (dependency for demo-v2plugin) will compile then version an archive with built assets (runs only on the VM) * tar target (locally or in VM) host-pluginfs-create can be run in the VM or locally The plugin rootfs is left as a tarball because: * docker in a VM had internal issues when creating the plugin when the rootfs was unarchived on the host and exposed though a virtualbox mount, at least on OS X * the rootfs is versioned This adjusted host-plugin-release and demo-v2plugin both use a new target 'host-pluginfs-unpack' Drive-by: * Fix config.template for the CONTIV_V2PLUGIN_NAME replacement * gtar support for OS X * use cp -a for maintaining owner Signed-off-by: Chris Plock <[email protected]>

chrisplo · 2017-11-06T19:17:56Z

build PR

unclejack · 2017-11-06T22:05:01Z

build PR

unclejack · 2017-11-07T21:02:45Z

build PR

chrisplo · 2017-11-08T16:41:42Z

build PR

chrisplo · 2017-11-08T20:51:27Z

build PR

dseevr · 2017-11-08T22:09:06Z

Makefile

+		host-plugin-restart host-swarm-restart)
+
+# Just like demo-v2plugin except builds are done locally and cached
+demo-v2plugin-from-local: export CONTIV_DOCKER_VERSION ?= 1.13.1


Can this var and the one for CONTIV_DOCKER_VERSION under demo-v2plugin below be consolidated into a variable at the top of the file so it only needs to be changed in one place?

dseevr · 2017-11-08T22:10:24Z

install/v2plugin/Dockerfile

-COPY netplugin netmaster netctl startcontiv.sh /
+# copy in binaries and scripts
+ARG TAR_FILE
+ADD ${TAR_FILE} /


If you're copying local files, use COPY instead of ADD ... generally you will not want to use ADD for anything, and all of Docker's official Dockerfiles have moved from ADD to COPY now

the ADD exploded the tar file, I'd have to create another layer for a RUN to expand, there is nothing suggesting deprecation on the docs:
https://docs.docker.com/engine/reference/builder/#add

Oh, right. "TAR_FILE" lol (used to looking for .tar.gz and so on) 😂

dseevr · 2017-11-08T22:11:10Z

Do we need to worry about spaces in paths with any of this stuff?

chrisplo · 2017-11-08T23:22:11Z

build PR

chrisplo · 2017-11-08T23:35:26Z

only place I could see that isn't a subdirectory is here, which was a pre-existing issue in many placements if GOPATH has spaces:
https://github.com/contiv/netplugin/pull/1040/files#diff-b67911656ef5d18c4ae36cb6741b7965R320

chrisplo · 2017-11-08T23:40:22Z

build PR

dseevr · 2017-11-09T17:38:51Z

.dockerignore

@@ -1,11 +1,15 @@
 # we want to get the git commit SHA but not all the binary repo data
 .git/objects/pack
 bin/
+**/*.pyc


Forgot to mention this earlier... could you do a git rm **/*.pyc and see if anything is currently checked in which shouldn't be? Could also be in a separate PR

dseevr · 2017-11-09T17:38:57Z

.dockerignore

 *.tar
+**/*.tar.gz


Same on this guy

unclejack

LGTM

chrisplo force-pushed the v2plugin_local_versioned branch from addec2e to e72937c Compare October 31, 2017 19:49

chrisplo force-pushed the v2plugin_local_versioned branch from e72937c to ecd69e3 Compare October 31, 2017 20:45

chrisplo changed the title ~~Build versioned v2plugin from versioned binaries~~ WIP: Build versioned v2plugin from versioned binaries Oct 31, 2017

unclejack reviewed Nov 1, 2017

View reviewed changes

unclejack requested changes Nov 1, 2017

View reviewed changes

unclejack reviewed Nov 1, 2017

View reviewed changes

chrisplo force-pushed the v2plugin_local_versioned branch 3 times, most recently from a89707c to b947648 Compare November 3, 2017 21:53

chrisplo added 4 commits November 3, 2017 16:13

review feedback

0ce1f51

comments, revert a var rename for smaller diff

f750f16

more dockerignore

13fcf40

chrisplo force-pushed the v2plugin_local_versioned branch from b947648 to 13fcf40 Compare November 3, 2017 23:25

comments, and don't remove rootfs

ca1b582

chrisplo changed the title ~~WIP: Build versioned v2plugin from versioned binaries~~ Build versioned v2plugin from versioned binaries Nov 3, 2017

chrisplo added the squash when merging label Nov 3, 2017

review testing feedback binaries-from-container and host-plugin-update

8446866

simplify usage for v2plugin-update

be82f8c

dseevr reviewed Nov 8, 2017

View reviewed changes

feedback for v2plugin docker version variable

a86597d

dseevr approved these changes Nov 9, 2017

View reviewed changes

unclejack approved these changes Nov 9, 2017

View reviewed changes

unclejack merged commit 06fa667 into contiv:master Nov 9, 2017

chrisplo deleted the v2plugin_local_versioned branch November 9, 2017 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build versioned v2plugin from versioned binaries #1040

Build versioned v2plugin from versioned binaries #1040

chrisplo commented Oct 31, 2017 •

edited

Loading

chrisplo commented Oct 31, 2017 •

edited

Loading

chrisplo commented Nov 1, 2017 •

edited

Loading

unclejack Nov 1, 2017

chrisplo Nov 3, 2017 •

edited

Loading

unclejack Nov 1, 2017

chrisplo Nov 3, 2017

unclejack left a comment •

edited

Loading

unclejack Nov 1, 2017

unclejack commented Nov 1, 2017

chrisplo commented Nov 1, 2017 •

edited

Loading

chrisplo commented Nov 6, 2017

unclejack commented Nov 6, 2017

unclejack commented Nov 7, 2017

chrisplo commented Nov 8, 2017

chrisplo commented Nov 8, 2017

dseevr Nov 8, 2017

dseevr Nov 8, 2017

chrisplo Nov 8, 2017

dseevr Nov 9, 2017

dseevr commented Nov 8, 2017

chrisplo commented Nov 8, 2017

chrisplo commented Nov 8, 2017 •

edited

Loading

chrisplo commented Nov 8, 2017

dseevr Nov 9, 2017

dseevr Nov 9, 2017

unclejack left a comment

		*.tar
		*/.tar.gz

Build versioned v2plugin from versioned binaries #1040

Build versioned v2plugin from versioned binaries #1040

Conversation

chrisplo commented Oct 31, 2017 • edited Loading

chrisplo commented Oct 31, 2017 • edited Loading

chrisplo commented Nov 1, 2017 • edited Loading

Choose a reason for hiding this comment

chrisplo Nov 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unclejack left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unclejack commented Nov 1, 2017

chrisplo commented Nov 1, 2017 • edited Loading

chrisplo commented Nov 6, 2017

unclejack commented Nov 6, 2017

unclejack commented Nov 7, 2017

chrisplo commented Nov 8, 2017

chrisplo commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dseevr commented Nov 8, 2017

chrisplo commented Nov 8, 2017

chrisplo commented Nov 8, 2017 • edited Loading

chrisplo commented Nov 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unclejack left a comment

Choose a reason for hiding this comment

chrisplo commented Oct 31, 2017 •

edited

Loading

chrisplo commented Oct 31, 2017 •

edited

Loading

chrisplo commented Nov 1, 2017 •

edited

Loading

chrisplo Nov 3, 2017 •

edited

Loading

unclejack left a comment •

edited

Loading

chrisplo commented Nov 1, 2017 •

edited

Loading

chrisplo commented Nov 8, 2017 •

edited

Loading