Review changes suggested to revise the Metrics related files into a more
logical class structure.
Also fixup grammar typos in docs strings and any trailing metrics that
have been recently added to vmpooler.
Use the example provided in the Ruby Client to provide a customised
collector appropriate to log all calls to the API. The customised
filtering is used to replace individual node names and templates
for the /vm and request ID's for the /ondemand endpoints.
This module was failing our rubocop checks so have updated it since
it now forms part of vmpooler.
Separate trapping for litmus jobs is also included so that they don't
interfere with stats from the jenkins pipelines.
Break down the usage stats into smaller groups so as to manage the
number of stat lines collected for Prometheus.
This may need some further revision to filter out Litmus stats, or
otherwise collect litmus usage information.
The redis pooler connection metric used "metric_prefix" which is
misleading, so split this into connpool_type and connpool_provider.
Also remove some earlier jruby compatibility code to reduce
rebase conflicts when this is rebased on top of Matt's changes.
Need the logger code in promstats.rb so move logger initialisation to
the top of the vmpooler script, remove the class method from vmpooler.rb
and make appropriate downstream changes to metrics and pooler manger
handling.
This also means we can start decent logging in the API if we wish do.
Fixed up to rebase on top of Matts POOLER-158 changes
This is a re-architect of the vmpooler initialisation code to:
1. Allow an API service for both manager and the api
2. Add the Prometheus endpoints to the web service.
Needed to change the way the Rack Service is started as instantiating
using ".New" leads to a failure to initialise the http Stats
collection.
3. Selectively load the pooler api and/or Prometheus endpoints.
4. Rework API Spec tests for revised API loading. Needed to tidy up the
initialisation and perform a reset! after each test to avoid "leaks"
and dependencies between the tests.
Add a new Prometheus class as an additional stats feed along with the
existing feeds.
Move the metrics initialisation code into its own class and sub-class
the individual metrics implementations under this.
* (POOLER-174) Reduce duplicate of on demand code introduced in POOLER-158
refactored every parsing of request of type 'pool_alias:pool:count' into a
utility class, that is used by pool_manager and the api v1 class
* add some metrics to the od request generation
* fix rubocop offenses, we are now friends
This commit updates vmpooler graphite stats behavior when a connection cannot be opened to the graphite server to warn instead of allowing the full backtrace to be output to stderr. Without this change a backtrace is output to stderr when a graphite connection fails.
It seems like generate_and_check_hostname does not need a method argument
this was fixed in one PR, and another change made in another PR and was used
inconsistently
This change adds a capability to vmpooler to provision instances on
demand. Without this change vmpooler only supports retrieving machines
from pre-provisioned pools.
Additionally, this change refactors redis interactions to reduce round
trips to redis. Specifically, multi and pipelined redis commands are
added where possible to reduce the number of times we are calling redis.
To support the redis refactor the redis interaction has changed to
leveraging a connection pool. In addition to offering multiple
connections for pool manager to use, the redis interactions in pool
manager are now thread safe.
Ready TTL is now a global parameter that can be set as a default for all
pools. A default of 0 has been removed, because this is an unreasonable
default behavior, which would leave a provisioned instance in the pool
indefinitely.
Pool empty messages have been removed when the pool size is set to 0.
Without this change, when a pool was set to a size of 0 the API and pool
manager would both show that a pool is empty.
Before this change if the smove returned false, we would continue handing out the VM
which presumably could still be in the 'ready' state. Upon 'delete' that ready VM would not be
picked up and return a 404 which is consistent with the behavior seen. Adding a metric to keep
track of the smove failures since this is not expected. I think some API logging would be good
to add in the future.
Before this PR, the current running time was being inspected to decide if the
vm lifetime could be extended. But since vm lifetime is absolute and not relative
this check is now removed.
This commit fixes the purge_unconfigured_folders feature to ensure that it can successfully identify folders and instances that are no longer used. Without this change the feature does not work as advertised.
This commit adds detection for redis connection failures to pool_manager. When a connection fails the error will be raised to executeforcing the connection to be re-established. Without this change, when a redis connection fails, it generates a redis connection error, which is swallowed by a rescue for StandardError, preventing the manager application component from recovering in the case of a redis connection failure.
This commit switches the early `return` in `migrate_vm`'s connection
pooling block to a `break`, since `return` implies you are returning
something from a method and I don't think `migrate_vm` wants to do that.
This is a place where a partial type system like Sorbet seems useful
because then we'd have some idea about what the intended return is here.
This commit drops the `has_` prefix from several `?` style methods
because that's against the ruby style guide and redundant for a method
ending in `?`.
Prior to this commit the codebase used the `zero?` method for comparing
to 0 on some places and not in others.
This commit makes all comparison to zero consistently use the `==`.
This commit fixes a bug in update_clone_target where I believe `=` was
intended, not `==` because `==` just goes to the void context here.
Thanks Rubocop Lint/Void!
This commit updates places where previously we were casting both terms
in a float division into floats in order to ensure that float division
occurs to use the `fdiv` method, which will always do float division and
is available on both `Floats` and `Integers` because they are
both `Numeric`.
This commit updates the dashboard to use `URI.parse` instead of `Kernel#open`
because open can potentially open files on the server and has other
possible security issues.
Also updated the logger to use `File.open` as it is more explicit and
doesn't have the extra potential for abuse like `Kernel#open`
https://rubocop.readthedocs.io/en/latest/cops_security/#securityopen
As far as I can tell, this variable doesn't exist before this assignment
and is immediately overwritten with the result of the method call so I
don't think it needs to be here?
Prior to this commit there were a couple locations where exceptions were
saved to `_e` but weren't used in the handler except to re-raise the
exception, which simply calling the `raise` keyword will do without a
provided argument.
This commit removes the unnecessary assignment of the exception to a
variable and simply uses `raise` instead.
