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

Non-deterministic crashing with well-formed but nonsensical main.jsonnet #299

Closed
amckinley opened this issue Jun 24, 2020 · 9 comments
Closed
Labels
kind/bug Something isn't working

Comments

@amckinley
Copy link

amckinley commented Jun 24, 2020

I ran into this having no idea what I'm doing with Tanka. Apparently this isn't a valid main.jsonnet file, but even if it's invalid, it's disconcerting to end up with different error messages every time. This might be a bug in the cortex-jsonnet project; if so let me know and I'll reopen it there.

tk version v0.10.0

My main.jsonnet:

local mixin = import 'cortex-mixin/mixin.libsonnet';
local cortex = import 'cortex/cortex.libsonnet';

cortex + mixin {
  _config+:: {
    namespace: 'cortex-test',
    schema: [{
      from: '2020-06-01',
      store: 'aws-dynamo',
      object_store: 's3',
      schema: 'v10',
      index: {
        prefix: 'cortex_weekly_dev_',
        period: '168h',
      },
    }],
    storage_backend: 'aws',
    table_prefix: 'cortex_weekly_dev_',
    aws_region: 'us-west-2',
    s3_bucket_name: 'robinhood-cortex-dev',
    // mixin
    storage_engine: 'chunks',
  },
}

To reproduce:

mkdir tanka-bug
cd tanka-bug
tk init
jb install github.com/grafana/cortex-jsonnet/cortex
jb install github.com/grafana/cortex-jsonnet/cortex-mixin
tk show environments/default/

Observed output:

$ tk show environments/default/
reconciling: flattening manifests: recursion did not resolve in a valid Kubernetes object.  In path `.` found key `grafanaDashboardShards` of type `float64` instead.
$ tk show environments/default/
reconciling: flattening manifests: recursion did not resolve in a valid Kubernetes object.  In path `.` found key `grafanaDashboardFolder` of type `string` instead.
$ tk show environments/default/
reconciling: flattening manifests: recursion did not resolve in a valid Kubernetes object.  In path `.grafanaDashboards.ruler.json` found key `editable` of type `bool` instead.
$ tk show environments/default/
reconciling: flattening manifests: recursion did not resolve in a valid Kubernetes object.  In path `.grafanaDashboards.cortex-scaling.json` found key `style` of type `string` instead.

Also, unrelated to the bug, if you could help me fix my main.jsonnet so I can generate both cortex and cortex-mixin manifests at the same time, that would be appreciated! :D

@issue-label-bot issue-label-bot bot added the kind/bug Something isn't working label Jun 24, 2020
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label kind/bug to this issue, with a confidence of 0.97. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@Duologic
Copy link
Member

You don't need the cortex-mixin to run cortex. The mixin is a concept that is intended to work with https://github.com/grafana/jsonnet-libs/tree/master/prometheus-ksonnet.

@jdbaldry
Copy link
Member

jdbaldry commented Jun 25, 2020

Hey!

As @Duologic said, the cortex-mixin has dashboards and prometheus rules that are intended to be added as a mixin for the prometheus-ksonnet Jsonnet library.

If you are looking just to deploy just cortex then removing the mixin should fix the problem.

To deploy cortex and the mixin, you'll need something like:

local prometheus_ksonnet = import 'prometheus-ksonnet/prometheus-ksonnet.libsonnet';
local mixin = import 'cortex-mixin/mixin.libsonnet';
local cortex = import 'cortex/cortex.libsonnet';

// Include the prometheus-ksonnet configuration. See https://github.com/grafana/jsonnet-libs/tree/master/prometheus-ksonnet for more information.
prometheus_ksonnet + cortex {

  // Top level mixins object contains all the mixins you wish to have added to your prometheus-ksonnet
  // configuration (grafana dashboards, prometheus rules, etc.).
  mixins: {
    cortex: mixin,
  },

  _config+:: {
    // cluster name is required by the prometheus-ksonnet library.
    cluster_name: 'test',

    // cortex configuration
    namespace: 'cortex-test',
    schema: [{
      from: '2020-06-01',
      store: 'aws-dynamo',
      object_store: 's3',
      schema: 'v10',
      index: {
        prefix: 'cortex_weekly_dev_',
        period: '168h',
      },
    }],
    storage_backend: 'aws',
    table_prefix: 'cortex_weekly_dev_',
    aws_region: 'us-west-2',
    s3_bucket_name: 'robinhood-cortex-dev',
    // mixin
    storage_engine: 'chunks',
  },
}

@jdbaldry
Copy link
Member

jdbaldry commented Jun 25, 2020

Just to clarify the error you are receiving, if you do a tk eval environments/default you'll see the nested JSON output by the Jsonnet evaluation isn't something you can just apply to Kubernetes. Tanka flattens the JSON and then expects all the resulting objects be valid Kubernetes objects (that have an apiVersion, kind, etc.). By merging the mixin into the top level, the evaluation and subsequent flattening results in JSON objects that are not valid Kubernetes objects and you get the error.

I'm not sure if this is a bug or not, @sh0rez would know better if the flattening is meant to be deterministic. Perhaps it would be nice since it would mirror the Jsonnet evaluation.

Hope this helps!

@sh0rez
Copy link
Member

sh0rez commented Jun 30, 2020

Hi, flattening itself is actually deterministic once completed, it only traverses the go map without order, so it may not always occur the same invalid thing first.

I agree we could clarify this error message even further, also providing a troubleshooting page in the docs.

Apart from that, this is working as intended :)

@sh0rez sh0rez closed this as completed Jun 30, 2020
@sbarzowski
Copy link

it only traverses the go map without order, so it may not always occur the same invalid thing first

Would it be hard to "artificially" make the order deterministic, i.e. sort the keys by whatever? I'm always really confused when a tool doesn't produce the same error message each time for the same input (because when I'm changing something and the message changes, I don't know if it's because of my change or just the tool randomly produces something different). Just my 2 cents.

@jdbaldry
Copy link
Member

Ah yeah, I forgot they intentionally randomized the map ordering. I agree it would be nice from a user perspective. I put together a PR that does this but I'm not sure how to write a test for it or if I'm being a little naive with the implementation.

#307

@sh0rez
Copy link
Member

sh0rez commented Jun 30, 2020

@sbarzowski I see, lets reopen this

@sh0rez sh0rez reopened this Jun 30, 2020
@sh0rez
Copy link
Member

sh0rez commented Jun 30, 2020

Closed in #307 😂

@sh0rez sh0rez closed this as completed Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants