[controller] Add logic to update StatefulSets #236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
==========================================
+ Coverage 75.59% 76.20% +0.61%
==========================================
Files 30 30
Lines 2118 2152 +34
==========================================
+ Hits 1601 1640 +39
+ Misses 392 379 -13
- Partials 125 133 +8 Continue to review full report at Codecov.
|
pkg/controller/controller.go
Outdated
| // ConfigMapName fields in the ClusterSpec don't match what's in the StatefulSet then | ||
| // this function will update the provided StatefulSet to match the desired state and | ||
| // return true. | ||
| func shouldUpdateStatefulSet(spec myspec.ClusterSpec, sts *appsv1.StatefulSet) (bool, error) { |
There was a problem hiding this comment.
Image and config updates are definitely the most common case we want to cover, so from that perspective this looks good. There are definitely other cases where the cluster CRD changes and we want to update the statefulsets, and I'm wondering how to address that here. Two thoughts:
-
Any idea what happens if we try to
DeepEqualthe generated spec and the current spec on the cluster? I'm guessing the Kubernetes API makes some sort of transformations that make that not totally possible, but I'm curious. -
Assuming
DeepEqualdoesn't work, what do you think about a second annotation that will trigger anUpdateregardless of whether image and config has changed?
There was a problem hiding this comment.
I've always had difficulties using DeepEqual in the past because the API server sets default (non-zero) values for so many fields. Based on this kubebuilder issue though, it seems that we could use DeepDerivative instead since it "is similar to DeepEqual except that unset fields in a1 are ignored ". That issue also mentions github.com/banzaicloud/k8s-objectmatcher as another alternative if DeepDerivative doesn't work for us. If neither package is sufficient though I'm not opposed to having a force-update annotation or something similar which will always trigger an update.
pkg/controller/controller.go
Outdated
| // expected state. The method returns the updated StatefulSet if so, otherwise nil. | ||
| func updatedStatefulSet( | ||
| actual *appsv1.StatefulSet, cluster *myspec.M3DBCluster, isoGroup myspec.IsolationGroup, | ||
| ) (*appsv1.StatefulSet, error) { |
There was a problem hiding this comment.
Can we modify the return signature to be *StatefulSet, bool, error and use the bool as whether we upgraded rather than implicit nil?
Definitely! I tested the changes on a running cluster yesterday using a local build, but want to deploy the changes and do some more thorough testing today. So I'll update the docs after that once I've gotten a better handle on the UX. |
This commit to the controller adds support for updating
StatefulSet's when their image or config map are out of date with theClusterSpec. After checking that the cluster is healthy, the controller will next check if anyStatefulSetneeds to be updated, and will then perform any needed updates one by one. To ensure that the controller will only perform updates when it is safe to do so, the controller will check for an annotation that must be set by a user to indicate the controller can perform an update.