Skip to content

Commit

Permalink
refactor(task): tasks will now use the flux language service (influxd…
Browse files Browse the repository at this point in the history
…ata#17104)

The tasks subsystem will now use the flux language service to parse and
evaluate flux instead of directly interacting with the parser or
runtime. This helps break the dependency on the libflux parser for the
base influxdb package.

This includes the task notification packages which were changed at the
same time.
  • Loading branch information
jsternberg authored Mar 5, 2020
1 parent a907e05 commit bcbb9df
Show file tree
Hide file tree
Showing 35 changed files with 425 additions and 107 deletions.
4 changes: 2 additions & 2 deletions check.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ const (

// Check represents the information required to generate a periodic check task.
type Check interface {
Valid() error
Valid(lang FluxLanguageService) error
Type() string
ClearPrivateData()
SetTaskID(ID)
GetTaskID() ID
GetOwnerID() ID
SetOwnerID(ID)
GenerateFlux() (string, error)
GenerateFlux(lang FluxLanguageService) (string, error)
json.Marshaler

CRUDLogSetter
Expand Down
3 changes: 2 additions & 1 deletion cmd/influxd/launcher/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@ func (m *Launcher) run(ctx context.Context) (err error) {
}

serviceConfig := kv.ServiceConfig{
SessionLength: time.Duration(m.sessionLength) * time.Minute,
SessionLength: time.Duration(m.sessionLength) * time.Minute,
FluxLanguageService: fluxlang.DefaultService,
}

flushers := flushers{}
Expand Down
12 changes: 8 additions & 4 deletions http/check_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type CheckBackend struct {
LabelService influxdb.LabelService
UserService influxdb.UserService
OrganizationService influxdb.OrganizationService
FluxLanguageService influxdb.FluxLanguageService
}

// NewCheckBackend returns a new instance of CheckBackend.
Expand All @@ -41,6 +42,7 @@ func NewCheckBackend(log *zap.Logger, b *APIBackend) *CheckBackend {
LabelService: b.LabelService,
UserService: b.UserService,
OrganizationService: b.OrganizationService,
FluxLanguageService: b.FluxLanguageService,
}
}

Expand All @@ -56,6 +58,7 @@ type CheckHandler struct {
LabelService influxdb.LabelService
UserService influxdb.UserService
OrganizationService influxdb.OrganizationService
FluxLanguageService influxdb.FluxLanguageService
}

const (
Expand Down Expand Up @@ -83,6 +86,7 @@ func NewCheckHandler(log *zap.Logger, b *CheckBackend) *CheckHandler {
UserService: b.UserService,
TaskService: b.TaskService,
OrganizationService: b.OrganizationService,
FluxLanguageService: b.FluxLanguageService,
}
h.HandlerFunc("POST", prefixChecks, h.handlePostCheck)
h.HandlerFunc("GET", prefixChecks, h.handleGetChecks)
Expand Down Expand Up @@ -294,7 +298,7 @@ func (h *CheckHandler) handleGetCheckQuery(w http.ResponseWriter, r *http.Reques
h.HandleHTTPError(ctx, err, w)
return
}
flux, err := chk.GenerateFlux()
flux, err := chk.GenerateFlux(h.FluxLanguageService)
if err != nil {
h.HandleHTTPError(ctx, err, w)
return
Expand Down Expand Up @@ -429,7 +433,7 @@ func decodePostCheckRequest(r *http.Request) (postCheckRequest, error) {
}, nil
}

func decodePutCheckRequest(ctx context.Context, r *http.Request) (influxdb.CheckCreate, error) {
func decodePutCheckRequest(ctx context.Context, lang influxdb.FluxLanguageService, r *http.Request) (influxdb.CheckCreate, error) {
params := httprouter.ParamsFromContext(ctx)
id := params.ByName("id")
if id == "" {
Expand Down Expand Up @@ -467,7 +471,7 @@ func decodePutCheckRequest(ctx context.Context, r *http.Request) (influxdb.Check
}
chk.SetID(*i)

if err := chk.Valid(); err != nil {
if err := chk.Valid(lang); err != nil {
return influxdb.CheckCreate{}, err
}

Expand Down Expand Up @@ -596,7 +600,7 @@ func (h *CheckHandler) mapNewCheckLabels(ctx context.Context, chk influxdb.Check
// handlePutCheck is the HTTP handler for the PUT /api/v2/checks route.
func (h *CheckHandler) handlePutCheck(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
chk, err := decodePutCheckRequest(ctx, r)
chk, err := decodePutCheckRequest(ctx, h.FluxLanguageService, r)
if err != nil {
h.log.Debug("Failed to decode request", zap.Error(err))
h.HandleHTTPError(ctx, err, w)
Expand Down
2 changes: 2 additions & 0 deletions http/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/influxdata/influxdb/notification"
"github.com/influxdata/influxdb/notification/check"
"github.com/influxdata/influxdb/pkg/testttp"
"github.com/influxdata/influxdb/query/fluxlang"
influxTesting "github.com/influxdata/influxdb/testing"
"go.uber.org/zap/zaptest"
)
Expand All @@ -34,6 +35,7 @@ func NewMockCheckBackend(t *testing.T) *CheckBackend {
LabelService: mock.NewLabelService(),
UserService: mock.NewUserService(),
OrganizationService: mock.NewOrganizationService(),
FluxLanguageService: fluxlang.DefaultService,
}
}

Expand Down
2 changes: 1 addition & 1 deletion http/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (r QueryRequest) Analyze(l influxdb.FluxLanguageService) (*QueryAnalysis, e

func (r QueryRequest) analyzeFluxQuery(l influxdb.FluxLanguageService) (*QueryAnalysis, error) {
a := &QueryAnalysis{}
pkg, err := l.Parse(r.Query)
pkg, err := query.Parse(l, r.Query)
if pkg == nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion http/query_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (h *FluxHandler) postFluxAST(w http.ResponseWriter, r *http.Request) {
return
}

pkg, err := h.LanguageService.Parse(request.Query)
pkg, err := query.Parse(h.LanguageService, request.Query)
if err != nil {
h.HandleHTTPError(ctx, &influxdb.Error{
Code: influxdb.EInvalid,
Expand Down
12 changes: 6 additions & 6 deletions kv/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func (s *Service) createCheck(ctx context.Context, tx Tx, c influxdb.CheckCreate
c.SetCreatedAt(now)
c.SetUpdatedAt(now)

if err := c.Valid(); err != nil {
if err := c.Valid(s.FluxLanguageService); err != nil {
return err
}

Expand All @@ -291,7 +291,7 @@ func (s *Service) createCheck(ctx context.Context, tx Tx, c influxdb.CheckCreate
}

func (s *Service) createCheckTask(ctx context.Context, tx Tx, c influxdb.CheckCreate) (*influxdb.Task, error) {
script, err := c.GenerateFlux()
script, err := c.GenerateFlux(s.FluxLanguageService)
if err != nil {
return nil, err
}
Expand All @@ -314,7 +314,7 @@ func (s *Service) createCheckTask(ctx context.Context, tx Tx, c influxdb.CheckCr

// PutCheck will put a check without setting an ID.
func (s *Service) PutCheck(ctx context.Context, c influxdb.Check) error {
if err := c.Valid(); err != nil {
if err := c.Valid(s.FluxLanguageService); err != nil {
return err
}
return s.kv.Update(ctx, func(tx Tx) error {
Expand Down Expand Up @@ -393,7 +393,7 @@ func (s *Service) updateCheck(ctx context.Context, tx Tx, id influxdb.ID, chk in
}

chk.SetTaskID(current.GetTaskID())
flux, err := chk.GenerateFlux()
flux, err := chk.GenerateFlux(s.FluxLanguageService)
if err != nil {
return nil, err
}
Expand All @@ -418,7 +418,7 @@ func (s *Service) updateCheck(ctx context.Context, tx Tx, id influxdb.ID, chk in
chk.SetCreatedAt(current.GetCRUDLog().CreatedAt)
chk.SetUpdatedAt(s.Now())

if err := chk.Valid(); err != nil {
if err := chk.Valid(s.FluxLanguageService); err != nil {
return nil, err
}

Expand Down Expand Up @@ -459,7 +459,7 @@ func (s *Service) patchCheck(ctx context.Context, tx Tx, id influxdb.ID, upd inf
tu.Status = strPtr(string(*upd.Status))
}

if err := c.Valid(); err != nil {
if err := c.Valid(s.FluxLanguageService); err != nil {
return nil, err
}

Expand Down
5 changes: 4 additions & 1 deletion kv/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/influxdata/influxdb"
"github.com/influxdata/influxdb/kv"
"github.com/influxdata/influxdb/query/fluxlang"
influxdbtesting "github.com/influxdata/influxdb/testing"
"go.uber.org/zap/zaptest"
)
Expand All @@ -28,7 +29,9 @@ func initBoltCheckService(f influxdbtesting.CheckFields, t *testing.T) (influxdb
}

func initCheckService(s kv.Store, f influxdbtesting.CheckFields, t *testing.T) (influxdb.CheckService, string, func()) {
svc := kv.NewService(zaptest.NewLogger(t), s)
svc := kv.NewService(zaptest.NewLogger(t), s, kv.ServiceConfig{
FluxLanguageService: fluxlang.DefaultService,
})
svc.IDGenerator = f.IDGenerator
svc.TimeGenerator = f.TimeGenerator
if f.TimeGenerator == nil {
Expand Down
5 changes: 4 additions & 1 deletion kv/notification_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/influxdata/influxdb"
"github.com/influxdata/influxdb/kv"
"github.com/influxdata/influxdb/query/fluxlang"
influxdbtesting "github.com/influxdata/influxdb/testing"
"go.uber.org/zap/zaptest"
)
Expand All @@ -28,7 +29,9 @@ func initBoltNotificationRuleStore(f influxdbtesting.NotificationRuleFields, t *
}

func initNotificationRuleStore(s kv.Store, f influxdbtesting.NotificationRuleFields, t *testing.T) (influxdb.NotificationRuleStore, func()) {
svc := kv.NewService(zaptest.NewLogger(t), s)
svc := kv.NewService(zaptest.NewLogger(t), s, kv.ServiceConfig{
FluxLanguageService: fluxlang.DefaultService,
})
svc.IDGenerator = f.IDGenerator
svc.TimeGenerator = f.TimeGenerator
if f.TimeGenerator == nil {
Expand Down
11 changes: 9 additions & 2 deletions kv/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ type Service struct {

IDGenerator influxdb.IDGenerator

// FluxLanguageService is used for parsing flux.
// If this is unset, operations that require parsing flux
// will fail.
FluxLanguageService influxdb.FluxLanguageService

// special ID generator that never returns bytes with backslash,
// comma, or space. Used to support very specific encoding of org &
// bucket into the old measurement in storage.
Expand Down Expand Up @@ -73,14 +78,16 @@ func NewService(log *zap.Logger, kv Store, configs ...ServiceConfig) *Service {
if s.clock == nil {
s.clock = clock.New()
}
s.FluxLanguageService = s.Config.FluxLanguageService

return s
}

// ServiceConfig allows us to configure Services
type ServiceConfig struct {
SessionLength time.Duration
Clock clock.Clock
SessionLength time.Duration
Clock clock.Clock
FluxLanguageService influxdb.FluxLanguageService
}

// Initialize creates Buckets needed.
Expand Down
6 changes: 3 additions & 3 deletions kv/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ func (s *Service) createTask(ctx context.Context, tx Tx, tc influxdb.TaskCreate)
// return nil, influxdb.ErrInvalidOwnerID
// }

opt, err := options.FromScript(tc.Flux)
opt, err := options.FromScript(s.FluxLanguageService, tc.Flux)
if err != nil {
return nil, influxdb.ErrTaskOptionParse(err)
}
Expand Down Expand Up @@ -714,12 +714,12 @@ func (s *Service) updateTask(ctx context.Context, tx Tx, id influxdb.ID, upd inf

// update the flux script
if !upd.Options.IsZero() || upd.Flux != nil {
if err = upd.UpdateFlux(task.Flux); err != nil {
if err = upd.UpdateFlux(s.FluxLanguageService, task.Flux); err != nil {
return nil, err
}
task.Flux = *upd.Flux

options, err := options.FromScript(*upd.Flux)
options, err := options.FromScript(s.FluxLanguageService, *upd.Flux)
if err != nil {
return nil, influxdb.ErrTaskOptionParse(err)
}
Expand Down
14 changes: 11 additions & 3 deletions kv/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
icontext "github.com/influxdata/influxdb/context"
"github.com/influxdata/influxdb/kv"
_ "github.com/influxdata/influxdb/query/builtin"
"github.com/influxdata/influxdb/query/fluxlang"
"github.com/influxdata/influxdb/task/backend"
"github.com/influxdata/influxdb/task/servicetest"
"go.uber.org/zap/zaptest"
Expand All @@ -27,7 +28,9 @@ func TestBoltTaskService(t *testing.T) {
t.Fatal(err)
}

service := kv.NewService(zaptest.NewLogger(t), store)
service := kv.NewService(zaptest.NewLogger(t), store, kv.ServiceConfig{
FluxLanguageService: fluxlang.DefaultService,
})
ctx, cancelFunc := context.WithCancel(context.Background())
if err := service.Initialize(ctx); err != nil {
t.Fatalf("error initializing urm service: %v", err)
Expand Down Expand Up @@ -78,7 +81,10 @@ func newService(t *testing.T, ctx context.Context, c clock.Clock) *testService {
t.Fatal("failed to create InmemStore", err)
}

ts.Service = kv.NewService(zaptest.NewLogger(t), ts.Store, kv.ServiceConfig{Clock: c})
ts.Service = kv.NewService(zaptest.NewLogger(t), ts.Store, kv.ServiceConfig{
Clock: c,
FluxLanguageService: fluxlang.DefaultService,
})
err = ts.Service.Initialize(ctx)
if err != nil {
t.Fatal("Service.Initialize", err)
Expand Down Expand Up @@ -246,7 +252,9 @@ func TestTaskRunCancellation(t *testing.T) {
}
defer close()

service := kv.NewService(zaptest.NewLogger(t), store)
service := kv.NewService(zaptest.NewLogger(t), store, kv.ServiceConfig{
FluxLanguageService: fluxlang.DefaultService,
})
ctx, cancelFunc := context.WithCancel(context.Background())
if err := service.Initialize(ctx); err != nil {
t.Fatalf("error initializing urm service: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion notification/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type Base struct {
}

// Valid returns err if the check is invalid.
func (b Base) Valid() error {
func (b Base) Valid(lang influxdb.FluxLanguageService) error {
if !b.ID.Valid() {
return &influxdb.Error{
Code: influxdb.EInvalid,
Expand Down
3 changes: 2 additions & 1 deletion notification/check/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/influxdata/flux/parser"
"github.com/influxdata/influxdb/notification"
"github.com/influxdata/influxdb/query/fluxlang"

"github.com/influxdata/influxdb/mock"

Expand Down Expand Up @@ -156,7 +157,7 @@ func TestValidCheck(t *testing.T) {
},
}
for _, c := range cases {
got := c.src.Valid()
got := c.src.Valid(fluxlang.DefaultService)
influxTesting.ErrorsEqual(t, got, c.err)
}
}
Expand Down
Loading

0 comments on commit bcbb9df

Please sign in to comment.