-
Notifications
You must be signed in to change notification settings - Fork 56
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
Conversation
Also chmod the log files we create to allow user to deleted them.
@@ -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" |
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.
Why did this become --net=host
?
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 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"]') |
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.
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 |
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.
Good to know >&
passes shfmt... TIL
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" |
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.
Will this remove NO_PROXY export in VPN as well ?
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.
No, it won't. That is a vagrant + VPN issue.
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.