forked from rook/rook
Anas Khan
1630ad1049
archiveCrash is meant to silence the RECENT_CRASH health warning for a removed OSD by archiving its crash, but the early-return guard was inverted. GetCrashList unmarshals `ceph crash ls` into a non-nil slice on every success, so `if crash != nil` was always true. The function always logged "no ceph crash to silence" and the archive loop was unreachable, so OSD removal never archived the crash and the warning lingered. Guard on `len(crash) == 0` so only a genuinely empty list short-circuits. Reviving the loop surfaced two latent bugs it had always masked. On a non-empty list with no entry for the removed OSD the crash id stayed empty and the code ran `ceph crash archive ""`, which the mgr rejects with EINVAL and logs as a spurious error during routine OSD removal, so skip the archive when no crash matches. The loop also broke on the first match, but `ceph crash ls` is oldest-first and includes already-archived entries, so an OSD with several crashes had only its oldest one archived while RECENT_CRASH persisted; archive every matching entry instead of just the first. Extend TestArchiveCrash to cover all four cases: a single matching crash, a list with several matching crashes that are all archived, an empty list, and a non-empty list with no matching entry where nothing is archived. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> Signed-off-by: Joshua Hoblitt <josh@hoblitt.com>
250 lines
8.8 KiB
Go
250 lines
8.8 KiB
Go
/*
|
|
Copyright 2022 The Rook Authors. All rights reserved.
|
|
|
|
Licensed under the Apache License, Version 2.0 (the "License");
|
|
you may not use this file except in compliance with the License.
|
|
You may obtain a copy of the License at
|
|
|
|
http://www.apache.org/licenses/LICENSE-2.0
|
|
|
|
Unless required by applicable law or agreed to in writing, software
|
|
distributed under the License is distributed on an "AS IS" BASIS,
|
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
See the License for the specific language governing permissions and
|
|
limitations under the License.
|
|
*/
|
|
|
|
package osd
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/pkg/errors"
|
|
cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
|
|
"github.com/rook/rook/pkg/clusterd"
|
|
"github.com/rook/rook/pkg/daemon/ceph/client"
|
|
oposd "github.com/rook/rook/pkg/operator/ceph/cluster/osd"
|
|
testexec "github.com/rook/rook/pkg/operator/test"
|
|
exectest "github.com/rook/rook/pkg/util/exec/test"
|
|
"github.com/stretchr/testify/assert"
|
|
corev1 "k8s.io/api/core/v1"
|
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
|
"k8s.io/apimachinery/pkg/runtime"
|
|
k8stesting "k8s.io/client-go/testing"
|
|
)
|
|
|
|
func TestRemovePVCs(t *testing.T) {
|
|
pvcSuffix := 0
|
|
pvcReactor := func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
|
|
// PVCs are created with generateName used, and we need to capture the create calls and
|
|
// generate a name for them in order for PVCs to all have unique names.
|
|
createAction, ok := action.(k8stesting.CreateAction)
|
|
if !ok {
|
|
t.Fatal("not a create action")
|
|
return false, nil, nil
|
|
}
|
|
obj := createAction.GetObject()
|
|
pvc, ok := obj.(*corev1.PersistentVolumeClaim)
|
|
if !ok {
|
|
t.Fatal("not a PVC")
|
|
return false, nil, nil
|
|
}
|
|
if pvc.Name == "" {
|
|
pvc.Name = fmt.Sprintf("%s-%d", pvc.GenerateName, pvcSuffix)
|
|
logger.Info("generated name for PVC:", pvc.Name)
|
|
pvcSuffix++
|
|
}
|
|
// setting pvc.Name above modifies the action in-place before future reactors occur
|
|
// we want the default reactor to create the resource, so return false as if we did nothing
|
|
return false, nil, nil
|
|
}
|
|
ns := "testns"
|
|
ctx := context.TODO()
|
|
clusterInfo := client.AdminTestClusterInfo(ns)
|
|
|
|
t.Run("remove osd with data pvc", func(t *testing.T) {
|
|
clientset := testexec.New(t, 1)
|
|
clientset.PrependReactor("create", "persistentvolumeclaims", pvcReactor)
|
|
context := &clusterd.Context{
|
|
Clientset: clientset,
|
|
}
|
|
|
|
// Create 3 PVCs for two OSDs in the device set
|
|
deviceSet := cephv1.StorageClassDeviceSet{
|
|
Name: "mydata",
|
|
Count: 2,
|
|
Portable: true,
|
|
VolumeClaimTemplates: []cephv1.VolumeClaimTemplate{testVolumeClaim("data")},
|
|
SchedulerName: "custom-scheduler",
|
|
}
|
|
err := createTestPVCs(context, clusterInfo, deviceSet)
|
|
assert.NoError(t, err)
|
|
|
|
// Verify the PVCs all exist
|
|
pvcs, err := clientset.CoreV1().PersistentVolumeClaims(clusterInfo.Namespace).List(ctx, metav1.ListOptions{})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 2, len(pvcs.Items))
|
|
|
|
// Verify the PVCs all exist for the given OSD
|
|
selector := fmt.Sprintf("%s=%s", oposd.CephSetIndexLabelKey, "0")
|
|
pvcs, err = clientset.CoreV1().PersistentVolumeClaims(clusterInfo.Namespace).List(ctx, metav1.ListOptions{LabelSelector: selector})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 1, len(pvcs.Items))
|
|
assert.Equal(t, "mydata-data-0-0", pvcs.Items[0].Name)
|
|
|
|
// Remove the PVCs for one of the OSDs
|
|
removePVCs(context, clusterInfo, "mydata-data-0-0", false)
|
|
|
|
// Verify the PVCs all exist
|
|
pvcs, err = clientset.CoreV1().PersistentVolumeClaims(clusterInfo.Namespace).List(ctx, metav1.ListOptions{})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 1, len(pvcs.Items))
|
|
assert.Equal(t, "mydata-data-1-1", pvcs.Items[0].Name)
|
|
})
|
|
|
|
t.Run("remove osd with metadata and wal pvcs", func(t *testing.T) {
|
|
clientset := testexec.New(t, 1)
|
|
clientset.PrependReactor("create", "persistentvolumeclaims", pvcReactor)
|
|
context := &clusterd.Context{
|
|
Clientset: clientset,
|
|
}
|
|
|
|
// Create 3 PVCs for two OSDs in the device set
|
|
deviceSet := cephv1.StorageClassDeviceSet{
|
|
Name: "mydata",
|
|
Count: 2,
|
|
Portable: true,
|
|
VolumeClaimTemplates: []cephv1.VolumeClaimTemplate{testVolumeClaim("data"), testVolumeClaim("metadata"), testVolumeClaim("wal")},
|
|
SchedulerName: "custom-scheduler",
|
|
}
|
|
err := createTestPVCs(context, clusterInfo, deviceSet)
|
|
assert.NoError(t, err)
|
|
|
|
// Verify the PVCs all exist
|
|
pvcs, err := clientset.CoreV1().PersistentVolumeClaims(ns).List(ctx, metav1.ListOptions{})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 6, len(pvcs.Items))
|
|
|
|
// Verify the PVCs all exist for the given OSD
|
|
selector := fmt.Sprintf("%s=%s", oposd.CephSetIndexLabelKey, "0")
|
|
pvcs, err = clientset.CoreV1().PersistentVolumeClaims(clusterInfo.Namespace).List(ctx, metav1.ListOptions{LabelSelector: selector})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 3, len(pvcs.Items))
|
|
|
|
// Remove the PVCs for one of the OSDs
|
|
removePVCs(context, clusterInfo, "mydata-data-0-2", false)
|
|
|
|
// Verify the PVCs all deleted for the given OSD
|
|
pvcs, err = clientset.CoreV1().PersistentVolumeClaims(clusterInfo.Namespace).List(ctx, metav1.ListOptions{LabelSelector: selector})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 0, len(pvcs.Items))
|
|
// Verify the PVCs for the other OSD still exist
|
|
pvcs, err = clientset.CoreV1().PersistentVolumeClaims(ns).List(ctx, metav1.ListOptions{})
|
|
assert.NoError(t, err)
|
|
assert.Equal(t, 3, len(pvcs.Items))
|
|
})
|
|
}
|
|
|
|
func createTestPVCs(clusterdContext *clusterd.Context, clusterInfo *client.ClusterInfo, deviceSet cephv1.StorageClassDeviceSet) error {
|
|
spec := cephv1.ClusterSpec{
|
|
Storage: cephv1.StorageScopeSpec{StorageClassDeviceSets: []cephv1.StorageClassDeviceSet{deviceSet}},
|
|
}
|
|
cluster := oposd.New(clusterdContext, clusterInfo, spec, "")
|
|
return cluster.PrepareStorageClassDeviceSets()
|
|
}
|
|
|
|
func testVolumeClaim(name string) cephv1.VolumeClaimTemplate {
|
|
storageClass := "mysource"
|
|
claim := cephv1.VolumeClaimTemplate{Spec: corev1.PersistentVolumeClaimSpec{
|
|
StorageClassName: &storageClass,
|
|
}}
|
|
claim.Name = name
|
|
return claim
|
|
}
|
|
|
|
func TestArchiveCrash(t *testing.T) {
|
|
newExecutor := func(crashList string, archived *[]string) *exectest.MockExecutor {
|
|
executor := &exectest.MockExecutor{}
|
|
executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
|
|
if args[0] == "crash" && args[1] == "ls" {
|
|
return crashList, nil
|
|
}
|
|
if args[0] == "crash" && args[1] == "archive" {
|
|
*archived = append(*archived, args[2])
|
|
return "", nil
|
|
}
|
|
return "", errors.Errorf("unexpected ceph command %q", args)
|
|
}
|
|
return executor
|
|
}
|
|
|
|
t.Run("archives the crash for the removed osd", func(t *testing.T) {
|
|
crashList := `[
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_ca918f58-c078-444d-a91a-bd972c14c155",
|
|
"entity_name": "osd.1"
|
|
},
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_bd972c14-c155-444d-a91a-ca918f58c078",
|
|
"entity_name": "osd.0"
|
|
}
|
|
]`
|
|
var archived []string
|
|
context := &clusterd.Context{Executor: newExecutor(crashList, &archived)}
|
|
|
|
archiveCrash(context, client.AdminTestClusterInfo("mycluster"), 0)
|
|
assert.Equal(t, []string{"2020-11-09_13:58:08.230130Z_bd972c14-c155-444d-a91a-ca918f58c078"}, archived)
|
|
})
|
|
|
|
t.Run("archives every crash for the removed osd", func(t *testing.T) {
|
|
// ceph crash ls is oldest-first and includes already-archived entries, so an osd
|
|
// with more than one crash must have all of them archived, not just the first match.
|
|
crashList := `[
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_00000000-0000-0000-0000-000000000000",
|
|
"entity_name": "osd.0"
|
|
},
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_ca918f58-c078-444d-a91a-bd972c14c155",
|
|
"entity_name": "osd.1"
|
|
},
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_bd972c14-c155-444d-a91a-ca918f58c078",
|
|
"entity_name": "osd.0"
|
|
}
|
|
]`
|
|
var archived []string
|
|
context := &clusterd.Context{Executor: newExecutor(crashList, &archived)}
|
|
|
|
archiveCrash(context, client.AdminTestClusterInfo("mycluster"), 0)
|
|
assert.Equal(t, []string{
|
|
"2020-11-09_13:58:08.230130Z_00000000-0000-0000-0000-000000000000",
|
|
"2020-11-09_13:58:08.230130Z_bd972c14-c155-444d-a91a-ca918f58c078",
|
|
}, archived)
|
|
})
|
|
|
|
t.Run("does not archive when there is no crash", func(t *testing.T) {
|
|
var archived []string
|
|
context := &clusterd.Context{Executor: newExecutor("[]", &archived)}
|
|
|
|
archiveCrash(context, client.AdminTestClusterInfo("mycluster"), 0)
|
|
assert.Empty(t, archived)
|
|
})
|
|
|
|
t.Run("does not archive when no crash matches the removed osd", func(t *testing.T) {
|
|
crashList := `[
|
|
{
|
|
"crash_id": "2020-11-09_13:58:08.230130Z_ca918f58-c078-444d-a91a-bd972c14c155",
|
|
"entity_name": "osd.1"
|
|
}
|
|
]`
|
|
var archived []string
|
|
context := &clusterd.Context{Executor: newExecutor(crashList, &archived)}
|
|
|
|
archiveCrash(context, client.AdminTestClusterInfo("mycluster"), 0)
|
|
assert.Empty(t, archived)
|
|
})
|
|
}
|