Add schema migration init container [K8SSAND-625] #57
Conversation
pkg/reconcile/reconcilers.go
Outdated
| @@ -490,6 +493,13 @@ func newDeployment(reaper *api.Reaper, cassDcService string) *appsv1.Deployment | |||
| } | |||
| } | |||
|
|
|||
| initContainerEnvVar := make([]corev1.EnvVar, len(envVars)) | |||
There was a problem hiding this comment.
Shouldn't this be len(envVars) + 1 ? Instead of letting append later on to grow the size again.
adejanovski
force-pushed
the
add-schema-migration-init-container
branch
3 times, most recently
from
ca43162
to
5c02e60
Compare
jdonenine
changed the title
adejanovski
force-pushed
the
add-schema-migration-init-container
branch
4 times, most recently
from
429a30e
to
0b229bb
Compare
|
e2e tests are fixed and things work as expected. |
test/config/reaper/bases/reaper.yaml
Outdated
| @@ -3,7 +3,7 @@ kind: Reaper | |||
| metadata: | |||
| name: reaper-test | |||
| spec: | |||
| image: docker.io/thelastpickle/cassandra-reaper:2.2.5 | |||
| image: docker.io/adejanovski/cassandra-reaper:413af30 | |||
There was a problem hiding this comment.
Should probably be in k8ssandra repo.
There was a problem hiding this comment.
Because of Reaper and Medusa being used outside of k8ssandra I believe the decision was made to keep them
in the TLP org. On that basis I don't see a need to publish the image under k8ssandra.
There was a problem hiding this comment.
But that's not in the TLP org either.
There was a problem hiding this comment.
I didn't mean to commit the PR like this, as said in my comment :)
We'll have to wait until a new Docker tag is released for Reaper before merging the PR, but it would be good to get an approval ASAP so that we can get this into 1.3.
I have mixed feelings about merging the PR on a tag that's not an actual release, but we'll probably have to wait a little bit more until we get a new release of Reaper. I may just create a tag with a name that's easy enough to spot in the tlp org and update this PR so it can be merged.
There was a problem hiding this comment.
I've moved to a tag on the tlp repo now. I'll try to remember to upgrade this when Reaper 2.3.0 gets released.
There was a problem hiding this comment.
Assuming Reaper 2.3.0 will be released before k8ssandra 1.3.0 then we can open a follow up PR to bump the default version. I am in favor of merging and updating in k8ssandra to give the changes some soak time before releasing 1.3.0.
There was a problem hiding this comment.
The trick here is that I don't think we're going to get reaper-2.3.0 out before k8ssandra-1.3.0. So if we want to include this in k8ssandra-1.3.0 we'll need to use some patch version of reaper that we could get out sooner.
@adejanovski what do you think about releasing a v2.2.6 of Reaper that would pickup your work in thelastpickle/cassandra-reaper#1096 and use that?
We could hopefully then get reaper-2.3.0 out post k8ssandra-1.3.0 and use that in k8ssandra-1.4.0 to pickup the incremental repair support?
There was a problem hiding this comment.
@adejanovski what do you think about releasing a v2.2.6 of Reaper that would pickup your work in thelastpickle/cassandra-reaper#1096 and use that?
It's doable but slightly breaks the contract that patch releases are for fixes only.
Since we have no solid ETA for incremental repair improvements, I guess we could release 2.3.0 right now and delay other planned features to 2.4.0.
wdyt @jdonenine ?
There was a problem hiding this comment.
That works for me @adejanovski
I'd vote we go ahead and release 2.3.0 and we'll come back with 2.4.0 for incremental repair when it's ready.
There was a problem hiding this comment.
For the record, so you don't think I've gone crazy, I was thinking a patch release because what we're generally trying to fix is a bug: #56
But you're totally right that what we added to reaper itself, to address the bug over here in K8ssandra, isn't a bug fix. So I think you've got the right suggestion with bump to 2.4.0.
adejanovski
force-pushed
the
add-schema-migration-init-container
branch
from
0b229bb
to
8338822
Compare
adejanovski
force-pushed
the
add-schema-migration-init-container
branch
from
8338822
to
fda2d7a
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
This PR requires Reaper PR 1096 to be merged and released first.
Fixes #56
This change adds an init container that will start Reaper in schema migration only mode, allowing for it to pass without probes that could trigger a restart mid-migration.