Reset agent state if the instance id changed on agent restart #892
Conversation
| @@ -96,6 +98,18 @@ func newDockerTaskEngineState() *DockerTaskEngineState { | |||
| } | |||
| } | |||
|
|
|||
| // Reset resets all the states | |||
| func (state *DockerTaskEngineState) Reset() { | |||
| state.lock.RLock() | |||
There was a problem hiding this comment.
This should be Lock and Unlock instead of RLock and RUnlock
| @@ -232,6 +232,8 @@ func (agent *ecsAgent) newTaskEngine(containerChangeEventStream *eventstream.Eve | |||
| log.Warnf(instanceIDMismatchErrorFormat, | |||
| previousEC2InstanceID, currentEC2InstanceID) | |||
|
|
|||
| // Reset agent state as a new container instance | |||
| state.Reset() | |||
There was a problem hiding this comment.
It seems like you'd need to invoke statemanager.Save() here as well? Otherwise, an agent restart here may still result in the wrong state, yes?
There was a problem hiding this comment.
I don't think so, the file will be override later. And if agent restart at this point, it will run into the same code path state.Reset will be executed again.
richardpen
force-pushed
the
fix-snapshot-agent
branch
from
0130339
to
f531ef9
Compare
| @@ -96,6 +98,18 @@ func newDockerTaskEngineState() *DockerTaskEngineState { | |||
| } | |||
| } | |||
|
|
|||
| // Reset resets all the states | |||
| func (state *DockerTaskEngineState) Reset() { | |||
There was a problem hiding this comment.
Can you modify this so that you extract all the state.xxx=make() bits into a new method, and invoke it here and from newDockerTaskEngineState() so that in future if we modify this struct to persist more state, there's no cognitive load of updating Reset method as well?
richardpen
force-pushed
the
fix-snapshot-agent
branch
from
f531ef9
to
e24a2b5
Compare
richardpen
force-pushed
the
fix-snapshot-agent
branch
from
e24a2b5
to
bd577e5
Compare
Summary
Fix #859
Implementation details
Rest the agent state when detected the instance id is different from the one stored in the agent state file.
Testing
make release)go build -out amazon-ecs-agent.exe ./agent)make test) passgo test -timeout=25s ./agent/...) passmake run-integ-tests) pass.\scripts\run-integ-tests.ps1) passmake run-functional-tests) pass.\scripts\run-functional-tests.ps1) passNew tests cover the changes:
Description for the changelog
Licensing
This contribution is under the terms of the Apache 2.0 License:
yes