Skip to content
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.

Sign up for GitHub

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

Fix build macos #33

Closed
wants to merge 2 commits into from
Closed

Fix build macos #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f08358d
Provide errors properly
Mar 23, 2015
e01b8b0
Fix builds on OS X
Mar 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ _testmain.go
*.exe
*.test
*.prof

.vagrant
8 changes: 5 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
.PHONY: all build clean default system-test unit-test

TO_BUILD := ./ ./netdcli/
TO_BUILD := ./netmaster/ ./netdcli/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikh - tried out your changes;
Comment1:

  • this change is not only not needed, but will not build the netplugin binary appropriately. netplugin binary, which comes from netd.go is in ./ direcotry and not ./netmaster. Unless we move the file locations, let's avoid this change.

ifeq ($(uname -s),Linux)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikh - tried out your changes:
Comment2:
Replace this line with following instead, basically $(uname -s) doesn't work unless qualified by shell.
+ifeq ($(shell uname -s),Linux)

One more diff in the Makefile (for some reason it was taking longer to finish, I don't think it has to do with your changes):

  •   go test --timeout 20m -v -run "sanity" github.com/contiv/netplugin/systemtests/twohosts

  •   go test --timeout 40m -v -run "sanity" github.com/contiv/netplugin/systemtests/twohosts

HOST_GOBIN := `which go | xargs dirname`
HOST_GOROOT := `go env GOROOT`
endif

all: build unit-test system-test

Expand All @@ -27,10 +29,10 @@ clean-demo:
unit-test: build
CONTIV_HOST_GOBIN=$(HOST_GOBIN) CONTIV_HOST_GOROOT=$(HOST_GOROOT) ./scripts/unittests -vagrant

system-test: build
system-test:
go test -v -run "sanity" github.com/contiv/netplugin/systemtests/singlehost
go test --timeout 20m -v -run "sanity" github.com/contiv/netplugin/systemtests/twohosts

regress-test: build
regress-test:
go test -v -run "regress" github.com/contiv/netplugin/systemtests/singlehost
go test --timeout 60m -v -run "regress" github.com/contiv/netplugin/systemtests/twohosts
49 changes: 34 additions & 15 deletions Vagrantfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,60 @@
# -*- mode: ruby -*-
# vi: set ft=ruby :

netplugin_synced_gopath="/opt/golang"
host_gobin_path="/opt/go/bin"
host_goroot_path="/opt/go/root"
netplugin_synced_gopath = "/opt/golang"
host_gobin_path = ENV['CONTIV_HOST_GOBIN']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very sure, but I think this might have a side effect in environment where host's go-binary might be in a standard path like /usr/bin.

Since we try to mount the host's environment at this location it might wipe out vagrant-vm's own binary location. So I had conservatively chosen a path unique to the vagrant-vm to avoid landing into such a situation, instead of keeping it same as host's location.

host_goroot_path = ENV['CONTIV_HOST_GOROOT']

provision_common = <<SCRIPT
## setup the environment file. Export the env-vars passed as args to 'vagrant up'
echo Args passed: [[ $@ ]]
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_gobin_path}:$GOBIN' >> /etc/profile.d/envvar.sh
if [ -n "#{host_goroot_path}" ]
then
echo 'export GOROOT=/usr/local/go' >> /etc/profile.d/envvar.sh
else
echo 'export GOROOT=#{host_goroot_path}' >> /etc/profile.d/envvar.sh
fi

echo 'export PATH=/usr/local/bin:$PATH:#{host_gobin_path}:$GOBIN' >> /etc/profile.d/envvar.sh
if [ $# -gt 0 ]; then
echo "export $@" >> /etc/profile.d/envvar.sh
fi

## set the mounted host filesystems to be read-only.Just a safety check
## to prevent inadvertent modifications from vm.
(mount -o remount,ro,exec /vagrant) || exit 1
if [ -e #{host_gobin_path} ]; then
if [ ! -n "#{host_gobin_path}" ]
then
(mount -o remount,ro,exec /vagrant) || exit 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to perhaps keep this mount as read-write now, since we allow builds in the vagrant-vm.

I wonder why Jenkins didn't catch this though, may be it is not doing a clean build always. I will check Jenkins side of settings.

fi

if [ -e "#{host_gobin_path}" ]; then
(mount -o remount,ro,exec #{host_gobin_path}) || exit 1
fi
if [ -e #{host_goroot_path} ]; then
if [ -e "#{host_goroot_path}" ]; then
(mount -o remount,ro,exec #{host_goroot_path}) || exit 1
fi
if [ -e #{netplugin_synced_gopath} ]; then

if [ ! -n "#{host_gobin_path}" ] && [ -e "#{netplugin_synced_gopath}" ]
then
(mount -o remount,ro,exec #{netplugin_synced_gopath}) || exit 1
fi

### 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
#
if [ ! -n "#{host_gobin_path}" ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current vagrant box, does has go1.4 installed at this location, so the dpwnload step can be skipped. It will help the vm-bringup time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually found this to be not the case, which is why I enabled it. It was working before, however. It seems the HOST_* change messed with it, hence the guard around installing go.

I'll look deeper today, in retrospect it's probably on the image, just not in the path or something.

-Erik

On Mar 23, 2015, at 11:18 AM, Madhav Puri [email protected] wrote:

In Vagrantfile #33 (comment):

@@ -34,10 +46,15 @@ fi
#(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
-#
+if [ ! -n "#{host_gobin_path}" ]
The current vagrant box, does has go1.4 installed at this location, so the dpwnload step can be skipped. It will help the vm-bringup time.

—
Reply to this email directly or view it on GitHub https://github.com/contiv/netplugin/pull/33/files#r26963773.

then
cd /usr/local && curl -sSL https://storage.googleapis.com/golang/go1.4.linux-amd64.tar.gz | tar vxz
for i in /usr/local/go/bin/*
do
ln -sf $i /usr/local/bin/$(basename $i)
done
fi

### 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 && \
Expand Down Expand Up @@ -99,10 +116,10 @@ Vagrant.configure(VAGRANTFILE_API_VERSION) do |config|
# mount the host directories
node.vm.synced_folder ".", "/vagrant"
node.vm.synced_folder ENV['GOPATH'], netplugin_synced_gopath
if ENV['CONTIV_HOST_GOBIN'] != nil
if ENV['CONTIV_HOST_GOBIN'] != nil && ENV["CONTIV_HOST_GOBIN"] != ""
node.vm.synced_folder ENV['CONTIV_HOST_GOBIN'], host_gobin_path
end
if ENV['CONTIV_HOST_GOROOT'] != nil
if ENV['CONTIV_HOST_GOROOT'] != nil && ENV['CONTIV_HOST_GOROOT'] != ""
node.vm.synced_folder ENV['CONTIV_HOST_GOROOT'], host_goroot_path
end
node.vm.provision "shell" do |s|
Expand All @@ -125,6 +142,8 @@ provision_node = <<SCRIPT
-listen-peer-urls http://#{node_addr}:2380 \
-initial-cluster #{node_peers} \
-initial-cluster-state new 0<&- &>/tmp/etcd.log &) || exit 1
cd /opt/golang/src/github.com/contiv/netplugin
GOPATH=/opt/golang make build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to explicitly set GOPATH in make here again as it should have been set in the environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that should not be necessary. I'll fix and validate.

-Erik

On Mar 23, 2015, at 11:18 AM, Madhav Puri [email protected] wrote:

In Vagrantfile #33 (comment):

@@ -125,6 +142,8 @@ provision_node = <<SCRIPT
-listen-peer-urls http://#{node_addr}:2380
-initial-cluster #{node_peers}
-initial-cluster-state new 0<&- &>/tmp/etcd.log &) || exit 1

  • cd /opt/golang/src/github.com/contiv/netplugin
  • GOPATH=/opt/golang make build
    Do we need to explicitly set GOPATH in make here again as it should have been set in the environment?

—
Reply to this email directly or view it on GitHub https://github.com/contiv/netplugin/pull/33/files#r26963783.

SCRIPT
node.vm.provision "shell" do |s|
s.inline = provision_node
Expand Down
4 changes: 2 additions & 2 deletions systemtests/singlehost/sanity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestSingleHostSingleVlanPingSuccess_sanity(t *testing.T) {
node.RunCommand(cmdStr)
}()
if err != nil {
t.Fatalf("Failed to launch the container. Error: %s, Output: \n%s\n",
t.Fatalf("Failed to launch the container. Error: %v, Output: \n%s\n",
err, output)
}

Expand All @@ -77,7 +77,7 @@ func TestSingleHostSingleVlanPingSuccess_sanity(t *testing.T) {
output, err = node.RunCommandWithOutput(cmdStr)

if err != nil || string(output) == "" {
t.Fatalf("Failed to get ip address of the container. Error: %s, Output: \n%s\n",
t.Fatalf("Failed to get ip address of the container. Error: %v, Output: \n%s\n",
err, output)
}

Expand Down