-
Notifications
You must be signed in to change notification settings - Fork 179
Makefile, Vagrantfile: refactor of build system #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7ac7425
e5ea3e8
ecf34e6
d33e2dd
1d74ec5
499c1bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,3 +28,7 @@ _testmain.go | |
|
|
||
| # vagrant | ||
| .vagrant | ||
|
|
||
| bin/* | ||
|
|
||
| resolv.conf | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,8 @@ | |
|
|
||
| # find all verifiable packages. | ||
| # XXX: explore a better way that doesn't need multiple 'find' | ||
| PKGS := `find . -mindepth 1 -maxdepth 1 -type d -name '*' | grep -vE '/\..*$\|Godeps|examples|docs|scripts|mgmtfn|systemtests'` | ||
| PKGS += `find . -mindepth 2 -maxdepth 2 -type d -name '*'| grep -vE '/\..*$\|Godeps|examples|docs|scripts'` | ||
| PKGS := `find . -mindepth 1 -maxdepth 1 -type d -name '*' | grep -vE '/\..*$\|Godeps|examples|docs|scripts|mgmtfn|systemtests|bin'` | ||
| PKGS += `find . -mindepth 2 -maxdepth 2 -type d -name '*'| grep -vE '/\..*$\|Godeps|examples|docs|scripts|bin'` | ||
| TO_BUILD := ./netplugin/ ./netmaster/ ./netdcli/ ./mgmtfn/k8contivnet/ ./mgmtfn/dockcontivnet/ | ||
| HOST_GOBIN := `if [ -n "$$(go env GOBIN)" ]; then go env GOBIN; else dirname $$(which go); fi` | ||
| HOST_GOROOT := `go env GOROOT` | ||
|
|
@@ -34,20 +34,35 @@ deps: | |
| checks: | ||
| ./scripts/checks "$(PKGS)" | ||
|
|
||
| build: deps checks | ||
| host-build: | ||
| sudo /bin/bash -c 'source /etc/profile.d/envvar.sh; make run-build' | ||
|
|
||
| run-build: deps checks clean | ||
| godep go install -v $(TO_BUILD) | ||
|
|
||
| build: start | ||
| vagrant ssh netplugin-node1 -c 'sudo -i bash -lc "source /etc/profile.d/envvar.sh && cd /opt/golang/src/github.com/contiv/netplugin && make run-build"' | ||
| make stop | ||
|
|
||
| clean: deps | ||
| rm -rf Godeps/_workspace/pkg | ||
| godep go clean -i -v ./... | ||
|
|
||
| update: | ||
| vagrant box update | ||
|
|
||
| # setting CONTIV_NODES=<number> while calling 'make demo' can be used to bring | ||
| # up a cluster of <number> nodes. By default <number> = 1 | ||
| demo: build | ||
| CONTIV_HOST_GOBIN=$(HOST_GOBIN) CONTIV_HOST_GOROOT=$(HOST_GOROOT) vagrant up | ||
| start: update | ||
| CONTIV_NODE_OS=${CONTIV_NODE_OS} vagrant up | ||
|
|
||
| demo-centos: | ||
| CONTIV_NODE_OS=centos make demo | ||
|
|
||
| stop: | ||
| CONTIV_NODES=$${CONTIV_NODES:-2} vagrant destroy -f | ||
|
|
||
| clean-demo: | ||
| vagrant destroy -f | ||
| demo: stop start | ||
|
|
||
| start-dockerdemo: | ||
| scripts/dockerhost/start-dockerhosts | ||
|
|
@@ -58,38 +73,41 @@ clean-dockerdemo: | |
| ssh: | ||
| @vagrant ssh netplugin-node1 || echo 'Please run "make demo"' | ||
|
|
||
| unit-test: build | ||
| CONTIV_HOST_GOPATH=$(GOPATH) CONTIV_HOST_GOBIN=$(HOST_GOBIN) \ | ||
| CONTIV_HOST_GOROOT=$(HOST_GOROOT) ./scripts/unittests -vagrant | ||
| unit-test: stop clean build | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC from discussion in #130 (here: #130 (comment)) the need for doing a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In short, this is quite a bit slower but I think it's worth the trouble. Without it, the build will break between multi-os tests. If you build on centos, you're going to get a centos-friendly binary in I don't know what the HOST flags are solving here. The goal is to make our CI line up with our real-world environments. Host flags like this change the build toolchain, which may not be what's in the VM.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, my concern is mostly around increased test times and you bring up a good point around consistent toolchain. We can keep this as is for now until we run into slow build/test times. |
||
| ./scripts/unittests -vagrant | ||
|
|
||
| unit-test-centos: build | ||
| CONTIV_NODE_OS=centos CONTIV_HOST_GOPATH=$(GOPATH) CONTIV_HOST_GOBIN=$(HOST_GOBIN) \ | ||
| CONTIV_HOST_GOROOT=$(HOST_GOROOT) ./scripts/unittests -vagrant | ||
| unit-test-centos: stop clean | ||
| CONTIV_NODE_OS=centos make build | ||
| CONTIV_NODE_OS=centos ./scripts/unittests -vagrant | ||
|
|
||
| # setting CONTIV_SOE=1 while calling 'make system-test' will stop the test | ||
| # on first failure and leave setup in that state. This can be useful for debugging | ||
| # as part of development. | ||
| system-test: build | ||
| CONTIV_HOST_GOROOT=$(HOST_GOROOT) CONTIV_HOST_GOBIN=$(HOST_GOBIN) CONTIV_HOST_GOPATH=$(GOPATH) godep go test --timeout 30m -run "sanity" \ | ||
| system-test: system-test-singlehost system-test-multihost | ||
|
|
||
| # the `make stop` here and below are necessary because build leaves around a VM (intentionally) | ||
| system-test-singlehost: stop clean build | ||
| make stop | ||
| godep go test -v --timeout 30m -run "sanity" \ | ||
| github.com/contiv/netplugin/systemtests/singlehost | ||
| CONTIV_HOST_GOROOT=$(HOST_GOROOT) CONTIV_HOST_GOBIN=$(HOST_GOBIN) CONTIV_HOST_GOPATH=$(GOPATH) godep go test --timeout 80m -run "sanity" \ | ||
| github.com/contiv/netplugin/systemtests/twohosts | ||
|
|
||
| system-test-centos: build | ||
| CONTIV_NODE_OS=centos CONTIV_HOST_GOPATH=$(GOPATH) godep go test --timeout 30m -run "sanity" \ | ||
| github.com/contiv/netplugin/systemtests/singlehost | ||
| CONTIV_NODE_OS=centos CONTIV_HOST_GOPATH=$(GOPATH) godep go test --timeout 90m -run "sanity" \ | ||
| system-test-multihost: stop clean | ||
| make build stop | ||
| godep go test -v --timeout 80m -run "sanity" \ | ||
| github.com/contiv/netplugin/systemtests/twohosts | ||
|
|
||
| system-test-centos: stop clean | ||
| CONTIV_NODE_OS=centos make build stop system-test-singlehost system-test-multihost | ||
|
|
||
| centos-tests: unit-test-centos system-test-centos | ||
|
|
||
| # setting CONTIV_SOE=1 while calling 'make regress-test' will stop the test | ||
| # on first failure and leave setup in that state. This can be useful for debugging | ||
| # as part of development. | ||
| regress-test: build | ||
| CONTIV_HOST_GOPATH=$(GOPATH) godep go test -run "regress" \ | ||
| godep go test -run "regress" \ | ||
| github.com/contiv/netplugin/systemtests/singlehost | ||
| CONTIV_HOST_GOPATH=$(GOPATH) godep go test --timeout 60m -run "regress" \ | ||
| godep go test --timeout 60m -run "regress" \ | ||
| github.com/contiv/netplugin/systemtests/twohosts | ||
|
|
||
| # Setting CONTIV_TESTBED=DIND uses docker in docker as the testbed instead of vagrant VMs. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,8 @@ Please do not use this code in production, until code goes through more testing | |
|
|
||
| ###Building and Testing | ||
|
|
||
| **Note:** Vagrant 1.7.4 and VirtualBox 5.0+ are required to build and test netplugin. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's wait for him to answer -- I think he said he was going on vacation. We can always merge that after the fact.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good. We can merge just this in for now then. |
||
|
|
||
| - Build: | ||
|
|
||
| `make build` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,90 +1,59 @@ | ||
| # -*- mode: ruby -*- | ||
| # vi: set ft=ruby : | ||
|
|
||
| require 'fileutils' | ||
|
|
||
| netplugin_synced_gopath="/opt/golang" | ||
| host_gobin_path="/opt/go/bin" | ||
| host_goroot_path="/opt/go/root" | ||
| FileUtils.cp "/etc/resolv.conf", Dir.pwd | ||
|
|
||
| provision_common = <<SCRIPT | ||
| ## setup the environment file. Export the env-vars passed as args to 'vagrant up' | ||
| echo Args passed: [[ $@ ]] | ||
|
|
||
| echo -n "$1" > /etc/hostname | ||
| hostname -F /etc/hostname | ||
|
|
||
| /sbin/ip addr add "$3/24" dev eth1 | ||
| /sbin/ip link set eth1 up | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not keep using the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's still in there... https://github.com/contiv/netplugin/pull/133/files#diff-23b6f443c01ea2efcb4f36eedfea9089R40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I just meant we could use CONTIV_ENV to pass proxy info, something like below: With current change we seem to be special casing proxy related environment info below. But thinking more, special casing these simplifies the instrumentation needed in the builds. So it shall be ok to keep what we have here, up to you.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The systemd stuff is very finicky (e.g., not even putting it into a shell profile and telling systemd to source it works) and was really hard to get right. I'd prefer to just leave it alone until someone really has a problem with it. |
||
| /sbin/ip link set eth2 up | ||
|
|
||
| echo 'export GOPATH=#{netplugin_synced_gopath}' > /etc/profile.d/envvar.sh | ||
| echo 'export GOBIN=$GOPATH/bin' >> /etc/profile.d/envvar.sh | ||
| echo 'export GOSRC=$GOPATH/src' >> /etc/profile.d/envvar.sh | ||
| echo 'export GOROOT=#{host_goroot_path}' >> /etc/profile.d/envvar.sh | ||
| echo 'export PATH=$PATH:#{host_goroot_path}/bin:#{host_gobin_path}:$GOBIN' >> /etc/profile.d/envvar.sh | ||
| if [ $# -gt 0 ]; then | ||
| echo "export $@" >> /etc/profile.d/envvar.sh | ||
| fi | ||
| echo 'export PATH=$PATH:/usr/local/go/bin:$GOBIN' >> /etc/profile.d/envvar.sh | ||
| echo "export http_proxy='$4'" >> /etc/profile.d/envvar.sh | ||
| echo "export https_proxy='$5'" >> /etc/profile.d/envvar.sh | ||
| echo "export no_proxy=192.168.0.0/16,localhost,127.0.0.0/8" >> /etc/profile.d/envvar.sh | ||
|
|
||
| source /etc/profile.d/envvar.sh | ||
|
|
||
| ## set the mounted host filesystems to be read-only.Just a safety check | ||
| ## to prevent inadvertent modifications from vm. | ||
| (mount -o remount,ro,exec,norelatime /vagrant) || exit 1 | ||
| if [ -e #{host_gobin_path} ]; then | ||
| (mount -o remount,ro,exec,norelatime #{host_gobin_path}) || exit 1 | ||
| fi | ||
| if [ -e #{host_goroot_path} ]; then | ||
| (mount -o remount,ro,exec,norelatime #{host_goroot_path}) || exit 1 | ||
| fi | ||
| if [ -e #{netplugin_synced_gopath} ]; then | ||
| (mount -o remount,ro,exec,norelatime #{netplugin_synced_gopath}) || exit 1 | ||
| fi | ||
| mv /etc/resolv.conf /etc/resolv.conf.bak | ||
| cp #{netplugin_synced_gopath}/src/github.com/contiv/netplugin/resolv.conf /etc/resolv.conf | ||
|
|
||
| ### install basic packages | ||
| #(apt-get update -qq > /dev/null && apt-get install -y vim curl python-software-properties git > /dev/null) || exit 1 | ||
| # | ||
| ### install Go 1.4 | ||
| #(cd /usr/local/ && \ | ||
| #curl -L https://storage.googleapis.com/golang/go1.4.linux-amd64.tar.gz -o go1.4.linux-amd64.tar.gz && \ | ||
| #tar -xzf go1.4.linux-amd64.tar.gz) || exit 1 | ||
| # | ||
| ### install etcd | ||
| #(cd /tmp && \ | ||
| #curl -L https://github.com/coreos/etcd/releases/download/v2.0.0/etcd-v2.0.0-linux-amd64.tar.gz -o etcd-v2.0.0-linux-amd64.tar.gz && \ | ||
| #tar -xzf etcd-v2.0.0-linux-amd64.tar.gz && \ | ||
| #mv /tmp/etcd-v2.0.0-linux-amd64/etcd /usr/bin/ && \ | ||
| #mv /tmp/etcd-v2.0.0-linux-amd64/etcdctl /usr/bin/ ) || exit 1 | ||
| # | ||
| ### install and start docker | ||
| #(curl -sSL https://get.docker.com/ubuntu/ | sh > /dev/null) || exit 1 | ||
| # | ||
| ## pass the env-var args to docker and restart the service. This helps passing | ||
| ## stuff like http-proxy etc | ||
| if [ $# -gt 0 ]; then | ||
| (echo "export $@" >> /etc/default/docker) || exit 1 | ||
| mkdir /etc/systemd/system/docker.service.d | ||
| echo "[Service]" | sudo tee -a /etc/systemd/system/docker.service.d/http-proxy.conf | ||
| echo "Environment=\\\"no_proxy=192.168.0.0/16,localhost,127.0.0.0/8\\\" \\\"http_proxy=$http_proxy\\\" \\\"https_proxy=$https_proxy\\\"" | sudo tee -a /etc/systemd/system/docker.service.d/http-proxy.conf | ||
| sudo systemctl daemon-reload | ||
| sudo systemctl stop docker | ||
| sudo systemctl start docker | ||
|
|
||
| if [ $# -gt 5 ]; then | ||
| shift; shift; shift; shift; shift | ||
| echo "export $@" >> /etc/profile.d/envvar.sh | ||
| fi | ||
| (service docker restart) || exit 1 | ||
|
|
||
| ## install openvswitch and enable ovsdb-server to listen for incoming requests | ||
| #(apt-get install -y openvswitch-switch > /dev/null) || exit 1 | ||
| ## Install OVS 2.3.1 | ||
| # (wget -nv -O ovs-common.deb https://cisco.box.com/shared/static/v1dvgoboo5zgqrtn6tu27vxeqtdo2bdl.deb && | ||
| # wget -nv -O ovs-switch.deb https://cisco.box.com/shared/static/ymbuwvt2qprs4tquextw75b82hyaxwon.deb) || exit 1 | ||
| # (dpkg -i ovs-common.deb && | ||
| # dpkg -i ovs-switch.deb) || exit 1 | ||
| (service docker restart) || exit 1 | ||
|
|
||
| (ovs-vsctl set-manager tcp:127.0.0.1:6640 && \ | ||
| ovs-vsctl set-manager ptcp:6640) || exit 1 | ||
|
|
||
| ### install consul | ||
| #(apt-get install -y unzip && cd /tmp && \ | ||
| # wget https://dl.bintray.com/mitchellh/consul/0.5.2_linux_amd64.zip && \ | ||
| # unzip 0.5.2_linux_amd64.zip && \ | ||
| # mv /tmp/consul /usr/bin) || exit 1 | ||
|
|
||
| # add vagrant user to docker group | ||
| (usermod -a -G docker vagrant) | ||
|
|
||
| SCRIPT | ||
|
|
||
| VAGRANTFILE_API_VERSION = "2" | ||
| Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| | ||
| if ENV['CONTIV_NODE_OS'] && ENV['CONTIV_NODE_OS'] == "centos" then | ||
| config.vm.box = "contiv/centos" | ||
| config.vm.box = "contiv/centos71-netplugin" | ||
| else | ||
| config.vm.box = "contiv/ubuntu-v4" | ||
| config.vm.box = "contiv/ubuntu1504-netplugin" | ||
| end | ||
| num_nodes = 1 | ||
| if ENV['CONTIV_NODES'] && ENV['CONTIV_NODES'] != "" then | ||
|
|
@@ -93,12 +62,12 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| | |
| base_ip = "192.168.2." | ||
| node_ips = num_nodes.times.collect { |n| base_ip + "#{n+10}" } | ||
| node_names = num_nodes.times.collect { |n| "netplugin-node#{n+1}" } | ||
| node_peers = [] | ||
|
|
||
| num_nodes.times do |n| | ||
| node_name = node_names[n] | ||
| node_addr = node_ips[n] | ||
| node_peers = "" | ||
| node_ips.length.times { |i| node_peers += "#{node_names[i]}=http://#{node_ips[i]}:2380 "} | ||
| node_peers = node_peers.strip().gsub(' ', ',') | ||
| node_peers += ["#{node_name}=http://#{node_addr}:2380,#{node_name}=http://#{node_addr}:7001"] | ||
| consul_join_flag = if n > 0 then "-join #{node_ips[0]}" else "" end | ||
| consul_bootstrap_flag = "-bootstrap-expect=3" | ||
| if num_nodes < 3 then | ||
|
|
@@ -109,11 +78,11 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| | |
| end | ||
| end | ||
| config.vm.define node_name do |node| | ||
| node.vm.hostname = node_name | ||
| # node.vm.hostname = node_name | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: suggest to remove the commented code
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I'll do that, sorry. |
||
| # create an interface for etcd cluster | ||
| node.vm.network :private_network, ip: node_addr, virtualbox__intnet: "true" | ||
| node.vm.network :private_network, ip: node_addr, virtualbox__intnet: "true", auto_config: false | ||
| # create an interface for bridged network | ||
| node.vm.network :private_network, ip: "0.0.0.0", virtualbox__intnet: "true" | ||
| node.vm.network :private_network, ip: "0.0.0.0", virtualbox__intnet: "true", auto_config: false | ||
| node.vm.provider "virtualbox" do |v| | ||
| # make all nics 'virtio' to take benefit of builtin vlan tag | ||
| # support, which otherwise needs to be enabled in Intel drivers, | ||
|
|
@@ -125,45 +94,25 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config| | |
| v.customize ['modifyvm', :id, '--nicpromisc3', 'allow-all'] | ||
| end | ||
| # mount the host directories | ||
| node.vm.synced_folder ".", "/vagrant" | ||
| # godep modifies the host's GOPATH env variable, CONTIV_HOST_GOPATH | ||
| # contains the unmodified path passed from the Makefile, use that | ||
| # when it is defined. | ||
| if ENV['CONTIV_HOST_GOPATH'] != nil | ||
| node.vm.synced_folder ENV['CONTIV_HOST_GOPATH'], netplugin_synced_gopath | ||
| else | ||
| node.vm.synced_folder ENV['GOPATH'], netplugin_synced_gopath | ||
| end | ||
| if ENV['CONTIV_HOST_GOBIN'] != nil | ||
| node.vm.synced_folder ENV['CONTIV_HOST_GOBIN'], host_gobin_path | ||
| end | ||
| if ENV['CONTIV_HOST_GOROOT'] != nil | ||
| node.vm.synced_folder ENV['CONTIV_HOST_GOROOT'], host_goroot_path | ||
| end | ||
| node.vm.synced_folder ".", "/opt/golang/src/github.com/contiv/netplugin" | ||
| node.vm.synced_folder File.join(File.dirname(__FILE__), "bin"), File.join(netplugin_synced_gopath, "bin") | ||
|
|
||
| node.vm.provision "shell" do |s| | ||
| s.inline = provision_common | ||
| s.args = ENV['CONTIV_ENV'] | ||
| s.args = [node_name, ENV["CONTIV_NODE_OS"] || "", node_addr, ENV["http_proxy"] || "", ENV["https_proxy"] || "", *ENV['CONTIV_ENV']] | ||
| end | ||
| provision_node = <<SCRIPT | ||
| ## start etcd with generated config | ||
| (echo etcd -name #{node_name} -data-dir /opt/etcd \ | ||
| -listen-client-urls http://0.0.0.0:2379,http://0.0.0.0:4001 \ | ||
| -advertise-client-urls http://#{node_addr}:2379,http://#{node_addr}:4001 \ | ||
| -initial-advertise-peer-urls http://#{node_addr}:2380 \ | ||
| -listen-peer-urls http://#{node_addr}:2380 \ | ||
| -initial-cluster #{node_peers} \ | ||
| -initial-cluster-state new) | ||
| (nohup etcd -name #{node_name} -data-dir /opt/etcd \ | ||
| -listen-client-urls http://0.0.0.0:2379,http://0.0.0.0:4001 \ | ||
| -advertise-client-urls http://#{node_addr}:2379,http://#{node_addr}:4001 \ | ||
| -initial-advertise-peer-urls http://#{node_addr}:2380 \ | ||
| -listen-peer-urls http://#{node_addr}:2380 \ | ||
| -initial-cluster #{node_peers} \ | ||
| -initial-cluster-state new 0<&- &>/tmp/etcd.log &) || exit 1 | ||
| set -x | ||
| (nohup etcd --name #{node_name} --data-dir /tmp/etcd \ | ||
| --listen-client-urls http://0.0.0.0:2379,http://0.0.0.0:4001 \ | ||
| --advertise-client-urls http://#{node_addr}:2379,http://#{node_addr}:4001 \ | ||
| --initial-advertise-peer-urls http://#{node_addr}:2380,http://#{node_addr}:7001 \ | ||
| --listen-peer-urls http://#{node_addr}:2380 \ | ||
| --initial-cluster #{node_peers.join(",")} --initial-cluster-state new \ | ||
| 0<&- &>/tmp/etcd.log &) || exit 1 | ||
|
|
||
| ## start consul | ||
| (echo && echo consul agent -server #{consul_join_flag} #{consul_bootstrap_flag} \ | ||
| -bind=#{node_addr} -data-dir /opt/consul) | ||
| (nohup consul agent -server #{consul_join_flag} #{consul_bootstrap_flag} \ | ||
| -bind=#{node_addr} -data-dir /opt/consul 0<&- &>/tmp/consul.log &) || exit 1 | ||
| SCRIPT | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should stop the vm once build is done, otherwise it will stay running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem with that is that each build will trigger a vm restart -- this is very slow for a build/test cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, but if user were to just do
make buildand stop. They won't know that we left a vm running behind. And If they decide to yank out their workspace and pull a new one, this vm will stay there consuming resources.I might be coming out a bit paranoid but cleanup somewhat seems a lot safer at expense of spending some time :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this is any different from typical vagrant usage -- if someone does that without stopping the VM, they will be stuck with the same solution,
pkill VBoxManageroughly.I understand your desire but we have to figure out whether keeping the build time down or protecting the user is more important here. I don't know if there's a way to get the best of both worlds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is not different in behavior. The only difference is wrt user's experience. In case of doing
vagrant upand then deleting the workspace we can consider it to be error on user's part if it leaves behind a vm. But if a user doesmake buildand then delete their workspace which leaves behind a vm, it is not an error on their part.You are right there is no easy way to achieve both in this case i.e. protect the user or improved build time. We can may be print a informative message about the running vm after build to alleviate this a bit. For now, I am ok starting with what we have here as it does solves the builds on mac.
ping @jainvipin @shaleman as well to get their thoughts.