pravega / pravega-operator

Pravega Kubernetes Operator
Apache License 2.0
41 stars 38 forks source link

Issue 483: Auth handler implementation using init containers #556

Closed anishakj closed 3 years ago

anishakj commented 3 years ago

Signed-off-by: anishakj anisha.kj@dell.com

Change log description

If Authhandler is speecified, init container is created and it will copy the binary and mount to an emptydir

Purpose of the change

Fixes #484

What the code does

(Detailed description of the code changes)

How to verify it

Verified that able to copy jar files to the controller pod

codecov-commenter commented 3 years ago

Codecov Report

Merging #556 (9aa87c9) into master (bb52746) will increase coverage by 0.35%. The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #556      +/-   ##
==========================================
+ Coverage   71.64%   71.99%   +0.35%     
==========================================
  Files          15       15              
  Lines        3629     3682      +53     
==========================================
+ Hits         2600     2651      +51     
- Misses        910      911       +1     
- Partials      119      120       +1     
Impacted Files Coverage Δ
pkg/apis/pravega/v1beta1/pravega.go 98.11% <ø> (ø)
pkg/apis/pravega/v1beta1/zz_generated.deepcopy.go 98.63% <90.90%> (-0.63%) :arrow_down:
pkg/controller/pravega/pravega_controller.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 bb52746...9aa87c9. Read the comment docs.

maddisondavid commented 3 years ago

I'm wondering if this could be achieved with just the standard generic init-container support and some documentation. The problem is that there could be other artifacts required such as a Bookkeeper Rack Aware placement script which would also need to be copied.

My suggestion would be that this use case is handled by the user with an init container image. The Operator would mount an EmptyDir at a well known documented path, i.e. /mnt/pravega into which the init container could copy any required artifacts. Documentation could provide a sample Dockerfile and bash script.

sarlaccpit commented 3 years ago

@maddisondavid , @anishakj

Ok this is a very encouraging PR. Let me pipe in a couple details; some of which were discussed offline, so it'd be best to share here.

Generic init containers (for controller, segment store pods) that literally are straight up init containers out of the K8 spec. VS. First class specialized pieces of the PravegaCluster spec that are known to the Operator it knows exactly what to do with them(maybe by doing init container stuff underneath, maybe not)

In the first case, the operator knows nothing at all and just appends the initcontainer to the deployment/statefulsets. In the second, the operator knows exactly what it's dealing with, it knows which pod it actually is relevant to(ctrl vs ss) and where to put what where.

Both have value and honestly my opinion is why not have both? Known use cases that Pravega supports in terms of customization, why not first class them? AND also have generic init containers for whatever we haven't thought about yet.

Why have specialized support? Because, look at these 2 use cases:

That sounds like a a bit too much to ask the user to know/learn, even with documentation. (maybe).

By first classing the two above, you ask the user for:

In this PR, @anishakj is going for some generic initcontainer support and a first class support for auth plugins.