Skip to content

Commit

Permalink
controller: don't count replica count in revision hash calculation (#10)
Browse files Browse the repository at this point in the history
Previous fix to prevent replica count update causing full
regionserver restart has few drawbacks due to the fact that
hash calculation is still based on historical statefulset count,
which means any time CRD's current replica count is incorrect
(i.e. CRD is out of sync, operator crashed and never finished
reconciliation run successfully, act of nature...) this still
creates a new hash that causes regionservers to erroroneously
restart.

We're removing replica count altogether from revision hash
calculation, and accepting the operational risk that the first time
this logic is introduced it will restart all components as revision
hash will be different.
  • Loading branch information
pidren authored Jun 4, 2021
1 parent 9ff52e7 commit 38f249b
Showing 1 changed file with 2 additions and 8 deletions.
10 changes: 2 additions & 8 deletions controllers/hbase_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,9 +572,9 @@ func (r *HBaseReconciler) ensureStatefulSetPods(ctx context.Context, sts *appsv1
func (r *HBaseReconciler) ensureStatefulSet(hb *hbasev1.HBase,
stsName, cmName types.NamespacedName, ss hbasev1.ServerSpec) (*appsv1.StatefulSet, bool, error) {
actual := &appsv1.StatefulSet{}
expected, expectedRevision := r.statefulSet(hb, stsName, cmName, ss)
if err := r.Get(context.TODO(), stsName, actual); err != nil {
if errors.IsNotFound(err) {
expected, _ := r.statefulSet(hb, stsName, cmName, ss, 0)

if err = controllerutil.SetControllerReference(hb, expected, r.Scheme); err != nil {
return nil, false, err
Expand All @@ -587,8 +587,6 @@ func (r *HBaseReconciler) ensureStatefulSet(hb *hbasev1.HBase,
}
return nil, false, err
}
expected, expectedRevision := r.statefulSet(hb, stsName, cmName, ss, *actual.Spec.Replicas)

r.Log.Info("updating revision", "sameRevision", expectedRevision != actual.Annotations[HBaseControllerRevisionKey])
r.Log.Info("updating replica count", "sameReplicaCount", *actual.Spec.Replicas != *expected.Spec.Replicas)

Expand Down Expand Up @@ -643,14 +641,10 @@ const (
)

func (r *HBaseReconciler) statefulSet(hb *hbasev1.HBase,
stsName, cmName types.NamespacedName, ss hbasev1.ServerSpec, rc int32) (*appsv1.StatefulSet, string) {
stsName, cmName types.NamespacedName, ss hbasev1.ServerSpec) (*appsv1.StatefulSet, string) {
spec := (&ss.PodSpec).DeepCopy()
spec.Volumes = append(spec.Volumes, configMapVolume(cmName))
stsSpec := appsv1.StatefulSetSpec{
// We don't want to use replica value as part of revision change (since
// scaling a statefulset is not the same as a new revision) so we just
// set current count for now just for hash calculation.
Replicas: pointer.Int32Ptr(rc),
PodManagementPolicy: appsv1.ParallelPodManagement,
// OnDelete because we managed the pod restarts ourselves
UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
Expand Down

0 comments on commit 38f249b

Please sign in to comment.