multiple metrics support for paasta

[drmorr0]

This change introduces the ability to add multiple metrics configs to an HPA via paasta. We transform the existing AutoscalingParamsDict into a new version that looks like this:

metrics_providers:
  - type: cpu
    setpoint: 0.8
  - type: active-requests
    desired_requests_per_replica: 10
scaledown_policies: ...
max_instances_alert_threshold: ...

In this PR, we support both the old and the new ways of doing things, and we transform the old autoscaling params dict into the new version when we read the YAML files. After this PR is shipped, we will do a mass-refactor of yelpsoa into the new format and remove the "transformation" code so that only the new format is supported.

The first commit shows the bulk of the changes, and the second commit shows additional validation code and tests that are performed.

Testing done

• make test/tox -e mypy passes (with new tests added)
• paasta validate passes on all existing yelpsoa instances
• paasta validate passes on yelpsoa instances using the new format (just tested locally, there's none of these in production yet)
• setup_prometheus_adapter_config diffs in pnw-prod and nova-prod are empty
• confirmed that the before/after instance configs and the prometheus adapter configs for all of yelpsoa are identical (part of the testing in https://github.yelpcorp.com/sysgit/yelpsoa-configs/pull/43847)

Open questions

• What to do if someone's using active-requests in check_autoscaler_max_instances? (link1, link2)

cc @sclg-yelp

[nemacysts]

approach seems sensible to me - i mostly just have a couple questions and some non-blocking/ignorable suggestions/comments

[nemacysts]

i'm 75% certain that this file is dead code from the mesos days and we can ignore completely

[drmorr0]

I am also 75% certain that none of this is needed. But if I don't make these changes then it will fail mypy checks. I am 50% tempted to just submit a PR to delete this file entirely but it seems like one of those things that is maybe still called somewhere in some arcane way that's hard to suss out.

[nemacysts]

@EvanKrall what are your thoughts here?

i can see us resolving this in two ways:

1. look at the autoscaling params and use the desired_active_requests_per_replica if present
2. give up on desired_active_requests_per_replica and rename it to setpoint so that the code is neater" (at the cost of people sometimes getting confused as to what setpoint means :p)

[EvanKrall]

It is kind of odd that for active-requests we have some positive number 0-infinity, where for other providers setpoint is typically a number between 0-1 expressed as a ratio of some maximum resource utilization that's possible (100% worker util, 100% cpu, etc). We could maybe change active-requests to have two parameters: max_active_requests_per_replica (which would be some larger number than desired is currently) and setpoint. But that's probably just adding complexity where it isn't necessary.

With my branch we could have setpoint be a large number for active requests and 0-1 for other providers, and then max_instances_alert_threshold for each of those would also follow the same rules.

[nemacysts]

comment: heh, i was confused by this diff until i read the previous code and realized (as you did) that we never wrote to annotations 🤣

[nemacysts]

@Krall should we add a conditional that exits early if the metrics_provider doesn't use setpoint or should we add some special-casing here to treat the desired_active_requests field as setpoint?