Defaults for top-level arguments don't appear to work using inline environments

[craigfurman]

Running tk show foo/bar --name NAME or tk diff foo/bar --name NAME against a main.jsonnet that contains a function with a default argument, e.g.

function(tag='latest') {
   // return a tanka.Environment
}

Returns an error "Error: Found no environments in ...". I haven't validated yet whether or not top-level argument defaults are still working in traditional, non-inline environments with a spec.json. I can look into making a PR, but wanted to raise an issue first in case it was known.

[sh0rez]

Ugh, good catch. This is related to the TLA hack put in place here:

tanka/pkg/tanka/evaluators.go

Lines 31 to 38 in f88c104

	if len(tla) != 0 {
	tlaJoin := strings.Join(tla, ", ")
	evalScript = fmt.Sprintf(`
	function(%s)
	local main = (import '%s')(%s);
	%s
	`, tlaJoin, entrypoint, tlaJoin, opts.EvalScript)
	}

Tanka uses internal Jsonnet snippets to only evaluate subsets of the Jsonnet tree when applicable, which gives massive speedup (e.g. only evaluating Environment metadata but omitting data is what makes tk env list fast)

We thought above would make TLA work, but yeah, because we create a new function which does not have these defaults, it fails.

[Duologic]

This is an interesting use case, similarly it doesn't show up in tk env list.

[craigfurman]

In my tanka travels I hadn't seen the parts of the codebase that actually evaluate jsonnet yet 😅. I can see the use case for pruning out data, for subcommands that only need metadata, and I'm guessing you'll want to preserve the performance offered by the current code here?

One way to do this that occurs to me, is to parse the jsonnet AST (but not evaluate the jsonnet) and delete the data subtrees from each environment object. This is just an avenue to explore, I have no idea if this will work, because:

1. I haven't yet researched what (go-)jsonnet-ast libraries exist, short of trying to reuse internals of go-jsonnet.
2. Even if they exist, I don't know how much speed we'll sacrifice over the current implementation.

wdyt?

[craigfurman]

It looks like it might be worth at least exploring an AST implementation: https://github.com/google/go-jsonnet/blob/7d810911490e86edee423dbf2cfb24f7d8b9dde8/vm.go#L517

Do you have any particular benchmarks you run? I have a large-ish inline environment (https://gitlab.com/gitlab-com/gl-infra/k8s-workloads/tanka-deployments/-/tree/master/environments/thanos) that anecdotally takes a good few seconds to evaluate, that I can use to gather some data. I might explore an AST impl over the next couple of weeks if I find some time.

[Duologic]

We have ~230 Tanka environments large and small, running tk env list and tk export against these show performance issues really well.

[sh0rez]

@craigfurman you're right, we would very much like to keep the performance benefits of the current implementation.

That being said, the current implementation has always been a glorified hack (Jsonnet snippets doing reflection is a real subtle thing), so yeah, I'd be very much for a more permanent solution, aka written in Go.

Given the Jsonnet our "optimizations" are heavy load Jsonnet (full tree traversal), this might turn out even faster :D

[craigfurman]

I've been poking around a bit with the go-jsonnet functions around ASTs just now, and while it's early days I'm now skeptical that an AST-pruning implementation has legs.

For example, if you have a simple tanka main.jsonnet like this:

function(foo='bar') {
  example: {
    apiVersion: 'tanka.dev/v1alpha1',
    kind: 'Environment',
    metadata: {
      name: 'example',
    },
    spec: {
      apiServer: 'https://127.0.0.1:16443',
      injectLabels: true,
      namespace: 'example',
    },
    data: {},
  },
}

It's possible to walk the AST to the function's return value, traverse down the tree until you find an object with an apiVersion and Kind field indicative of a tanka environment, and prune the data field out, before evaluating the jsonnet.

However, as soon as any library-based indirection is introduced, we have a harder problem. Take this example main.jsonnet:

local tanka = import 'tanka_helpers.libsonnet';

function(foo='bar') {
  example: tanka.environment('example'),
}

Let's assume tanka.environment returns a tanka environment object. We can't perform the AST pruning without effectively mimicking most of what jsonnet evaluation already does: delving into function calls etc. That wouldn't really be a viable solution.

I've got quite a busy week ahead so I'm not sure how much time I'll be able to spend on this, but wanted to update this issue with some more early thoughts. I suppose the next avenue to explore is fixing the tlaCode generator functions and/or the eval scripts in https://github.com/grafana/tanka/blob/master/pkg/tanka/evaluators.go to preserve tla defaults.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[zerok]

Would the following environment config represent the original issue?

local tanka = import 'github.com/grafana/jsonnet-libs/tanka-util/main.libsonnet';

function(namespace='var') {
  env: tanka.environment.new(
         name='namespace',
         namespace=namespace,
         apiserver='https://localhost:6443',
       )
       + tanka.environment.withData({
         namespace: {
           apiVersion: 'v1',
           kind: 'Namespace',
           metadata: {
             name: namespace,
           },
         },
       }),
}

This works now:

> tk show environments/dummy
apiVersion: v1
kind: Namespace
metadata:
  name: var

Or did I misunderstand the issue? 🙂