Add CLI command to generate k8s manifests #1046

chlunde · 2020-04-29T10:13:23Z

Defaults to stdin/stdout

Examples:

jaeger-operator generate \
    --jaeger-all-in-one-image localimage \
    --cr ./deploy/examples/all-in-one-with-options.yaml \
    | kubectl apply -f -

cat jaeger.yaml | podman run jaeger-operator generate > manifest.yaml

Fixes #375

chlunde · 2020-04-29T10:13:59Z

POC for discussion, primarily because I wanted when looking at the code :)

pkg/strategy/strategy.go

codecov · 2020-04-29T10:47:32Z

Codecov Report

Merging #1046 into master will increase coverage by 0.19%.
The diff coverage is 94.59%.

@@            Coverage Diff             @@
##           master    #1046      +/-   ##
==========================================
+ Coverage   64.58%   64.78%   +0.19%     
==========================================
  Files          84       84              
  Lines        6715     6758      +43     
==========================================
+ Hits         4337     4378      +41     
- Misses       2236     2237       +1     
- Partials      142      143       +1

Impacted Files	Coverage Δ
pkg/strategy/strategy.go	`95.53% <93.93%> (-0.67%)`	⬇️
pkg/account/main.go	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 465ee77...62ebfae. Read the comment docs.

pavolloffay · 2020-05-04T15:57:29Z

Could you please document this feature in readme how to use it as a docker command?

pkg/strategy/strategy.go

pavolloffay · 2020-05-04T16:02:40Z

pkg/cmd/generate/main.go

+
+	// TODO: Did I get all the relevant options? Too many?
+
+	cmd.Flags().String("jaeger-agent-image", "jaegertracing/jaeger-agent", "The Docker image for the Jaeger Agent")


This is no-go to have double copy of all the flags. It should be unified with start/main.go

Now shared, if you have an idea for a better name/package, please say where :)

pkg/cmd/generate/main.go

pavolloffay · 2020-05-04T16:08:25Z

Could you add a e2e test that would generate all-in-one deployment from a generated file?

pkg/account/main.go

pkg/cmd/generate/main.go

Signed-off-by: Carl Henrik Lunde <[email protected]>

Defaults to stdin/stdout Examples: jaeger-operator generate --jaeger-all-in-one-image localimage --cr ./deploy/examples/all-in-one-with-options.yaml | kubectl apply -f - cat jaeger.yaml | podman run jaeger-operator generate > manifest.yaml Fixes jaegertracing#375 Signed-off-by: Carl Henrik Lunde <[email protected]>

Signed-off-by: Carl Henrik Lunde <[email protected]>

pavolloffay

LGTM, great work!

@kevinearls could you please have a look at e2e tests?

README.md

pavolloffay · 2020-05-07T14:00:22Z

README.md

+
+## Command line invocation
+
+To dry-run, experiment or run adapt the generated output with kustomize, command line invocation using the `generate`subcommand is possible. In this example we apply the manifest generated by examples/simple-prod.yaml to the namespace `jaeger-test`:


It might not be just dry-run some people still prefer to use plain manifest files. This paragraph should tell that jaeger-operator generate generates kubernetes manifest file from a given CR.

Updated, but I'm not sure if you wanted the introduction sentence at all. Maybe you want to spell our the whole paragraph instead? :)

If anything, maybe we should have an example with kustomize or hint that people should use kustomize instead of manually patching the generated file. This would allow them to run generate with a newer version of the operator to get upgrades later, and hopefully not update/redo the local changes.

pkg/cmd/generate/main.go

kevinearls

I've requested 2 minor test changes. I am also running this PR on our internal CI that uses OpenShift and will update this review once that finishes.

test/e2e/generator_test.go

Signed-off-by: Carl Henrik Lunde <[email protected]>

kevinearls

LGTM

pavolloffay

LGTM, just a couple of nits

README.md

pkg/cmd/start/main.go

pkg/cmd/generate/main.go

Signed-off-by: Carl Henrik Lunde <[email protected]>

pavolloffay · 2020-05-12T14:09:39Z

thanks @chlunde 🚀 !

chlunde commented Apr 29, 2020

View reviewed changes

pkg/strategy/strategy.go Outdated Show resolved Hide resolved

pavolloffay reviewed May 4, 2020

View reviewed changes

chlunde force-pushed the generate-cmd branch 3 times, most recently from 88fb60a to b697b77 Compare May 6, 2020 22:07

pavolloffay reviewed May 7, 2020

View reviewed changes

pkg/account/main.go Show resolved Hide resolved

pkg/cmd/generate/main.go Outdated Show resolved Hide resolved

chlunde added 7 commits May 7, 2020 13:46

Add missing TypeMeta for corev1.ServiceAccount

dd2c1fe

Signed-off-by: Carl Henrik Lunde <[email protected]>

Add e2e tests for jaeger-operator generate

0aaefe3

Signed-off-by: Carl Henrik Lunde <[email protected]>

Avoid command line flag duplication

47c9033

Signed-off-by: Carl Henrik Lunde <[email protected]>

Fixup - allow EOF on stdin

3b478b2

Signed-off-by: Carl Henrik Lunde <[email protected]>

README: Explain generate subcommand

d8dc644

Signed-off-by: Carl Henrik Lunde <[email protected]>

Generate command description: update

7865002

Signed-off-by: Carl Henrik Lunde <[email protected]>

chlunde force-pushed the generate-cmd branch from b697b77 to 7865002 Compare May 7, 2020 11:51

chlunde added 4 commits May 7, 2020 15:44

Strategy: Test that All includes all types

2c5b670

Signed-off-by: Carl Henrik Lunde <[email protected]>

Remove solved TODOs

05c90c7

Signed-off-by: Carl Henrik Lunde <[email protected]>

Use AllInOneSmokeTest for e2e CLI generate

ee429f7

Signed-off-by: Carl Henrik Lunde <[email protected]>

Fix tag name in README

dec1b41

Signed-off-by: Carl Henrik Lunde <[email protected]>

chlunde changed the title ~~WIP Add CLI command jaeger-operator generate to generate manifests~~ Add CLI command jaeger-operator generate to generate manifests May 7, 2020

pavolloffay reviewed May 7, 2020

View reviewed changes

kevinearls suggested changes May 11, 2020

View reviewed changes

test/e2e/generator_test.go Show resolved Hide resolved

test/e2e/generator_test.go Show resolved Hide resolved

chlunde added 2 commits May 11, 2020 20:38

Update docs

f1e3ff1

Signed-off-by: Carl Henrik Lunde <[email protected]>

Use require.No* for better error message in e2e test

c5b56bb

Signed-off-by: Carl Henrik Lunde <[email protected]>

kevinearls approved these changes May 12, 2020

View reviewed changes

pavolloffay reviewed May 12, 2020

View reviewed changes

Update docs, mark as experimental

083461a

Signed-off-by: Carl Henrik Lunde <[email protected]>

Minor refactor/cleanup

62ebfae

Signed-off-by: Carl Henrik Lunde <[email protected]>

pavolloffay approved these changes May 12, 2020

View reviewed changes

pavolloffay merged commit f3f1f23 into jaegertracing:master May 12, 2020

pavolloffay changed the title ~~Add CLI command jaeger-operator generate to generate manifests~~ Add CLI command to generate k8s manifests May 12, 2020

jpkrohling mentioned this pull request Jul 10, 2020

Command line flags are not being handled properly #1128

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CLI command to generate k8s manifests #1046

Add CLI command to generate k8s manifests #1046

chlunde commented Apr 29, 2020 •

edited

Loading

chlunde commented Apr 29, 2020

codecov bot commented Apr 29, 2020 •

edited

Loading

pavolloffay commented May 4, 2020

pavolloffay May 4, 2020

chlunde May 7, 2020

pavolloffay commented May 4, 2020

pavolloffay left a comment

pavolloffay May 7, 2020

chlunde May 11, 2020

kevinearls left a comment

kevinearls left a comment

pavolloffay left a comment

pavolloffay commented May 12, 2020


		// TODO: Did I get all the relevant options? Too many?

		cmd.Flags().String("jaeger-agent-image", "jaegertracing/jaeger-agent", "The Docker image for the Jaeger Agent")


		## Command line invocation

		To dry-run, experiment or run adapt the generated output with kustomize, command line invocation using the `generate`subcommand is possible. In this example we apply the manifest generated by examples/simple-prod.yaml to the namespace `jaeger-test`:

Add CLI command to generate k8s manifests #1046

Add CLI command to generate k8s manifests #1046

Conversation

chlunde commented Apr 29, 2020 • edited Loading

chlunde commented Apr 29, 2020

codecov bot commented Apr 29, 2020 • edited Loading

Codecov Report

pavolloffay commented May 4, 2020

pavolloffay May 4, 2020

Choose a reason for hiding this comment

chlunde May 7, 2020

Choose a reason for hiding this comment

pavolloffay commented May 4, 2020

pavolloffay left a comment

Choose a reason for hiding this comment

pavolloffay May 7, 2020

Choose a reason for hiding this comment

chlunde May 11, 2020

Choose a reason for hiding this comment

kevinearls left a comment

Choose a reason for hiding this comment

kevinearls left a comment

Choose a reason for hiding this comment

pavolloffay left a comment

Choose a reason for hiding this comment

pavolloffay commented May 12, 2020

chlunde commented Apr 29, 2020 •

edited

Loading

codecov bot commented Apr 29, 2020 •

edited

Loading