Blue-green deployment causes leaking PVs

[ruifung]

Is your feature request related to a problem? Please describe.
With the introduction of the new blue-green deployment method, every change that impacts the manifests results in a new StatefulSet being generated. While this in itself isn't a problem. What could be a problem is the fact that each statefulset created and deleted will leave behind N PVCs and their associated PV.

This could cause unwanted costs and/or resource wastage where PVs are backed by persistent disks or similar mechanisms due to leaking PVs until manually cleaned up.

Describe the solution you'd like
Some way to automatically clean up the PVCs left behind due to blue-green deployment after deletion of the old statefulset.

Describe alternatives you've considered
An option to disable blue-green deployment and revert to using StatefulSet RollingUpdate behavior.

Additional context
Add any other context or screenshots about the feature request here.

[Rory-Z]

Hi @ruifung because PVC stores some data, I think it is very dangerous for Operator to delete it. I hope to keep PVC for data recovery in certain situations.

Maybe I can provide a revisionHistoryLimit like deployment. I will delete the sts and its associated PVC that exceeds the revisionHistoryLimit. What do you think?

[ruifung]

Honestly, I'd personally have preferred to just have the previous behaviour where it would just patch the STS and let the STS do a rolling update.

But I suppose having a revisionHistoryLimit that will clean up the STS, and I think probably a separate option that enables PVC cleanup for the STS. So the STS cleanup can be enabled without enabling PVC cleanup if desired. This would certainly limit the amount of STS/PVCs that are left behind.

[Rory-Z]

Honestly, I'd personally have preferred to just have the previous behaviour where it would just patch the STS and let the STS do a rolling update.

Very reasonable, let me think about how to achieve it. This may be implemented in version 2.3 and the new APIVersion.

But I suppose having a revisionHistoryLimit that will clean up the STS, and I think probably a separate option that enables PVC cleanup for the STS. So the STS cleanup can be enabled without enabling PVC cleanup if desired. This would certainly limit the amount of STS/PVCs that are left behind.

I think STS and PVC should maintain a corresponding relationship in order to perform rollback operations in certain situations. If there is only STS without PVC, or only PVC without STS, then the rollback operation would be meaningless.

[Rory-Z]

Hi @ruifung After discussing with colleagues, we believe it is reasonable not to retain the original rolling update of sts:

1. In the cluster of core + repl, randomly killing off core nodes may cause side effects. These side effects can be recovered during normal operation, but in rolling upgrades, they may cause some unknown errors. We do not have enough testing yet.
2. During the rolling upgrade process, the client may reconnect multiple times to the wrong Pod. For example, if we currently have three Pods named old 0, old 1, and old 2, when we perform a rolling upgrade and kill off old 2, clients connected to old 2 may reconnect to old 1. Then when old 1 is killed off, clients will reconnect again to old 0. Only after old 0 is killed off will clients finally reconnect to the correct new pod.
3. The svc of k8s defaults to using a round-robin strategy, which means that when rolling update pods, the last created pod has the least number of connections.

Due to the above reasons, we did not retain the rolling update of sts. Maybe revisionHistoryLimit can meet your needs? For example, set revisionHistoryLimit = 1?

[ruifung]

Perhaps it will. I suppose I could always fall back to using a helm deployment instead if it does not?

Also, will this leave behind "offline" nodes in emqx or will that clean up offline nodes from the cluster state eventually?

[Rory-Z]

Also, will this leave behind "offline" nodes in emqx or will that clean up offline nodes from the cluster state eventually?

Yes, it will leave a stop node in EMQX cluster status, because the default value of the cluster.autoclean is 24h, so it will clean stop nodes after 24h

[Rory-Z]

Hi @ruifung add reversionHistoryLimit in #936

[Rory-Z]

EMQX Operator 2.2.2 has been released, I will close this issue