Add ECS Service scale up/down controls #2197
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These controls do nothing for now, this was just to get the control buttons working
ekimekim
force-pushed
the
mike/ecs/scale-controls
branch
from
63288ed to
5a47717
Compare
|
rebased onto master now that #2186 is merged. |
|
Tests failed due to #2220, retrying for now since this obviously isn't happening in most test runs. |
2opremio
reviewed
Feb 9, 2017
probe/awsecs/reporter.go
Outdated
| ServiceDesiredCount: fmt.Sprintf("%d", service.DesiredCount), | ||
| ServiceRunningCount: fmt.Sprintf("%d", service.RunningCount), | ||
| report.ControlProbeID: r.probeID, | ||
| }).WithLatestActiveControls(ScaleUp, ScaleDown)) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Not AFAIK. But to be consistent with the rest (e.g. stop button for
containers) I think making it disappear is the right thing
…On Feb 9, 2017 11:10 PM, "Mike Lang" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In probe/awsecs/reporter.go
<#2197>:
> @@ -126,10 +149,11 @@ func (r Reporter) Tag(rpt report.Report) (report.Report, error) {
for serviceName, service := range ecsInfo.Services {
serviceID := report.MakeECSServiceNodeID(cluster, serviceName)
rpt.ECSService = rpt.ECSService.AddNode(report.MakeNodeWith(serviceID, map[string]string{
- Cluster: cluster,
- ServiceDesiredCount: fmt.Sprintf("%d", service.DesiredCount),
- ServiceRunningCount: fmt.Sprintf("%d", service.RunningCount),
- }))
+ Cluster: cluster,
+ ServiceDesiredCount: fmt.Sprintf("%d", service.DesiredCount),
+ ServiceRunningCount: fmt.Sprintf("%d", service.RunningCount),
+ report.ControlProbeID: r.probeID,
+ }).WithLatestActiveControls(ScaleUp, ScaleDown))
Ack. Is there a way to have the minus button there but greyed out? I know
I can set it "dead" but that makes it disappear, not "disabled", iiuc?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#2197>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACQOJLZBkvjvB2NrkynGGChOq_ozO3cNks5ra47igaJpZM4L4y4E>
.
|
|
@ekimekim ping |
ekimekim
added a commit
that referenced
this pull request
Feb 16, 2017
as currently this would make it disappear (#2085). See also #2197 (comment)
|
PTAL |
2opremio
reviewed
Feb 17, 2017
| ScaleUp: {Dead: false}, | ||
| // We've decided for now to disable ScaleDown when only 1 task is desired, | ||
| // since scaling down to 0 would cause the service to disappear (#2085) | ||
| ScaleDown: {Dead: service.DesiredCount <= 1}, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
as currently this would make it disappear (#2085). See also #2197 (comment)
ekimekim
force-pushed
the
mike/ecs/scale-controls
branch
from
9278b73 to
a49f1c9
Compare
|
ptal |
2opremio
approved these changes
Feb 21, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds Scale Up / Scale Down controls for ECS services in the same manner as Kubernetes Replica Sets/Deployments.
Note this requires an additional permission "ecs:UpdateService". Documentation will need to be changed to include this.
However I don't know where that particular documentation is kept (perhaps an argument for keeping the documentation in-repo,
so it can be modified in conjunction with code changes instead of as an afterthought).
Partially addresses #2051