Skip to content
This repository was archived by the owner on Jun 21, 2022. It is now read-only.
Merged
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 Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions models/agent_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ func AgentAddExporter(q *reform.Querier, agentType AgentType, params *AddExporte
return nil, err
}

if _, err := AgentFindByID(q, params.PMMAgentID); err != nil {
return nil, err
}

if _, err := FindServiceByID(q, params.ServiceID); err != nil {
return nil, err
}
Expand Down
15 changes: 15 additions & 0 deletions models/service_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,21 @@ func ServicesForAgent(q *reform.Querier, agentID string) ([]*Service, error) {
return res, nil
}

// ServicesForNode returns all Services for Node with given ID.
func ServicesForNode(q *reform.Querier, nodeID string) ([]*Service, error) {
tail := fmt.Sprintf("WHERE node_id = %s ORDER BY service_id", q.Placeholder(1)) //nolint:gosec

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.

btw, @dexterHD @BupycHuk: what do you think about switching to plain placeholders? Like:

structs, err := q.SelectAllFrom(ServiceTable, "WHERE node_id = $1 ORDER BY service_id", nodeID)

We used q.Placeholder(1) because we knew we are going to switch away from MySQL to either SQLite or PostgreSQL, and they have different placeholder syntax. We switched, and very unlikely to switch again, so we can make things clearer.

structs, err := q.SelectAllFrom(ServiceTable, tail, nodeID)
if err != nil {
return nil, errors.Wrap(err, "failed to select Services")
}

res := make([]*Service, len(structs))
for i, s := range structs {
res[i] = s.(*Service)
}
return res, nil
}

func checkServiceUniqueID(q *reform.Querier, id string) error {
if id == "" {
panic("empty Service ID")
Expand Down
4 changes: 3 additions & 1 deletion services/agents/postgresql.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ func postgresqlDSN(service *models.Service, exporter *models.Agent) string {
q.Set("sslmode", "disable") // TODO: make it configurable
q.Set("connect_timeout", "5")

address := net.JoinHostPort(*service.Address, strconv.Itoa(int(*service.Port)))
host := pointer.GetString(service.Address)
port := pointer.GetUint16(service.Port)
address := net.JoinHostPort(host, strconv.Itoa(int(port)))
uri := url.URL{
Scheme: "postgres",
User: url.UserPassword(*exporter.Username, *exporter.Password),
Expand Down
11 changes: 9 additions & 2 deletions services/inventory/grpc/services_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ func NewServicesServer(s *inventory.ServicesService) inventorypb.ServicesServer
return &servicesServer{s}
}

// ListServices returns a list of all Services.
// ListServices returns a list of Services for a given filters.
func (s *servicesServer) ListServices(ctx context.Context, req *inventorypb.ListServicesRequest) (*inventorypb.ListServicesResponse, error) {
services, err := s.s.List(ctx)
filters := inventory.ServiceFilters{
NodeID: req.GetNodeId(),

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.

In general, prefer fields to methods. reg.NodeId. Methods are useful sometimes (for chains where nil is possible), but also send a signal that something is complected there, and that place is a simple one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can't use field here because in proto file we used oneof

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.

ok

}
services, err := s.s.List(ctx, filters)
if err != nil {
return nil, err
}
Expand All @@ -53,6 +56,8 @@ func (s *servicesServer) ListServices(ctx context.Context, req *inventorypb.List
res.AmazonRdsMysql = append(res.AmazonRdsMysql, service)
case *inventorypb.MongoDBService:
res.Mongodb = append(res.Mongodb, service)
case *inventorypb.PostgreSQLService:
res.Postgresql = append(res.Postgresql, service)
default:
panic(fmt.Errorf("unhandled inventory Service type %T", service))
}
Expand All @@ -75,6 +80,8 @@ func (s *servicesServer) GetService(ctx context.Context, req *inventorypb.GetSer
res.Service = &inventorypb.GetServiceResponse_AmazonRdsMysql{AmazonRdsMysql: service}
case *inventorypb.MongoDBService:
res.Service = &inventorypb.GetServiceResponse_Mongodb{Mongodb: service}
case *inventorypb.PostgreSQLService:
res.Service = &inventorypb.GetServiceResponse_Postgresql{Postgresql: service}
default:
panic(fmt.Errorf("unhandled inventory Service type %T", service))
}
Expand Down
15 changes: 13 additions & 2 deletions services/inventory/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,24 @@ func NewServicesService(db *reform.DB, r registry) *ServicesService {
}
}

// ServiceFilters represents filters for services list.
type ServiceFilters struct {
// Return only Services runs on that Node.
NodeID string
}

// List selects all Services in a stable order.
//nolint:unparam
func (ss *ServicesService) List(ctx context.Context) ([]inventorypb.Service, error) {
func (ss *ServicesService) List(ctx context.Context, filters ServiceFilters) ([]inventorypb.Service, error) {
services := make([]*models.Service, 0)
e := ss.db.InTransaction(func(tx *reform.TX) error {
var err error
services, err = models.FindAllServices(tx.Querier)
switch {
case filters.NodeID != "":
services, err = models.ServicesForNode(tx.Querier, filters.NodeID)
default:
services, err = models.FindAllServices(tx.Querier)
}
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions services/inventory/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestServices(t *testing.T) {
ss, teardown := setup(t)
defer teardown(t)

actualServices, err := ss.List(ctx)
actualServices, err := ss.List(ctx, ServiceFilters{})
require.NoError(t, err)
require.Len(t, actualServices, 0)

Expand All @@ -83,7 +83,7 @@ func TestServices(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expectedService, actualService)

actualServices, err = ss.List(ctx)
actualServices, err = ss.List(ctx, ServiceFilters{})
require.NoError(t, err)
require.Len(t, actualServices, 1)
assert.Equal(t, expectedService, actualServices[0])
Expand Down Expand Up @@ -114,7 +114,7 @@ func TestServices(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expectedMdbService, actualService)

actualServices, err = ss.List(ctx)
actualServices, err = ss.List(ctx, ServiceFilters{})
require.NoError(t, err)
require.Len(t, actualServices, 1)
assert.Equal(t, expectedMdbService, actualServices[0])
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestServices(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expectedPostgreSQLService, actualService)

actualServices, err = ss.List(ctx)
actualServices, err = ss.List(ctx, ServiceFilters{NodeID: models.PMMServerNodeID})
require.NoError(t, err)
require.Len(t, actualServices, 1)
assert.Equal(t, expectedPostgreSQLService, actualServices[0])
Expand Down
Loading