Add managed agent stopped and running event integ test #2702
Conversation
fierlion
force-pushed
the
integ_test_emit_message
branch
from
3292273 to
cf77aca
Compare
| assert.Equal(t, apicontainerstatus.ManagedAgentRunning, containerEvent.ManagedAgents[0].Status, | ||
| "expected managedAgent container state change event did not match actual event") | ||
| for event := range stateChangeEvents { | ||
| if containerEvent, ok := event.(api.ContainerStateChange); ok { |
There was a problem hiding this comment.
there's a possibility here that we receive an api.ContainerStateChange but not the one we were looking for. Is there a way to make sure it's a container event for managed agent?
I am thinking actually that maybe api.ContainerStateChange was meant for actual changes in the container state (i.e. docker changes), maybe we should consider to create a api.ManagedAgentStateChange? Not for this PR maybe, but something to consider
There was a problem hiding this comment.
Even with the redundant event fix added to docker_task_engine.go I was seeing events with ManagedAgentStatusNone ("NONE") prior to the expected (RUNNING|STOPPED) events, and this function sifted past these NONE events.
I agree that we might want to differentiate the container events used to chat with docker from those we use to make our updates
(not in present pr though.)
| client, err := sdkClient.NewClientWithOpts(sdkClient.WithHost(endpoint), sdkClient.WithVersion(sdkclientfactory.GetDefaultVersion().String())) | ||
| require.NoError(t, err, "Creating go docker client failed") | ||
|
|
||
| testExecCmdHostBinDir, err := filepath.Abs("../../misc/exec-command-agent-test") |
fierlion
force-pushed
the
integ_test_emit_message
branch
from
cf77aca to
d43aef8
Compare
| } | ||
| // whether we restarted or failed to restart, we'll want to emit a state change event | ||
| // redundant state change events like RUNNING->RUNNING are allowed | ||
| mTask.emitContainerEvent(mTask.Task, c, "") |
There was a problem hiding this comment.
just curious, why was this modified?
There was a problem hiding this comment.
So redundant events (RUNNING->RUNNING) aren't emitted. This change means we only emit a container event when there is a "restarted" (aka STOPPED->RUNNING) or an error (aka RUNNING->STOPPED)
Summary
Adds integration test for state change events RUNNING and STOPPED. Removes redundant container event emission of managed agent RUNNING->RUNNING.
Implementation details
Added a condition around emitContainerEvent.
Re-used a minimal section of TestExecCommandAgent to test managed agent container event emission for RUNNING and for STOPPED
Testing
ran 30x to test for flakiness locally:
sudo -E env "PATH=$PATH" go test -tags sudo -count=1 -v -run="TestManagedAgentEvent" ./agent/engine/...New tests cover the changes: yes
Description for the changelog
Add ManagedAgentEvent integration test.
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.