Skip to content
This repository was archived by the owner on Jun 21, 2022. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions handlers/inventory_agents.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ type AgentsServer struct {
Agents *inventory.AgentsService
}

// ListAgents returns a list of all Agents.
// ListAgents returns a list of Agents for a given filters.
func (s *AgentsServer) ListAgents(ctx context.Context, req *api.ListAgentsRequest) (*api.ListAgentsResponse, error) {
agents, err := s.Agents.List(ctx)
agents, err := s.Agents.List(ctx, req.ServiceId)
if err != nil {
return nil, err
}
Expand Down
38 changes: 38 additions & 0 deletions models/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"github.com/go-sql-driver/mysql"
"github.com/pkg/errors"
"gopkg.in/reform.v1"
)

Expand Down Expand Up @@ -211,3 +212,40 @@ func (q *QanAgent) DSN(service *MySQLService) string {
// TODO Other parameters?
return cfg.FormatDSN()
}

// AgentFilters represents filters for agents list.
type AgentFilters struct {
ServiceID *string
}

// AgentsByFilters returns agents providing insights for a given filters.
func AgentsByFilters(q *reform.Querier, filters AgentFilters) ([]*AgentRow, error) {
var agentIDs []interface{}
var structs []reform.Struct
var err error
if filters.ServiceID != nil {
agentServices, err := q.SelectAllFrom(AgentServiceView, "WHERE service_id = ? ORDER BY ID", *filters.ServiceID)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no ID column.

That code branch is not covered by tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but that code can't work: there is no ID column in agent_services table. The only conclusion is that tests are not run or not testing that branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if err != nil {
return nil, errors.WithStack(err)
}
for _, str := range agentServices {
agentIDs = append(agentIDs, str.(*AgentService).AgentID)
}

if len(agentIDs) == 0 {
return []*AgentRow{}, nil
}

structs, err = q.FindAllFrom(AgentRowTable, "id", agentIDs...)

Choose a reason for hiding this comment

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

ineffectual assignment to err (from ineffassign)

Copy link
Contributor

Choose a reason for hiding this comment

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

The real problem there: err is shadowed.

} else {
structs, err = q.SelectAllFrom(AgentRowTable, "ORDER BY ID")
}
if err != nil {
return nil, errors.WithStack(err)
}
agents := make([]*AgentRow, len(structs))
for i, str := range structs {
agents[i] = str.(*AgentRow)
}
return agents, nil
}
17 changes: 10 additions & 7 deletions services/inventory/agents.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,20 @@ func (as *AgentsService) get(ctx context.Context, id string) (*models.AgentRow,
}
}

// List selects all Agents in a stable order.
func (as *AgentsService) List(ctx context.Context) ([]inventory.Agent, error) {
structs, err := as.q.SelectAllFrom(models.AgentRowTable, "ORDER BY id")
// List selects all Agents in a stable order for a given service.
func (as *AgentsService) List(ctx context.Context, serviceID string) ([]inventory.Agent, error) {
filters := models.AgentFilters{}
if serviceID != "" {
filters.ServiceID = &serviceID
}
agentRows, err := models.AgentsByFilters(as.q, filters)
if err != nil {
return nil, errors.WithStack(err)
}

// TODO That loop makes len(structs) SELECTs, that can be slow. Optimize when needed.
res := make([]inventory.Agent, len(structs))
for i, str := range structs {
row := str.(*models.AgentRow)
// TODO That loop makes len(agentRows) SELECTs, that can be slow. Optimize when needed.
res := make([]inventory.Agent, len(agentRows))
for i, row := range agentRows {
agent, err := as.makeAgent(ctx, row)
if err != nil {
return nil, err
Expand Down
11 changes: 8 additions & 3 deletions services/inventory/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ func TestAgents(t *testing.T) {
ss, as, teardown := setup(t)
defer teardown(t)

actualAgents, err := as.List(ctx)
actualAgents, err := as.List(ctx, "")
require.NoError(t, err)
require.Len(t, actualAgents, 0)

Expand Down Expand Up @@ -407,12 +407,17 @@ func TestAgents(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expectedMySQLdExporterAgent, actualAgent)

actualAgents, err = as.List(ctx)
actualAgents, err = as.List(ctx, "")
require.NoError(t, err)
require.Len(t, actualAgents, 2)
assert.Equal(t, expectedNodeExporterAgent, actualAgents[0])
assert.Equal(t, expectedMySQLdExporterAgent, actualAgents[1])

actualAgents, err = as.List(ctx, "gen:00000000-0000-4000-8000-000000000002")
require.NoError(t, err)
require.Len(t, actualAgents, 1)
assert.Equal(t, expectedMySQLdExporterAgent, actualAgents[0])

err = as.Remove(ctx, "gen:00000000-0000-4000-8000-000000000001")
require.NoError(t, err)
actualAgent, err = as.Get(ctx, "gen:00000000-0000-4000-8000-000000000001")
Expand All @@ -425,7 +430,7 @@ func TestAgents(t *testing.T) {
tests.AssertGRPCError(t, status.New(codes.NotFound, `Agent with ID "gen:00000000-0000-4000-8000-000000000003" not found.`), err)
assert.Nil(t, actualAgent)

actualAgents, err = as.List(ctx)
actualAgents, err = as.List(ctx, "")
require.NoError(t, err)
require.Len(t, actualAgents, 0)
})
Expand Down