Added configuration for the agent's securityContext

[chgl]

Unfortunately, I can't just use the Jaeger CommonSpec's securityContext as that is a PodSecurityContext whereas the sidecar can only have a SecurityContext. This required me to add an additional config option containerSecurityContext. This does add some redundancy as the Jaeger.Spec.SecurityContext is not reused.

Is sidecarContainerSecurityContext a better name for this config option as the other agent deployments do use the default pod security context?

Resolves #1186

[codecov]

Codecov Report

Merging #1190 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1190   +/-   ##
=======================================
  Coverage   87.32%   87.32%           
=======================================
  Files          89       89           
  Lines        4891     4892    +1     
=======================================
+ Hits         4271     4272    +1     
  Misses        457      457           
  Partials      163      163           

Impacted Files	Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go	100.00% <ø> (ø)
pkg/inject/sidecar.go	94.14% <100.00%> (+0.03%)	⬆️

---

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 01f738a...f6ee9c8. Read the comment docs.

[jpkrohling]

That's very unfortunate. Most of the fields that are commonly used are in both objects, but unless we settle for our own lowest common denominator, we can't reuse the info.

About the name: what do you think about sidecarSecurityContext?

[chgl]

@jpkrohling Oh, I like that one! Done.

Just a quick question because I'm not too failiar with the CRD generation process, running make generate creates additional changes to the jaegertracing.io_jaegers_crd.yaml beyond just the added sidecarSecurityContext:

diff --git a/deploy/crds/jaegertracing.io_jaegers_crd.yaml b/deploy/crds/jaegertracing.io_jaegers_crd.yaml
index d3c60575..580f8c90 100644
--- a/deploy/crds/jaegertracing.io_jaegers_crd.yaml
+++ b/deploy/crds/jaegertracing.io_jaegers_crd.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.2.4
+    controller-gen.kubebuilder.io/version: v0.3.0
   creationTimestamp: null
   name: jaegers.jaegertracing.io
 spec:
@@ -589,6 +589,7 @@ spec:
                         type: string
                     type: object
                   type: array
+                  x-kubernetes-list-type: atomic
                 labels:
                   additionalProperties:
                     type: string
@@ -600,11 +601,19 @@ spec:
                   properties:
                     limits:
                       additionalProperties:
-                        type: string
+                        anyOf:
+                        - type: integer
+                        - type: string
+                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                        x-kubernetes-int-or-string: true
                       type: object

Is this OK?

[jpkrohling]

No, that's not expected. You probably have a different Operator SDK version than the one that was last used with make generate. I think we are still with v0.18.2:

jaeger-operator/.github/workflows/base-checks.yaml

Line 14 in c6ba26b

operator-sdk-version: v0.18.2

Which version are you using?

[chgl]

Which version are you using?

$ operator-sdk version
operator-sdk version: "v0.18.2", commit: "f059b5e17447b0bbcef50846859519340c17ffad", kubernetes version: "v1.18.2", go version: "go1.13.10 linux/amd64"

However running make generate causes an error so I had to manually install controller-gen, openapi-gen, etc. via go install sigs.k8s.io/controller-tools/cmd/controller-gen - which probably installed a wrong version. Which version should the *-gen tools be? (or am I doing something entirely wrong?)

make generate
./.ci/generate.sh: line 28: /home/chgl/go/bin/controller-gen: No such file or directory
Failed to generate CRDs.
make: *** [Makefile:309: internal-generate] Error 127

[jpkrohling]

We have a make target (install-tools) for installing the tools we need for all other targets:

jaeger-operator/Makefile

Lines 329 to 337 in 5f7472f

	.PHONY: install-tools
	install-tools:
	@${GO_FLAGS} go install \
	golang.org/x/lint/golint \
	github.com/securego/gosec/cmd/gosec \
	golang.org/x/tools/cmd/goimports \
	sigs.k8s.io/controller-tools/cmd/controller-gen \
	k8s.io/code-generator/cmd/client-gen \
	k8s.io/kube-openapi/cmd/openapi-gen

I believe this should use the information from the go.mod. Could you check whether we have this info in the CONTRIBUTING.md? If we don't, it would also be a great contribution to the project to add this info there ;-)

[jpkrohling]

I'm blocking this from being merged for now, to sort out the controller-gen changes. The remaining of the PR is looking good.

[chgl]

Thank you, I completely missed the make install-tools or make install (although I'd have expected that one to actually install the operator as opposed to installing build tools). I can definitely update the docs on this, but maybe making install-sdk invoke install-tools makes more sense? Or simply suggest a single make install as part of the build setup.

I've been struggling quite a bit to get it working, even downgrading go from 1.15 to 1.14, but there are still quite a lot of diffs between master and what a local make generate does. To reproduce:

FROM ubuntu:20.04
RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \
    build-essential \
    git \
    curl \
    golang-1.14

RUN export GOROOT=/usr/lib/go-1.14 && \
    export PATH=$PATH:/usr/lib/go-1.14/bin/ && \
    export PATH=$PATH:/root/go/bin && \
    git clone https://github.com/jaegertracing/jaeger-operator.git && \
    cd jaeger-operator && \
    make install && \
    make generate && \
    git --no-pager diff

It always updates the kubebuilder version to v0.3.0 and mostly seems to add additional validation around resource limits/requests (which doesn't seem too bad (?)). I still don't understand why.

[jpkrohling]

which doesn't seem too bad

No, the change in itself isn't bad. Just worrying that this happens unrelated to the actual change. I can confirm that I see the same once I delete the cached binary that I had. I wonder why the CI isn't having this...

I still don't understand why.

I can't either, but I can confirm that it does work when I do a go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.4.

[chgl]

@jpkrohling I've tried again with sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.4 and it looks better now. Thank you for your help & patience!

[jpkrohling]

LGTM!