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

Add a make dev to output a development manifest #131

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

liztio
Copy link
Contributor

@liztio liztio commented Nov 6, 2017

Refactor 20-pod.jsonnet to take a few parameters

Closes #107

Signed-off-by: liz liz@heptio.com

};

local myconf = conf(opts);
local sonobuoyPod =
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the open syntax kosher? dangling =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this really confusing at first too. I think it's basically a "let", the pod is defined in the scope of defining sonobuoy.

Makefile Outdated

$(DEV_OUTPUT): examples/ksonnet/dev.jsonnet examples/ksonnet/components/*.jsonnet
$(KUBECFG_CMD)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably remove these extra lines

if opts.debug then " --debug" else ""
]),
],
image: "gcr.io/heptio-images/sonobuoy:master",
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if the image:tag was a var at the top

"/bin/bash",
"-c",
std.join("", [
"/sonobuoy master --no-exit=true -v 3 --logtostderr",
Copy link
Contributor

Choose a reason for hiding this comment

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

-v 5 on debug

pod.mixin.spec.volumes(conf.volumes);
pod.mixin.spec.volumes(myconf.volumes);

local sonobuoyService =
Copy link
Contributor

Choose a reason for hiding this comment

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

same.

local configmaps = import "examples/ksonnet/components/10-configmaps.jsonnet";
local pod = import "examples/ksonnet/components/20-pod.jsonnet";

rbac + configmaps + pod.objects(pullPolicy="IfNotPresent", debug=true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we pass a var in from Make targets vs. default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it this way because I don't like the idea of storing configuration in our Makefile.

@liztio liztio force-pushed the liz-make-dev branch 2 times, most recently from 24ea0ac to 1863332 Compare November 7, 2017 16:35
Copy link
Contributor

@timothysc timothysc left a comment

Choose a reason for hiding this comment

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

/lgtm - Did you test a local submission with the dev.yaml and verify the results?

Refactor 20-pod.jsonnet to take a few parameters

Signed-off-by: liz <liz@heptio.com>
@liztio liztio merged commit ddcfda6 into vmware-tanzu:master Nov 7, 2017
@liztio liztio deleted the liz-make-dev branch November 7, 2017 17:15
jhamilton1 pushed a commit to jhamilton1/sonobuoy that referenced this pull request Jun 27, 2018
Refactor 20-pod.jsonnet to take a few parameters

Signed-off-by: liz <liz@heptio.com>
Signed-off-by: Jesse Hamilton jesse.hamilton@heptio.com
jhamilton1 pushed a commit to jhamilton1/sonobuoy that referenced this pull request Jun 27, 2018
Refactor 20-pod.jsonnet to take a few parameters

Signed-off-by: liz <liz@heptio.com>
Signed-off-by: Jesse Hamilton jesse.hamilton@heptio.com

Signed-off-by: Jesse Hamilton jesse.hamilton@heptio.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants