Opened 6 years ago

Closed 6 years ago

#11417 closed task (implemented)

add vagrant file for weather

Reported by: feverDream Owned by: baumanno
Priority: Medium Milestone:
Component: Metrics/Tor Weather Version:
Severity: Keywords: weather-rewrite
Cc: karsten, baumanno, meejah Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Add support for vagrant so that setting up weather for local development/testing is easier and faster :)

Child Tickets

Change History (18)

comment:1 Changed 6 years ago by feverDream

All the vagrant and puppet code has been moved to this branch [1]. Its pretty basic but its a start.

[1] https://github.com/achintangal/tor-weather-rewrite/tree/vagrant

comment:2 Changed 6 years ago by baumanno

Owner: set to baumanno
Status: newassigned

comment:3 Changed 6 years ago by feverDream

Oliver,

What vagrant tasks are you working/planning to work on?
To put things into context, I am talking about tasks we have added during our etherpad meeting [1]

[1]: https://gitweb.torproject.org/user/karsten/weather.git/blob/5385ea0f4a1cd56a2cfa37e1ce23c3d69220044e:/Vagrant.md

Last edited 6 years ago by feverDream (previous) (diff)

comment:4 Changed 6 years ago by karsten

Two things that this branch is missing:

  • Quoting feverDream: "About the background script, the current script waits for new_consensus events and exits. So making it sleep for a couple of minutes did the trick for me. Heres my modified listener.py file (1). Also, I had to add a cron-job which checks if the listener process is running, if not it spawns a new one. (this script should come in handy (2))"
  • We shouldn't add "weather.torproject.org" to /etc/hosts on the host machine, but rather something like "weather.dev".

If somebody wants to work on this, feel free to base your branch on mine, make these changes, squash everything into nice commits, and get it merged into meejah's "next" branch (or whatever it will be called).

comment:5 Changed 6 years ago by baumanno

I was thinking of making as much use of the provisioner as possible so that we can largely avoid working through that huge Vagrant.md.

I don't know how much of this can actually be handled from within puppet, but at least the software-stack and the apache virtualhost should be manageable.

I will then move on and try covering the points karsten mentioned above.

Will keep you posted here.

comment:6 Changed 6 years ago by feverDream

Yes, I believe that most of the tasks can be accomodated using a provisioner in vagrant.While you are working on the provisiong part, I will sync up the background-task that I fixed a while back.

Let me know if you have any questions.

comment:7 Changed 6 years ago by baumanno

Status: assignedneeds_review

Implementation of a vagrant box completed.
Provisioning takes place at initial boot of the VM, the only changes will have to be made to /etc/hosts on the developer's local machine to resolve 'weather.dev' to the VM's IP - the pull request contains a Vagrant.md explaining all this.

comment:8 Changed 6 years ago by karsten

Oh, nice! Here's some feedback:

  • Do you know git commit --squash and git commit --fixup? You could have made a single commit for each change and used git commit --squash 533b7688cbc004cde27c3263ff19cd997f251bfa to imply that these commits shall be squashed before merging to another branch. Not important, but can be useful at times.
  • Feel free to delete obsolete parts from Vagrant.md, rather than doing things like the "Stop here" trick. Commented-out instructions are bit-rotting even faster than everything else.
  • Is there a way to avoid touching weather/config/config.py? That is, can we have a /home/weather/opt/current/ in the VM? And can we leave base_url unchanged and still reach the web app under weather.dev or even 192.168.33.10?
  • What is weather/initial_data.json for? Can it be autogenerated by Django in the bootstrap process?
  • Same questions for weather/settings.py and weather/weather.wsgi as for weather/config/config.py: if we can, we should try not to touch these, or we'll make it more difficult to deploy weather to the production machine later.
  • You added a comment to GitHub: "I just realized I could have used PROJECT_PATH probably... we had to drop it due to the relative path, Django choked on the "../" Let's re-visit this later on down the line!" Can you elaborate? And if using PROJECT_PATH makes things easier, can you add another comment on top using git commit --squash?

Thanks!

comment:9 Changed 6 years ago by baumanno

Thanks for the feedback!

  • I used git rebase -i HEAD~XX and squashed the commits via that. Under the hood, I guess the result should be the same, but maybe git commit --squash has prettier output - will keep it in mind!
  • Vagrant.md will be merged with the README to get rid of the obsolete content
  • Regarding directory structure: we can stick to the current layout, i.e. /home/weather/opt/current, if this eases the deployment.
  • weather/initial_data.json is there to avoid user input on running manage.py syncdb from the shell provisioner. All it does is create a superuser, you might remember the django prompt Would you like to create a superuser now? [yN] - initial_data.json is simply the dump immediately after creating the superuser. Were we to drop this fixture, the provisioner would hang. The only option then is to do this by hand from within the VM. Shall we do that?
  • regarding PROJECT_PATH: this could have been used instead of a hardcoded absolute path considering the current layout. However, if we decide to replicate the production-host as much as possible, this approach might be moot.

Todo
I'll try to replicate the structure as it is on bahri as much as possible for the vagrant box. This will hopefully avoid touching any django-config and ease deployment later.

Thanks for reviewing this, and I'll keep you updated on progress!

comment:10 Changed 6 years ago by baumanno

Status: needs_reviewassigned

comment:11 Changed 6 years ago by karsten

Cc: meejah added

Reviewing 25b4a93 in https://github.com/meejah/tor-weather/tree/vagrant-baumanno:

  • weather/config/config.py still contains changes to comments. Can we take them out? (Is there even a config-vagrant.py available somewhere?)
  • baumanno and I discussed that modifying the config file for development has the drawback that somebody will someday commit those changes when running commit -a. Maybe we can avoid having to modify it for the dev environment at all?
  • weather/settings.py now contains a reference to /vagrant/weather/var/WeatherDB. Can we change that to /srv/weather.torproject.org/opt/weather/var/WeatherDB (not tested)?

I think baumanno was also planning to improve his branch based on our discussion today.

comment:12 Changed 6 years ago by meejah

See also some fixups I'd been doing to buamanno's branch here: https://github.com/meejah/tor-weather/tree/vagrant-baumanno

Using /home/weather/... in vagrant works fine.

There are two things I don't know how to handle:

  1. file-based email (vs. "real"). Karsten suggested on #tor-dev to just have file-only backend configured in Git, and an actual-deployer would have to change that.
  2. the name of the server (in config.py): weather.dev seems like a good idea for development (so you don't panic when you changed an /etc/hosts entry and try to visit the "real" weather etc) but then that's two things for the deployer to remember :)

I think I forgot to commit/push the config-vagrant.py I had.

What about this for the general case:

  1. Git is set up for dev "out of the box"; "git clone" + "vagrant up" gives you a running weather.
  2. There is a "deploy" script of some sort which does fixups like the two things above. (so "git clone" + "make deploy" or something makes it go on "the real infrastructure").

Still we don't want to have many differences between the two environments, so e.g. paths should all be the same etcetera.

comment:13 Changed 6 years ago by karsten

meejah, what you describe sounds like a fine plan to me. I assume this is not yet reflected in the branch, is it? Want to update your branch and change the ticket to needs_review again? (Thanks!)

comment:14 Changed 6 years ago by meejah

Status: assignedneeds_review

I kind of punted on the "deploy" stuff, just writing down enough to remind us what needs to be fixed in a "real" deploy...
https://github.com/meejah/tor-weather/tree/vagrant-baumanno

comment:15 Changed 6 years ago by karsten

Looks good. Last concern: the following change doesn't look correct (found in git diff 8f3f07d..):

-        'NAME': PROJECT_PATH + "/../var/WeatherDB",                      # Or p
+        'NAME': os.path.join(PROJECT_PATH, 'var', 'WeatherDB'),

After that is resolved, how about we squash all commits since 8f3f07d into a single commit and list everyone who helped out with this commit in the commit message?

comment:16 Changed 6 years ago by meejah

I agree it looks wrong, but it works (at least in vagrant). Since I don't see any changes to PROJECT_PATH with "git log -p weather/settings.py" for now I'll change it back and move where WeatherDB is for vagrant. It looks like "weather/var/WeatherDB" was committed as part of this vagrant branch anyway.

I will squash when merging to next.

comment:17 Changed 6 years ago by meejah

Oops, I kind of jumped the gun and pushed the merge.
Let me know ASAP if you see any other problems :)

comment:18 Changed 6 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Pushed to my next branch. I think that concludes this task, right? Closing. (Please re-open if we're not done, yet.) And thanks everyone who helped with this!

Note: See TracTickets for help on using tickets.