There were several problems with how the pooler checked out vms with
respect to empty pools, invalid pools, and aliases:
- If the vmpooler config did not contain any aliases and the caller
requested a vm from an empty pool or a non-existent one, the vmpooler
would error with:
NoMethodError - undefined method `[]' for nil:NilClass
If the config contained a non-nil alias section, then:
- If the caller requested a vm from an empty pool and either the vm
didn't have an alias or the aliased pool was empty or non-existent, then
the request for that vm would be silently ignored. The vmpooler would
return 200 if the caller asked for multiple vms and the vmpooler was
able to checkout at least one vm. Otherwise it would return 404.
- Similarly, if the caller requested a vm from a non-existent pool, then
the request was silently ignored.
This commit adds a `pool_names` Set to the config containing all valid
pool names including aliases. This is used to determine whether a
requested template name is valid or not. This is necessary because redis
does not distinguish between empty and non-existent sets, e.g. the
following returns false in both cases:
backend.exists('vmpooler__ready__' + key)
If the caller requests a vm (single or multiple), and any vm references
an invalid pool name, we immediately return 404. Otherwise, we know the
request is for valid pool names, since the vmpooler requires a restart
to change pool names and counts.
We then attempt to acquire each vm, trying to match on pool name or
failing back to aliased pool name, as was the previous behavior.
The resulting behavior is:
- If the caller asks for at least one vm from an unknown pool, then
don't try to checkout any vms and respond with 404.
- If the caller asks for a vm, and at least one pool is empty, then
respond with 503, returning checked out vms back to the pool.
- Otherwise return 200 with the list of checked out vms.
This commit also makes `alias` optional again.
This commit also re-enables tests that were merged in from master, but
originally commented out due to the bugs described above..
* Add redis to our travis runs
* De-mockistify v1/token specs; use real redis.
Open questions:
- Do we need to do better cleanup here?
- Should we be using a separate database to prevent clobbering other local db's?
* Remove mockist tests from main suite.
* (MAINT) gitignore some common files
* (maint) Clean up some of the /vm/ tests
* (maint) Convert specs for /vm/template
* (maint) Clean up, reorganize specs
* (maint) Move extracted spec helper methods to spec_helper
* (maint) rename create_vm -> create_ready_vm
* (WIP) add partially-converted /vm/hostname specs
* (maint) clean up vm_spec
* (WIP) notes for next steps
* (maint) Define :config in token tests
Miscellaneous whitespace cleanup.
* (maint) Lift #redis definition into spec helper library
* (maint) drop unneeded clear_pool helper
Given the way we're flushing redis (which seems super performant), we don't
need to clear pools any more at the beginning of tests.
* (maint) Drop clear_pool from vm/template specs
* (maint) Update vm/hostname tag and lifetime specs
* (maint) Convert vm deletion specs
* (maint) Convert specs for vm snapshot operations
* (maint) Drop now-obsolete v1_spec.rb
* (maint) cosmetic cleanup in spec helper
* (maint) begin de-mockistifying api_spec.rb
* (maint) repair incorrect test
The mockist version of the test allows redis' scard to return nil, which
it does not actually do in real life. Verified the behavior in the code
via a debugger. Fixed the test.
* (maint) finish converting dashboard specs
* (maint) rename api_spec to dashboard_spec
* (maint) Don't clobber default redis database when running specs
