Skip to content

[v17.06] Cherry-pick "Adding new Task Status REMOVE"#2463

Merged
anshulpundir merged 1 commit into
moby:bump_v17.06from
dperny:bump_v17.06-cherry-pick-2461
Dec 1, 2017
Merged

[v17.06] Cherry-pick "Adding new Task Status REMOVE"#2463
anshulpundir merged 1 commit into
moby:bump_v17.06from
dperny:bump_v17.06-cherry-pick-2461

Conversation

@dperny
Copy link
Copy Markdown
Collaborator

@dperny dperny commented Nov 29, 2017

Set task desired state to REMOVE for service removal and scale down. Tasks in
REMOVE state can be deleted after they are reported SHUTDOWN. Avoids a race
conditions where network resources are freed before the running task has
shut down fully.

Add unit tests for Task state REMOVE behavior.

Started by Nishant Totla nishanttotla@gmail, finished by Drew Erny
drew.erny@docker.com.

Signed-off-by: Nishant Totla nishanttotla@gmail.com
Signed-off-by: Drew Erny drew.erny@docker.com

Backports #2461 to 17.06. Cherry pick did not apply cleanly

(cherry picked from commit b9e2c2f)
Signed-off-by: Drew Erny drew.erny@docker.com

Set task desired state to REMOVE for service removal and scale down. Tasks in
REMOVE state can be deleted after they are reported SHUTDOWN. Avoids a race
conditions where network resources are freed before the running task has
shut down fully.

Add unit tests for Task state REMOVE behavior.

Started by Nishant Totla <nishanttotla@gmail>, finished by Drew Erny
<drew.erny@docker.com>.

Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
Signed-off-by: Drew Erny <drew.erny@docker.com>

Backports moby#2461 to 17.06. Cherry pick did not apply cleanly

(cherry picked from commit b9e2c2f)
Signed-off-by: Drew Erny <drew.erny@docker.com>
@dperny dperny changed the title Adding new Task Status REMOVE [v17.06] Cherry-pick "Adding new Task Status REMOVE" Nov 29, 2017
@dperny
Copy link
Copy Markdown
Collaborator Author

dperny commented Nov 29, 2017

lol whoops title.

Comment thread api/types.pb.go
// shutting them down. Once the agent has shut down tasks with desired state
// REMOVE, the task reaper is responsible for removing them.
TaskStateRemove TaskState = 800
// TaskStateOrphaned is used to free up resources associated with service
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to pick these changes too ? They were part of a different comment.

Comment thread api/types.proto
// shutting them down. Once the agent has shut down tasks with desired state
// REMOVE, the task reaper is responsible for removing them.
REMOVE = 800 [(gogoproto.enumvalue_customname)="TaskStateRemove"];
// TaskStateOrphaned is used to free up resources associated with service
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to pick these changes too ? They were part of a different comment.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were they? I think this was just a reword of the comment for orphaned that i did

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anshulpundir those were reworded in b9e2c2f

Copy link
Copy Markdown
Contributor

@anshulpundir anshulpundir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marcusmartins
Copy link
Copy Markdown

LGTM assuming tests pass.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 30, 2017

Codecov Report

Merging #2463 into bump_v17.06 will increase coverage by 1.26%.
The diff coverage is 57.14%.

@@               Coverage Diff               @@
##           bump_v17.06    #2463      +/-   ##
===============================================
+ Coverage        59.83%   61.09%   +1.26%     
===============================================
  Files              120      120              
  Lines            25131    23825    -1306     
===============================================
- Hits             15037    14557     -480     
+ Misses            8754     7937     -817     
+ Partials          1340     1331       -9

@anshulpundir anshulpundir merged commit b187b24 into moby:bump_v17.06 Dec 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants