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.

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

Revert to shell script for build version #86

Merged
merged 1 commit into from
Mar 30, 2017

Conversation

neelimamukiri
Copy link
Contributor

@neelimamukiri neelimamukiri commented Mar 29, 2017

Also chmod the log files we create to allow user to deleted them.

Fixes https://github.com/contiv/ccn/issues/258 and removes the python requests dependency for getting build version (fixes #85). Allows user to override BUILD_VERSION as well.

Also chmod the log files we create to allow user to deleted them.
@gaurav-dalvi gaurav-dalvi self-requested a review March 29, 2017 22:17
@@ -142,4 +143,4 @@ ansible_mount="-v $(pwd)/ansible:/ansible:Z"
config_mount="-v $src_conf_path:$container_conf_path:Z"
cache_mount="-v $(pwd)/contiv_cache:/var/contiv_cache:Z"
mounts="$install_mount $ansible_mount $cache_mount $config_mount"
docker run --rm $mounts $image_name sh -c "./install/ansible/uninstall.sh $netmaster_param -a \"$ans_opts\" $uninstall_scheduler -m $contiv_network_mode -d $fwd_mode $aci_param $reset_params $cluster_param"
docker run --rm --net=host $mounts $image_name sh -c "./install/ansible/uninstall.sh $netmaster_param -a \"$ans_opts\" $uninstall_scheduler -m $contiv_network_mode -d $fwd_mode $aci_param $reset_params $cluster_param"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this become --net=host?

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 made this change to the installer, but missed it in the uninstaller. Hit the issue on Linux while testing the access privileges for uninstall log file and added this.

exit 1
fi
fi
release=$(echo "$releases" | python -c 'import json, sys;print json.load(sys.stdin)[0]["name"]')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

@@ -92,16 +92,16 @@ env_file=install/ansible/env.json
# Verify ansible can reach all hosts

echo "Verifying ansible reachability"
ansible all $ans_opts -i $host_inventory -m setup -a 'filter=ansible_distribution*' >& $inventory_log
egrep 'FAIL|UNREACHABLE' $inventory_log >& /dev/null
ansible all $ans_opts -i $host_inventory -m setup -a 'filter=ansible_distribution*' >&$inventory_log
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know >& passes shfmt... TIL

@dseevr
Copy link
Contributor

dseevr commented Mar 29, 2017

Just a few comments, LGTM otherwise

@@ -142,4 +143,4 @@ ansible_mount="-v $(pwd)/ansible:/ansible:Z"
config_mount="-v $src_conf_path:$container_conf_path:Z"
cache_mount="-v $(pwd)/contiv_cache:/var/contiv_cache:Z"
mounts="$install_mount $ansible_mount $cache_mount $config_mount"
docker run --rm $mounts $image_name sh -c "./install/ansible/uninstall.sh $netmaster_param -a \"$ans_opts\" $uninstall_scheduler -m $contiv_network_mode -d $fwd_mode $aci_param $reset_params $cluster_param"
docker run --rm --net=host $mounts $image_name sh -c "./install/ansible/uninstall.sh $netmaster_param -a \"$ans_opts\" $uninstall_scheduler -m $contiv_network_mode -d $fwd_mode $aci_param $reset_params $cluster_param"
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this remove NO_PROXY export in VPN as well ?

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, it won't. That is a vagrant + VPN issue.

@neelimamukiri neelimamukiri merged commit 9f89939 into contiv:master Mar 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make demo-swarm is asking for additional python modules.
3 participants