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

Ansible package for easy deployment on GCP #257

Merged
merged 8 commits into from
May 1, 2020
Merged

Conversation

ksraj123
Copy link
Contributor

@ksraj123 ksraj123 commented Apr 10, 2020

Fixes Issue #256

This ansible package simplifies the current deployment method for deploying sugarizer-server on vm instances on google compute engine.
It creates and configures a vm instance and firewall, installs dependencies and deploys sugarizer-server onto the vm instance, it also prints the external IP of the vm instance where the deployment can be accessed.

This could be tested with steps described in ansible/docs/ansible-deployToGcp.md of this PR.

Copy link
Collaborator

@NikhilM98 NikhilM98 left a comment

Choose a reason for hiding this comment

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

Few remarks:

  • The gce module is deprecated.
    As of Ansible 2.8, Ansible community is encouraging everyone to move from the gce module to the gcp_compute_instance module. The gcp_compute_instance module has better support for all of GCP’s features, fewer dependencies, more flexibility, and better supports GCP’s authentication systems.
    Reference: 1 2 (Ansible Docs)

  • There was no need to create Install Dependencies playbook. They can be installed in ansible/deploy-to-gcp/gcp-start-script.sh itself.

  • What is touch testing.js command for in ansible/deploy-to-gcp/gcp-start-script.sh?

  • Add an empty line as the end of each file.

  • The ansible/deploy-to-gcp has no direct dependence on the Sugarizer Server code, since it pulls a fresh copy of Sugarizer-Server from the GitHub. Also, the get_start_script uses the code from GitHub even though the Start Script was already present in the system. Maybe instead of merging the Deploy to GCP in Sugarizer-Server, we can create another repository for the same (Like Sugarizer-APK-Builder and Sugarizer-School-Box). What do you think @llaske ?

@ksraj123
Copy link
Contributor Author

ksraj123 commented Apr 11, 2020

@NikhilM98 Thanks for reviewing. I have made the suggested changes. Deprecated module gce has been replaced with gcp_compute_instance

Yes, ansible/deploy-to-gcp does not have any direct dependence upon Sugarizer Server Code that is why only this specific directory is downloaded to the user's local machine by steps mentioned in ansible/docs as we do not want the user to unnecessarily download the entire repository.

get_start_script uses code from github as this script is being executed in our vm instance on GCP not the user's local machine where the code might already be present.

Ansible packages for other deployments methods too can be created in future and they can all be inside the ansible folder, I think it will promote gradual transition to using ansible as the preferred way to setup sugarizer-server specially by non-developers. However separating it into another repository is also a good option. What do you think?

Requesting suggestions @llaske

@NikhilM98
Copy link
Collaborator

NikhilM98 commented Apr 11, 2020

get_start_script uses code from github as this script is being executed in our vm instance on GCP not the user's local machine where the code might already be present.

Have you tested providing startup-script directly instead of startup-script-url?
Reference: link

If you can provide startup-script directly then you can pass the script directly. It will remove the dependence on the code hosted on GitHub. It will allow the users to make small changes in the startup script like change source repository/branch. In current implementation, there is no possible way to make changes in the startup script without hosting it on the internet.

@ksraj123
Copy link
Contributor Author

@NikhilM98 Thanks for suggestion. startup-script was not used initially as providing path to script file as described in the reference link did not work, maybe because I could not seem to find gcp_compute_instance counterpart of --metadata-from-file flag of gcloud. We could have installed google cloud sdk on the local machine of the user and used the gcloud command as in the reference link but its relatively heavy weight or provide the content directly but it would clutter our playbook. However another work around is used in which the script is defined in a yaml file and variables from that file is included in our playbook. This approach also ensures that user does not execute the startup script which is meant to be executed on the vm instance on his local machine.

@NikhilM98
Copy link
Collaborator

NikhilM98 commented Apr 12, 2020

It looks better now. The reference link was added to point out that we can use startup-script as a metadata key instead of startup-script-url to provide the startup instructions. (I was using my phone while writing that comment, so couldn't provide better references at that point)

Btw the file 4.png is uploaded but not used.

@ksraj123
Copy link
Contributor Author

@NikhilM98 the unused image 4.png has been removed.

@llaske
Copy link
Owner

llaske commented Apr 13, 2020

@mikklfr could you review this and see if it could be helpful for Sugarizer School Portal project.

@llaske llaske requested a review from mikklfr April 16, 2020 09:36
@mikklfr
Copy link
Collaborator

mikklfr commented Apr 22, 2020

Hi!
This is nice, this Ansible recipe will allow to easily create and fill up an instance.

That's not linked with the school project technically as this approach is not about sharing a pool of resources for many schools but about setting up a single deploy.

But that is nice and could simplify deployments for some.

@llaske
Copy link
Owner

llaske commented Apr 27, 2020

Okay.

@ksraj123 could you:

  • Reduce images size, currently 1920x880, should be near 800x400
  • Could you put all documents into docs directory instead of the ansible/docs directory
  • Could you reference - or even include - your documentation into existing GCP documentation. May be you could describe it as an alternative to manual install
  • Use relative path llaske/sugarizer-server/dev instead of ksraj123/sugarizer-server/patch-1

@ksraj123
Copy link
Contributor Author

ksraj123 commented Apr 29, 2020

@llaske Suggested changes made, please review.

  • Some changes to steps made so that the it is independent of owner of repo and branch
  • images cropped and resized to desired to be of desired resolution
  • documentation for new deployment method moved to /docs
  • New deployment method reference in old gcp deployment documentation, it is not included there as I think it will make the old documentation confusing

@llaske llaske merged commit dba8b96 into llaske:dev May 1, 2020
@llaske
Copy link
Owner

llaske commented May 1, 2020

Nice. Thanks.

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.

4 participants