docs: Add instructions for cancelling an in-progress rollout #34226
Conversation
jubrad
left a comment
jubrad
left a comment
There was a problem hiding this comment.
I'm still confused about this being in installation/_index.md rather than a specific upgrade overview file, but that doesn't need to be addressed in this PR.
| ### Cancelling the Upgrade | ||
| To cancel an in-progress rollout and revert to the last completed rollout state, first retrieve the last rollout request ID from your Materialize CR. You can do this with the following command: | ||
|
|
||
| ```shell | ||
| kubectl get materialize <instance-name> -n materialize-environment -o jsonpath='{.status.lastCompletedRolloutRequest}' | ||
| ``` | ||
|
|
||
| Next, get the previous `environmentdImageRef` from the `last-applied-configuration` annotation: | ||
| ```shell | ||
| kubectl get materialize <instance-name> -n materialize-environment -o jsonpath='{.metadata.annotations.kubectl\.kubernetes\.io/last-applied-configuration}' | ||
| ``` | ||
| Finally, run the following command using the values obtained above: | ||
|
|
||
| ```shell | ||
| kubectl patch materialize <instance-name> \ | ||
| -n materialize-environment \ | ||
| --type='merge' \ | ||
| -p "{\"spec\": {\"requestRollout\": \"<lastCompletedRolloutRequest-value>\", \"environmentdImageRef\": \"<previous-environmentdImageRef>\" }}" | ||
| ``` |
There was a problem hiding this comment.
In general I like the idea of having examples, but I also think we need to also make sure we're providing adequate context on the interfaces we're providing. People are going to deploy this through terraform, kustomize, pulumi, and who knows what else.
I would keep what we have here, but also talk about this a bit more in the context of the Materialize spec, when in the rollout one might want to use this (and why). I also am not sure about this process. We don't support downgrade so we need to make sure we're not rolling back a completed upgrade.
There was a problem hiding this comment.
I also am not sure about this process. We don't support downgrade so we need to make sure we're not rolling back a completed upgrade.
This makes sense to me! Given the rollout request goes back to the one for the last successful upgrade however, wouldn't it not try to initiate a new rollout?
There was a problem hiding this comment.
I also am not sure about this process.
These are the instructions @doy-materialize gave me too. But what's the correct process here?
There was a problem hiding this comment.
i think i would probably just not try to restore environmentdImageRef or anything like that - there are a lot of different things that could have changed other than the version, and the specific changes don't really have anything to do with cancelling the rollout. i would just give instructions on how to restore the requestRollout field and leave it at that.
There was a problem hiding this comment.
I think if someone wanted to revert to the prior version the only way it would be safe is to first revert requestRollout then revert the environmentdImageRef, but it's probably fine to just show how to cancel without reverting this.
There was a problem hiding this comment.
not reverting environmentdImageRef wouldn't be unsafe - it won't try doing the upgrade again unless they explicitly trigger it
There was a problem hiding this comment.
Change is here 0283767. However I think I added this instruction because if you were to just reset the requestRollout field in upgrade-materialize.yaml but keep the environmentdImageRef you tried to update with, then it doesn't fully go back to the original state. Notably the balancerd pods don't get cleaned up:
But this might just be a bug w/ orchestratord
There was a problem hiding this comment.
oh, yeah, that's just a bug - i'm currently rewriting how balancerd gets deployed, i'll make sure to fix that as part of this
3 participants
Motivation
I feel like these are pretty important instructions to have!
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.