remind101 / empire

A PaaS built on top of Amazon EC2 Container Service (ECS)
BSD 2-Clause "Simplified" License
2.69k stars 158 forks source link

emp rollback rolls back scaling #1108

Open phobologic opened 7 years ago

phobologic commented 7 years ago

Saw this earlier today in staging/prod. Something recently has caused empire to reset scale for an app when rolling back, which I'm pretty sure isn't the behavior we want. This happened at first with a 0.11 emp client, so I upgraded and saw the same behavior with 0.13.

# emp_stage scale -a acme-inc web=4
A commit message is required, enter one below:
> testing scaling rollback
Scaled acme-inc to web=4:1X.
# emp_stage scale -l -a acme-inc
web=4:1X worker=0:1X
# emp_stage set -a acme-inc "TESTING_SCALE_ROLLBACK=herewego"
A commit message is required, enter one below:
> testing rollback in scaling
Set env vars and restarted acme-inc.
# emp_stage scale -l -a acme-inc
web=4:1X worker=0:1X
# emp_stage releases -a acme-inc
v1    May 24 11:00  Deploy phobologic/acme-inc:latest (phobologic: 'testing deploy on destryed app')
v2    May 24 12:47  Set WEB_STRING config var (phobologic: 'testing setting vars')
v3    Sep 21 13:03  Set TESTING_SCALE_ROLLBACK config var (phobologic: 'testing rollback in scaling')
# emp_stage scale -a acme-inc web=2
A commit message is required, enter one below:
> another test
Scaled acme-inc to web=2:1X.
# emp_stage scale -l -a acme-inc
web=2:1X worker=0:1X
# emp_stage rollback -a acme-inc v2
A commit message is required, enter one below:
> testing scale rollback
Rolled back acme-inc to v2 as v4.
# emp_stage scale -l -a acme-inc
web=4:1X worker=0:1X
ejholmes commented 7 years ago

I think the confusion here comes from the fact that scaling is the only change that can ever mutate a release. So if you scale up, it mutates the current release, instead of creating a new release. However, rollbacks literally just take an old release, and copies it as the new release. I agree, the behavior here is a little strange, but it's the intended (originally designed) behavior, so I'm gonna change the tag on this from bug to enhancement.

A better behavior would probably be to remove the mutability of scaling, and make scaling events create a new release, so it's easier to tell what you're rolling back to, and easier to see a history of scaling (and why with the -m flag).