Skip to content

Follow conventions when creating objects #67

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Follow conventions when creating objects #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba95ada
Follow conventions when creating objects
redbaron May 22, 2019
138c136
Do not create dashboards provisioning config if there are no dashboards
redbaron May 23, 2019
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
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
Copy link
Owner

@brancz brancz May 28, 2019

Choose a reason for hiding this comment

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

Last time I checked lists couldn't be nested, which is why we did what we did. Has this changed recently?

grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down Expand Up @@ -128,8 +128,8 @@ local grafana =
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down Expand Up @@ -171,8 +171,8 @@ local grafana = (
).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down Expand Up @@ -238,8 +238,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.config,
grafana.dashboardSources,
grafana.dashboardDatasources,
Expand Down Expand Up @@ -272,8 +272,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down
2 changes: 1 addition & 1 deletion examples/basic-with-mixin.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ local grafana = (
).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down
2 changes: 1 addition & 1 deletion examples/basic.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down
2 changes: 1 addition & 1 deletion examples/custom-ini.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.config,
grafana.dashboardSources,
grafana.dashboardDatasources,
Expand Down
2 changes: 1 addition & 1 deletion examples/dashboard-definition.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ local grafana =
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down
2 changes: 1 addition & 1 deletion examples/plugins.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ local grafana = ((import 'grafana/grafana.libsonnet') + {
}).grafana;

k.core.v1.list.new(
grafana.dashboardDefinitions +
[
grafana.dashboardDefinitions,
grafana.dashboardSources,
grafana.dashboardDatasources,
grafana.deployment,
Expand Down
21 changes: 12 additions & 9 deletions grafana/grafana.libsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet';
local configMapList = k.core.v1.configMapList;

{
_config+:: {
Expand Down Expand Up @@ -32,7 +33,7 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet';
},
},
},
grafanaDashboards: {},
grafanaDashboards:: {},
grafana+: {
[if std.length($._config.grafana.config) > 0 then 'config']:
local secret = k.core.v1.secret;
Expand All @@ -42,13 +43,15 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet';
secret.mixin.metadata.withNamespace($._config.namespace),
dashboardDefinitions:
local configMap = k.core.v1.configMap;
[
local dashboardName = 'grafana-dashboard-' + std.strReplace(name, '.json', '');
configMap.new(dashboardName, { [name]: std.manifestJsonEx($._config.grafana.dashboards[name], ' ') }) +
configMap.mixin.metadata.withNamespace($._config.namespace)
configMapList.new(
Copy link
Author

@redbaron redbaron May 28, 2019

Choose a reason for hiding this comment

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

see here, it used to be a [], but now a single { kind: ConfigmapList, items: [] } object, it allows embedding it

Copy link
Owner

@brancz brancz May 28, 2019

Choose a reason for hiding this comment

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

right, but when you render out a kind: List, does kubectl actually create deep lists recursively?

Copy link
Author

@redbaron redbaron May 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Doesn't seem so. See same transformation in kube-prometheus:

https://github.com/coreos/kube-prometheus/blob/3fbc968930c2fef025470c59060d25635070f8e7/jsonnet/kube-prometheus/kube-prometheus.libsonnet#L18

kubectl seems to flatten kind: List items, at least one level deep for sure. here we are exactly one level deep, so should be fine

Copy link
Owner

@brancz brancz May 28, 2019

Choose a reason for hiding this comment

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

That only works because what is generated is not a List object but a directory of objects, where one of the files happens to be a ConfigMapList, so for kubectl the ConfigMapList is actually the top level item.

Copy link
Author

@redbaron redbaron May 28, 2019

Choose a reason for hiding this comment

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

let me run few tests, I'll come back to you

Copy link
Owner

@brancz brancz May 28, 2019

Choose a reason for hiding this comment

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

correct :)

Copy link
Author

@redbaron redbaron May 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

"library" object should still have "standard" shape:

{ key: { kind: ... } }

to make it easier to use it in other jsonnet deployments.

it is only examples which need to change. I can go with

local grafana =
  ((import 'grafana/grafana.libsonnet') + .... ).grafana;

local flatten(lst) = std.foldl(function(acc, r) if ! std.endsWith(r.kind, "List") then acc + [r] else acc + r.items, lst, []);

k.core.v1.list.new(flatten([grafana[k] for k in grafana]))

what do you think?

Copy link
Owner

@brancz brancz May 28, 2019

Choose a reason for hiding this comment

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

yeah, I quite like that

Copy link
Author

@redbaron redbaron May 29, 2019

Choose a reason for hiding this comment

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

Another approach would be to return plain array and change exampes to call jsonnet --yaml-stream.

Either way output is pipeable into kubectl . Which one do you prefer?

Copy link
Owner

@brancz brancz May 29, 2019

Choose a reason for hiding this comment

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

I think I prefer the first, as that way when people want to they can still make use of the *List types for grouping, in the way I think it's good that the dashboard configmaps are currently grouped that way.

[
local dashboardName = 'grafana-dashboard-' + std.strReplace(name, '.json', '');
configMap.new(dashboardName, { [name]: std.manifestJsonEx($._config.grafana.dashboards[name], ' ') }) +
configMap.mixin.metadata.withNamespace($._config.namespace)

for name in std.objectFields($._config.grafana.dashboards)
],
for name in std.objectFields($._config.grafana.dashboards)
]
),
dashboardSources:
local configMap = k.core.v1.configMap;
local dashboardSources = import 'configs/dashboard-sources/dashboards.libsonnet';
Expand Down Expand Up @@ -111,8 +114,8 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet';
[
storageVolumeMount,
datasourcesVolumeMount,
dashboardsVolumeMount,
] +
(if std.length(self.dashboardDefinitions.items) > 0 then [dashboardsVolumeMount] else []) +
[
local dashboardName = std.strReplace(name, '.json', '');
containerVolumeMount.new('grafana-dashboard-' + dashboardName, '/grafana-dashboard-definitions/0/' + dashboardName)
Expand All @@ -124,8 +127,8 @@ local k = import 'ksonnet/ksonnet.beta.3/k.libsonnet';
[
storageVolume,
datasourcesVolume,
dashboardsVolume,
] +
(if std.length(self.dashboardDefinitions.items) > 0 then [dashboardsVolume] else []) +
[
local dashboardName = 'grafana-dashboard-' + std.strReplace(name, '.json', '');
volume.withName(dashboardName) +
Expand Down