DEVPROD-13572 Update agent to end after running single task host task #8789
Conversation
operations/agent.go
Outdated
| @@ -185,6 +186,15 @@ func Agent() cli.Command { | |||
| ctx, cancel := context.WithCancel(context.Background()) | |||
| defer cancel() | |||
|
|
|||
| h, err := host.FindOneId(ctx, opts.HostID) | |||
There was a problem hiding this comment.
Can we do this here? I'm not familiar with the agent command, but I don't think it has a database connection to get a host- so I think this would stop agents from starting
There was a problem hiding this comment.
ah yeah, this is my bad
made a new agent method for this
| defer cancel() | ||
|
|
||
| // The loop should exit after one execution because it is on a single host distro | ||
| s.NoError(s.a.loop(ctx)) |
There was a problem hiding this comment.
Can we test if it only looped once somehow?
There was a problem hiding this comment.
I don't think so
there isnt a super good test for the agents working in general 😂 because most of them checks that we break under certain conditions
There was a problem hiding this comment.
Yeah I thought so. I had some ideas of having it run a command and the command modifies a test variable but we don't have anything like that setup and it's non-trivial to do. I'm honestly surprised we haven't had a test command like this yet. This tests looks good to me then!
| @@ -59,6 +59,10 @@ type SharedCommunicator interface { | |||
| GetDistroView(context.Context, TaskData) (*apimodels.DistroView, error) | |||
| // GetDistroAMI gets the AMI for the given distro/region | |||
| GetDistroAMI(context.Context, string, string, TaskData) (string, error) | |||
| // GetDistroByName returns a distro given a distro ID. | |||
| GetDistroByName(context.Context, string) (*restmodel.APIDistro, error) | |||
There was a problem hiding this comment.
Does this function get it by name or id (the comment above says id but the name says name)
EDIT: I see that the method was already implemented and it gets it by id rather than name like the name suggests. Could you rename it to say GetDistroByDistroID maybe? Or GetDistroByID (if it makes sense that the ID in question is the Distro's)?
| @@ -223,6 +225,13 @@ func (a *Agent) Start(ctx context.Context) error { | |||
| a.tryCleanupDirectory(a.opts.WorkingDirectory) | |||
| } | |||
|
|
|||
| distro, err := a.comm.GetDistroByHostID(ctx, a.opts.HostID) | |||
DEVPROD-13572
Description
agent exits after looping once if the task is a single task distro task
it does not currently exit if the task is a single host task but does not fully handle the single host task group case which will be covered later
Testing
added a unit test for agent ending the loop
will do a full end to end test with some logging next ticket