Add support for securityContext and serviceAccount

[mwieczorek]

Signed-off-by: mwieczorek [email protected]

fix: #421

[codecov]

Codecov Report

Merging #456 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
+ Coverage   91.59%   91.66%   +0.06%     
==========================================
  Files          64       64              
  Lines        3142     3167      +25     
==========================================
+ Hits         2878     2903      +25     
  Misses        184      184              
  Partials       80       80

Impacted Files	Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go	100% <ø> (ø)	⬆️
pkg/util/util.go	100% <100%> (ø)	⬆️
pkg/account/main.go	100% <100%> (ø)	⬆️
pkg/deployment/all-in-one.go	100% <100%> (ø)	⬆️
pkg/deployment/collector.go	100% <100%> (ø)	⬆️
pkg/deployment/ingester.go	100% <100%> (ø)	⬆️
pkg/deployment/query.go	100% <100%> (ø)	⬆️
pkg/deployment/agent.go	100% <100%> (ø)	⬆️

---

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 1af775c...634da50. Read the comment docs.

[codecov]

Codecov Report

Merging #456 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
+ Coverage   91.59%   91.67%   +0.07%     
==========================================
  Files          64       64              
  Lines        3142     3170      +28     
==========================================
+ Hits         2878     2906      +28     
  Misses        184      184              
  Partials       80       80

Impacted Files	Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go	100% <ø> (ø)	⬆️
pkg/deployment/query.go	100% <100%> (ø)	⬆️
pkg/util/util.go	100% <100%> (ø)	⬆️
pkg/account/main.go	100% <100%> (ø)	⬆️
pkg/deployment/all-in-one.go	100% <100%> (ø)	⬆️
pkg/deployment/collector.go	100% <100%> (ø)	⬆️
pkg/deployment/ingester.go	100% <100%> (ø)	⬆️
pkg/deployment/agent.go	100% <100%> (ø)	⬆️

---

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 1af775c...be1b1e4. Read the comment docs.

[jpkrohling]

LGTM apart from a couple of nits. What's really missing is documenting this. Could you please either change the readme to document how to specify/override the sec context and per-component service account (preferred), or open a new issue for the documentation task?

[mwieczorek]

Added 2 new configuration options to readme file.
Let me know if I should provide more extended description somewhere else.

Also one question:
I see in https://github.com/jaegertracing/jaeger-operator/blob/master/pkg/deployment/agent.go#L76 that agent daemonset has no explicit serviceAccount set. Is it by design or we should also add the configuration option there?

[jpkrohling]

agent daemonset has no explicit serviceAccount set

This is not by design, I see no problems in adding the configuration option for it as well.

[jpkrohling]

Not sure util is the right place for this. We have some constants already in the main v1 package (github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1), but that might also not be suitable.

As all usages of this new constant are inside the deployment package, I'd keep it private there in a file named const.go. We can move it later if we need it somewhere else.

[mwieczorek]

I also use it in account package, so if I put it in deployment package I cannot use it in account (cyclic import).
Put it in account package? (there's JaegerServiceAccountFor func where it's used)

[jpkrohling]

Good idea, placing it close to the JaegerServiceAccountFor sounds logical to me.

[jpkrohling]

By the way, Travis fails here:

/home/travis/gopath/src/github.com/jaegertracing/jaeger-operator/pkg/util/util.go:17:2: exported const CollectorComponent should have comment (or a comment on this block) or be unexported

[mwieczorek]

This is not by design, I see no problems in adding the configuration option for it as well.

With default value default (as it is now) or jaeger (as it is for other components. Will it make a breaking change and should it be documented somewhere too - release notes etc?)

[jpkrohling]

I would vote for jaeger + release notes, as I think the potential to break existing users is very low.

[jpkrohling]

LGTM, thanks for your contribution!