use non-root C* images #336
Conversation
|
@emerkle826 I would appreciate it if we could sync up on this. Given all the work you have done lately with the management-api, I am hoping you can help me debug this. |
I'd be happy to help |
|
I just pushed a commit with a temporary work around to create |
|
The error: Looks like |
Apparently this has been reported in CASSANDRA-16199 (John gets the credit for that find, not me) |
|
@jsanda with the |
|
It did report the status. The error looked more related to logging configuration which honestly I think is noise since |
|
I have been testing with a non-root Medusa image (see thelastpickle/cassandra-medusa#283). In this PR I am defining a SecurityContext to run the I hit a problem with restores. After Medusa downloads the backup files it moves them to /var/lib/cassandra and then does a Option 1 - Revert the changes in the Medusa image and have it run as root Option 2 - Run the Option 3- Run |
|
Here is the error I try running the Medusa containers as the The issue is that ~/.local/bin is the default install directory for pip. When I change the SecurityContext to run Medusa as the |
jsanda
force-pushed
the
cass-image-upgrade
branch
from
3e54ede to
5516f56
Compare
…lify the helm chart rendering, remove restriction on the type of storage and remove the restriction of bucketSecret for S3
This is tempoary until datastax/cass-config-definitions#49 is merged.
The SecurityContext isn't needed since the Cassandra and Medusa images are both already configured to run as the cassandra user/group.
jsanda
force-pushed
the
cass-image-upgrade
branch
from
7322d48 to
d06b610
Compare
| {{- $medusaImage := (printf "%s:%s" .Values.medusa.image.repository .Values.medusa.image.tag) -}} | ||
| {{- $cassandrUid := 999 -}} | ||
| {{- $cassandraGid := 999 -}} | ||
| {{- $medusaUid := 999 -}} |
There was a problem hiding this comment.
This is kinda useless since medusa uses username cassandra
| @@ -1,5 +1,8 @@ | |||
| {{- $datacenter := (index .Values.cassandra.datacenters 0) -}} | |||
| {{- $medusaImage := (printf "%s:%s" .Values.backupRestore.medusa.image.repository .Values.backupRestore.medusa.image.tag) -}} | |||
| {{- $medusaImage := (printf "%s:%s" .Values.medusa.image.repository .Values.medusa.image.tag) -}} | |||
| {{- $cassandrUid := 999 -}} | |||
There was a problem hiding this comment.
Typo? Can't seem to find a match for this (missing one a)
There was a problem hiding this comment.
Yeah, and I will remove these. They are no longer used since I took out the SecurityContext.
| {{- end}} | ||
| {{- if .Values.backupRestore.medusa.enabled }} | ||
| {{- end }} | ||
| {{- if (eq .Values.cassandra.version "4.0.0") }} |
There was a problem hiding this comment.
Should this be if hasPrefix "4.0" .Values.cassandra.version (or the other way around, can't remember hasPrefix's order)
There was a problem hiding this comment.
Let me know if you decide to do that; I'll match it in my Stargate changes for 4.0 support.
There was a problem hiding this comment.
@jakerobb I updated the code to use hasPrefix.
| @@ -1,10 +1,10 @@ | |||
| {{- $bucketStorageTypes := list "s3" "gcs" -}} | |||
| {{- $bucketStorageTypes := list "s3" "google_storage" -}} | |||
There was a problem hiding this comment.
I think this is wrong now? Or is there a reason to have these two only listed here when we also support s3_compatible and local ?
There was a problem hiding this comment.
Although I'm not sure if this is used at all.. maybe this should be a map and the if clause should check for existence of key
There was a problem hiding this comment.
Good question. I wasn't sure. It is only used here now:
{{- if and .Values.medusa.multiTenant (has .Values.medusa.storage $bucketStorageTypes)}}
prefix = {{ .Values.clusterName }}.{{ .Release.Namespace }}
{{- end }}
I assume we do multi-tenant with s3_compatible and local. I will update.
There was a problem hiding this comment.
See line 39 for where it's used; it's already checking for existence in the list. Not sure what we'd gain from changing to a map.
| {{- nindent 10 "- name: CQL_USERNAME" }} | ||
| {{- nindent 12 "valueFrom:" }} | ||
| {{- nindent 14 "secretKeyRef:" }} | ||
| {{- nindent 16 (print "name: " .Values.medusa.cassandraUser.secret) }} | ||
| {{- nindent 16 "key: username" }} | ||
| {{- nindent 10 "- name: CQL_PASSWORD" }} | ||
| {{- nindent 12 "valueFrom:" }} | ||
| {{- nindent 14 "secretKeyRef:" }} | ||
| {{- nindent 16 (print "name: " .Values.medusa.cassandraUser.secret) }} | ||
| {{- nindent 16 "key: password" }} | ||
| {{- else }} | ||
| {{- nindent 10 "- name: CQL_USERNAME" -}} | ||
| {{- nindent 12 "valueFrom:" }} | ||
| {{- nindent 14 "secretKeyRef:" }} | ||
| {{- nindent 16 (print "name: " (include "k8ssandra.clusterName" . ) "-medusa") }} | ||
| {{- nindent 16 "key: username" }} | ||
| {{- nindent 10 "- name: CQL_PASSWORD" }} | ||
| {{- nindent 12 "valueFrom:" }} | ||
| {{- nindent 14 "secretKeyRef:" }} | ||
| {{- nindent 16 (print "name: " (include "k8ssandra.clusterName" . ) "-medusa") }} | ||
| {{- nindent 16 "key: password" }} | ||
| {{- end -}} |
There was a problem hiding this comment.
wouldn't this be nicer as a template?
There was a problem hiding this comment.
Not sure I follow. Can you explain?
There was a problem hiding this comment.
I guess what I'm suggesting is that you use actual spaces instead of nindent ##.
| {{- end}} | ||
| {{- if .Values.backupRestore.medusa.enabled }} | ||
| {{- end }} | ||
| {{- if (eq .Values.cassandra.version "4.0.0") }} |
There was a problem hiding this comment.
Let me know if you decide to do that; I'll match it in my Stargate changes for 4.0 support.
| @@ -1,10 +1,10 @@ | |||
| {{- $bucketStorageTypes := list "s3" "gcs" -}} | |||
| {{- $bucketStorageTypes := list "s3" "google_storage" -}} | |||
There was a problem hiding this comment.
See line 39 for where it's used; it's already checking for existence in the list. Not sure what we'd gain from changing to a map.
charts/k8ssandra/values.yaml
Outdated
| # work if `ingress.traefik.enabled` is also `true` | ||
| enabled: true | ||
|
|
||
| # -- Traefik entrypoints where traffic is sourced. |
There was a problem hiding this comment.
In #372, I added a link to the Traefik entrypoint docs here (and two other places). Several other changes in values.yaml are also missing. Did it get trampled in your merge?
There was a problem hiding this comment.
I guess so :(
I will do a diff with main and fix.
* Add ability to define s3_compatible settings as well as aws region * Change storage_properties to use map with open properties. Also, simplify the helm chart rendering, remove restriction on the type of storage and remove the restriction of bucketSecret for S3 * Fixes and a test * Fix tests * use non-root C* images * fix wget command, update groupid and userid * generate jvm11-clients.options which is needed for nodetool This is tempoary until datastax/cass-config-definitions#49 is merged. * simply the command * fix unit tests * rebase, fix security contexts, and fix google storage support * fix tests * update default management-api images * fix configmap name and introduce helper template to reduce duplication * refactor common medusa env var code into a helper template * move medusa properties up a level * fix syntax error * remove SecurityContext and update Medusa image The SecurityContext isn't needed since the Cassandra and Medusa images are both already configured to run as the cassandra user/group. * rebase, fix merge conflicts, bump chart version * remove unused code * updates from PR review * update comment on supported C* versions and fix bad merge * add 4.0 logic for jvm options and update tests Co-authored-by: Michael Burman <[email protected]> Co-authored-by: Michael Burman <[email protected]> Co-authored-by: Erik Merkle <[email protected]>
What this PR does:
Change the default images to ones that run the Cassandra pod as a non-root user.
Which issue(s) this PR fixes:
Fixes #259
Checklist