Enable annotations to be specified with the deployable components #86
Conversation
Signed-off-by: Gary Brown <[email protected]>
|
This change is |
|
Was also going to add to ingress - but the code that creates the Ingress doesn't know whether it was called from all-in-one or query - so would not know which part of the supplied JaegerSpec to look at. Think the ingress definition (spec) needs to be pulled out from the all-in-one and query and made a separate part of the spec, as it is independent on the deployment strategy used. |
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 99.34% 99.35% +<.01%
==========================================
Files 18 18
Lines 769 777 +8
==========================================
+ Hits 764 772 +8
Misses 5 5
Continue to review full report at Codecov.
|
pkg/deployment/agent_test.go
Outdated
|
|
||
| assert.Equal(t, "false", ds.Spec.Template.Annotations["sidecar.istio.io/inject"]) | ||
| assert.Equal(t, "false", dep.Spec.Template.ObjectMeta.Annotations["sidecar.istio.io/inject"]) |
There was a problem hiding this comment.
nit: ObjectMeta is embedded into the Template, you don't need to reference it explicitly.
pkg/deployment/agent_test.go
Outdated
|
|
||
| assert.Equal(t, "false", ds.Spec.Template.Annotations["sidecar.istio.io/inject"]) | ||
| assert.Equal(t, "false", dep.Spec.Template.ObjectMeta.Annotations["sidecar.istio.io/inject"]) |
There was a problem hiding this comment.
Could you also add a test where you override an annotation that we set implicitly, like prometheus.io/scrape? The code in this PR makes me believe that a user is able to set this to false, so, having a test that explicitly tests this behavior would also serve as documentation.
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jpkrohling)
Signed-off-by: Gary Brown <[email protected]>
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jpkrohling)
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
Provides a more general way to define annotations, instead of specific approach used in #81.
Signed-off-by: Gary Brown [email protected]