Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(export): only call FindEnvs once #553

Merged
merged 2 commits into from
Apr 12, 2021
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
23 changes: 10 additions & 13 deletions cmd/tk/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package main

import (
"fmt"
"path/filepath"

"github.com/go-clix/cli"
"github.com/pkg/errors"

"github.com/grafana/tanka/pkg/jsonnet/jpath"
"github.com/grafana/tanka/pkg/process"
"github.com/grafana/tanka/pkg/spec/v1alpha1"
"github.com/grafana/tanka/pkg/tanka"
)

Expand Down Expand Up @@ -62,29 +60,28 @@ func exportCmd() *cli.Command {
Parallelism: *parallel,
}

var paths []string
var exportEnvs []*v1alpha1.Environment
for _, path := range args[1:] {
// find possible environments
if *recursive {
rootDir, err := jpath.FindRoot(path)
if err != nil {
return errors.Wrap(err, "resolving jpath")
}

// get absolute path to Environment
envs, err := tanka.FindEnvs(path, tanka.FindOpts{Selector: opts.Selector})
if err != nil {
return err
}

for _, env := range envs {
paths = append(paths, filepath.Join(rootDir, env.Metadata.Namespace))
if opts.Opts.Name != "" && opts.Opts.Name != env.Metadata.Name {
continue
}
exportEnvs = append(exportEnvs, env)
}
continue
}

// validate environment
if _, err := tanka.Peek(path, opts.Opts); err != nil {
env, err := tanka.Peek(path, opts.Opts)
if err != nil {
switch err.(type) {
case tanka.ErrMultipleEnvs:
fmt.Println("Please use --name to export a single environment or --recursive to export multiple environments.")
Expand All @@ -94,11 +91,11 @@ func exportCmd() *cli.Command {
}
}

paths = append(paths, path)
exportEnvs = append(exportEnvs, env)
}

// export them
return tanka.ExportEnvironments(paths, args[0], &opts)
return tanka.ExportEnvironments(exportEnvs, args[0], &opts)
}
return cmd
}
7 changes: 4 additions & 3 deletions pkg/tanka/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/apimachinery/pkg/labels"

"github.com/grafana/tanka/pkg/kubernetes/manifest"
"github.com/grafana/tanka/pkg/spec/v1alpha1"
)

// BelRune is a string of the Ascii character BEL which made computers ring in ancient times
Expand Down Expand Up @@ -43,7 +44,7 @@ type ExportEnvOpts struct {
Parallelism int
}

func ExportEnvironments(paths []string, to string, opts *ExportEnvOpts) error {
func ExportEnvironments(envs []*v1alpha1.Environment, to string, opts *ExportEnvOpts) error {
// Keep track of which file maps to which environment
fileToEnv := map[string]string{}

Expand All @@ -57,7 +58,7 @@ func ExportEnvironments(paths []string, to string, opts *ExportEnvOpts) error {
}

// get all environments for paths
envs, err := parallelLoadEnvironments(paths, parallelOpts{
loadedEnvs, err := parallelLoadEnvironments(envs, parallelOpts{
Opts: opts.Opts,
Selector: opts.Selector,
Parallelism: opts.Parallelism,
Expand All @@ -66,7 +67,7 @@ func ExportEnvironments(paths []string, to string, opts *ExportEnvOpts) error {
return err
}

for _, env := range envs {
for _, env := range loadedEnvs {
// get the manifests
loaded, err := LoadManifests(env, opts.Opts.Filters)
if err != nil {
Expand Down
43 changes: 20 additions & 23 deletions pkg/tanka/parallel.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ package tanka

import (
"fmt"
"log"
"path/filepath"

"k8s.io/apimachinery/pkg/labels"

"github.com/grafana/tanka/pkg/jsonnet/jpath"
"github.com/grafana/tanka/pkg/spec/v1alpha1"
"github.com/pkg/errors"
)

const defaultParallelism = 8
Expand All @@ -17,22 +21,9 @@ type parallelOpts struct {
}

// parallelLoadEnvironments evaluates multiple environments in parallel
func parallelLoadEnvironments(paths []string, opts parallelOpts) ([]*v1alpha1.Environment, error) {
func parallelLoadEnvironments(envs []*v1alpha1.Environment, opts parallelOpts) ([]*v1alpha1.Environment, error) {
jobsCh := make(chan parallelJob)
list := make(map[string]string)
for _, path := range paths {
envs, err := FindEnvs(path, FindOpts{Selector: opts.Selector, JsonnetOpts: opts.JsonnetOpts})
if err != nil {
return nil, err
}
for _, env := range envs {
if opts.Name != "" && opts.Name != env.Metadata.Name {
continue
}
list[env.Metadata.Name] = path
}
}
outCh := make(chan parallelOut, len(list))
outCh := make(chan parallelOut, len(envs))

if opts.Parallelism <= 0 {
opts.Parallelism = defaultParallelism
Expand All @@ -42,7 +33,7 @@ func parallelLoadEnvironments(paths []string, opts parallelOpts) ([]*v1alpha1.En
go parallelWorker(jobsCh, outCh)
}

for name, path := range list {
for _, env := range envs {
o := opts.Opts

// TODO: This is required because the map[string]string in here is not
Expand All @@ -52,32 +43,37 @@ func parallelLoadEnvironments(paths []string, opts parallelOpts) ([]*v1alpha1.En
// to Tanka workflow thus being able to handle such cases
o.JsonnetOpts = o.JsonnetOpts.Clone()

o.Name = name
o.Name = env.Metadata.Name
path := env.Metadata.Namespace
rootDir, err := jpath.FindRoot(path)
if err != nil {
return nil, errors.Wrap(err, "finding root")
}
jobsCh <- parallelJob{
path: path,
path: filepath.Join(rootDir, path),
opts: o,
}
}
close(jobsCh)

var envs []*v1alpha1.Environment
var outenvs []*v1alpha1.Environment
var errors []error
for i := 0; i < len(list); i++ {
for i := 0; i < len(envs); i++ {
out := <-outCh
if out.err != nil {
errors = append(errors, out.err)
continue
}
if opts.Selector == nil || opts.Selector.Empty() || opts.Selector.Matches(out.env.Metadata) {
envs = append(envs, out.env)
outenvs = append(outenvs, out.env)
}
}

if len(errors) != 0 {
return envs, ErrParallel{errors: errors}
return outenvs, ErrParallel{errors: errors}
}

return envs, nil
return outenvs, nil
}

type parallelJob struct {
Expand All @@ -92,6 +88,7 @@ type parallelOut struct {

func parallelWorker(jobsCh <-chan parallelJob, outCh chan parallelOut) {
for job := range jobsCh {
log.Printf("Loading %s from %s", job.opts.Name, job.path)
Copy link
Member

Choose a reason for hiding this comment

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

Supposed to be here? Sounds quite verbose imo

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it intentionally. It gives a bit of feedback to the user as opposed to no feedback right now. I'm happy to remove or accept other suggestions to give a sense of progress.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I'm not totally familiar, does this codepath affect any other actions than export?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not that I know off.

env, err := LoadEnvironment(job.path, job.opts)
if err != nil {
err = fmt.Errorf("%s:\n %w", job.path, err)
Expand Down