Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ceph: reconcile osd pdb if allowed disruption is 0 #8698

Merged
merged 1 commit into from Sep 15, 2021
Merged

ceph: reconcile osd pdb if allowed disruption is 0 #8698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions pkg/operator/ceph/disruption/clusterdisruption/osd.go
View file
Open in desktop
Expand Up @@ -407,6 +407,21 @@ func (r *ReconcileClusterDisruption) reconcilePDBsForOSDs(
return reconcile.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
}

// requeue if allowed disruptions in the default PDB is 0
allowedDisruptions, err := r.getAllowedDisruptions(osdPDBAppName, request.Namespace)
if err != nil {
if apierrors.IsNotFound(err) {
logger.Debugf("default osd pdb %q not found. Skipping reconcile", osdPDBAppName)
return reconcile.Result{}, nil
}
return reconcile.Result{}, errors.Wrapf(err, "failed to get allowed disruptions count from default osd pdb %q.", osdPDBAppName)
leseb marked this conversation as resolved.
Show resolved Hide resolved
}

if allowedDisruptions == 0 {
logger.Info("reconciling osd pdb reconciler as the allowed disruptions in default pdb is 0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an OSD is crashlooping, but the PGs are healthy because they have been backfilled, this will cause the pdb reconcile to continue every 30s until the OSD stops crashing, right? Or will this be skipped if the PGs are healthy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When OSDs are crashlooping and PGs are not healthy, then we won't be seeing the default PDB (osdPDBAppName). Only blocking PDBs would be there. This condition won't hit because of the error in line 412

Its a good point though. I'll test it a bit more and confirm.

Copy link
Contributor Author

@sp98 sp98 Sep 14, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So OSD goes into CLBO and pgs become degraded. When pgs become active again after re-balancing, we end up with following:

Every 2.0s: oc get pdb -n rook-ceph                                     localhost.localdomain: Tue Sep 14 12:04:08 2021

NAME                MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
rook-ceph-mon-pdb   N/A             1                 1                     37m
rook-ceph-osd       N/A             1                 0                     112s

Observe Allowed Disruptions is 0 because the OSD is still down. PDB reconciles because this is hit

Now user purges the OSD for the CLBO OSD pod. PBDs move back to the desired state again

$ oc get pdb -n rook-ceph 
NAME                MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
rook-ceph-mon-pdb   N/A             1                 1                     71m
rook-ceph-osd       N/A             1                 1                     36m

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so the only way to return to the state of clean OSD PVCs is to remove/replace the crashing OSD. And from your link, the PDBs will continue reconciling every 15s? That might be worth looking into for a separate PR so it isn't so frequent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thats true. Only way is to remove the crashed OSD. This behavior would still have been the same even before this PR. I'll a take a look (in a separate PR) if we handle this situation better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sp98 please open up an issue to track this. Thanks

return reconcile.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
}

return reconcile.Result{}, nil
}

Expand Down Expand Up @@ -640,6 +655,30 @@ func getLastDrainTimeStamp(pdbStateMap *corev1.ConfigMap, key string) (time.Time
return lastDrainTimeStamp, nil
}

func (r *ReconcileClusterDisruption) getAllowedDisruptions(pdbName, namespace string) (int32, error) {
leseb marked this conversation as resolved.
Show resolved Hide resolved
usePDBV1Beta1, err := k8sutil.UsePDBV1Beta1Version(r.context.ClusterdContext.Clientset)
if err != nil {
return -1, errors.Wrap(err, "failed to fetch pdb version")
}
if usePDBV1Beta1 {
pdb := &policyv1beta1.PodDisruptionBudget{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: namespace}, pdb)
if err != nil {
return -1, err
}

return pdb.Status.DisruptionsAllowed, nil
}

pdb := &policyv1.PodDisruptionBudget{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: namespace}, pdb)
if err != nil {
return -1, err
}

return pdb.Status.DisruptionsAllowed, nil
}

func resetPDBConfig(pdbStateMap *corev1.ConfigMap) {
pdbStateMap.Data[drainingFailureDomainKey] = ""
delete(pdbStateMap.Data, drainingFailureDomainDurationKey)
Expand Down
28 changes: 28 additions & 0 deletions pkg/operator/ceph/disruption/clusterdisruption/osd_test.go
View file
Open in desktop
Expand Up @@ -464,3 +464,31 @@ func TestHasNodeDrained(t *testing.T) {
assert.NoError(t, err)
assert.True(t, expected)
}

func TestGetAllowedDisruptions(t *testing.T) {
r := getFakeReconciler(t)
clientset := test.New(t, 3)
test.SetFakeKubernetesVersion(clientset, "v1.21.0")
r.context = &controllerconfig.Context{ClusterdContext: &clusterd.Context{Clientset: clientset}}

// Default PDB is not available
allowedDisruptions, err := r.getAllowedDisruptions(osdPDBAppName, namespace)
assert.Error(t, err)
assert.Equal(t, int32(-1), allowedDisruptions)

// Default PDB is available
pdb := &policyv1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{
Name: osdPDBAppName,
Namespace: namespace,
},
Status: policyv1.PodDisruptionBudgetStatus{
DisruptionsAllowed: int32(0),
},
}
err = r.client.Create(context.TODO(), pdb)
assert.NoError(t, err)
allowedDisruptions, err = r.getAllowedDisruptions(osdPDBAppName, namespace)
assert.NoError(t, err)
assert.Equal(t, int32(0), allowedDisruptions)
}