-
Notifications
You must be signed in to change notification settings - Fork 55
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
Update env.json for v2plugin option (-p) #145
Conversation
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.
PTAL. I think we need to clean up the installer story a bit. Legacy swarm installation vs. native swarm mode is going to be confusing.
Should we take out the docker+swarm step outside of our installer? That is more in line with how it works with k8s. @neelimamukiri ?
README.md
Outdated
@@ -18,6 +18,7 @@ Note: The full image contains only Contiv components. Installing Docker Swarm wi | |||
* Change directories to the extracted folder <br>`cd contiv-$VERSION` | |||
* To install Contiv with Docker Swarm:<br> `./install/ansible/install_swarm.sh -f cfg.yml -e <ssh key> -u <username> -i` |
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.
Can you also update these two as well to say this docker swarm install is only for legacy swarm mode, and is incompatible with native swarm and v2 plugin?
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 update
README.md
Outdated
@@ -18,6 +18,7 @@ Note: The full image contains only Contiv components. Installing Docker Swarm wi | |||
* Change directories to the extracted folder <br>`cd contiv-$VERSION` | |||
* To install Contiv with Docker Swarm:<br> `./install/ansible/install_swarm.sh -f cfg.yml -e <ssh key> -u <username> -i` | |||
* To install Contiv with Docker Swarm and ACI:<br> `./install/ansible/install_swarm.sh -f aci_cfg.yml -e <ssh key> -u <username> -i -m aci` | |||
* To install Contiv v2plugin:<br> `./install/ansible/install_swarm.sh -f aci_cfg.yml -e <ssh key> -u <username> -p` |
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.
See above. Please explicitly note docker swarm-mode here.
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.
We are only installing v2plugin. We are not installing swarm-mode here
@@ -131,6 +131,9 @@ cp /var/contiv/key.pem /ansible/roles/auth_proxy/files/ | |||
if [ "$aci_image" != "" ]; then | |||
sed -i.bak "s#.*aci_gw_image.*#\"aci_gw_image\":\"$aci_image\",#g" "$env_file" | |||
fi | |||
if [ "$contiv_v2plugin_install" == "true" ]; then |
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.
Should it not be exclusive with -i option? AFAIK, there isn't a point in installer covering the docker swarm install and v2plugin install option (even though they can be used together).
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.
We shouldn't exclude it. v2plugin with legacy swarm can be installed. We will not officially support it
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.
@rhim, Made -p and -i as mutually exclusive.
@@ -140,6 +140,9 @@ sed -i.bak "s#__CLUSTER_STORE__#$cluster#g" $env_file | |||
if [ "$aci_image" != "" ]; then | |||
sed -i.bak "s#.*aci_gw_image.*#\"aci_gw_image\":\"$aci_image\",#g" "$env_file" | |||
fi | |||
if [ "$contiv_v2plugin_install" == "true" ]; then |
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.
How do we set the docker version to ensure that v2plugin is only used with 17.03 or something such? Is it possible for ppl to try v2plugin with an old version of docker?
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.
if docker version is lesser than 17.03, docker plugin install command is not supported and install fails.
LGTM. Go ahead and merge. Please open a pr for doc changes. I think the v2plugin Neelima to be documented better on contiv/install and/or contiv.github.io. |
@@ -131,6 +136,9 @@ cp /var/contiv/key.pem /ansible/roles/auth_proxy/files/ | |||
if [ "$aci_image" != "" ]; then | |||
sed -i.bak "s#.*aci_gw_image.*#\"aci_gw_image\":\"$aci_image\",#g" "$env_file" | |||
fi | |||
if [ "$contiv_v2plugin_install" == "true" ]; then | |||
sed -i.bak "s#.*contiv_v2plugin_install.*#\"contiv_v2plugin_install\":\"True\",#g" "$env_file" | |||
fi |
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.
Opened #149 to handle resetting this for the non-v2 plugin case.
Rest of it LGTM.
fixes #141