From fbf3837f13317351803e52094b51a74a462eab13 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Fri, 6 Dec 2019 15:32:04 -0700 Subject: [PATCH 1/2] ceph: stop removing osds for any cephcluster CR update Removing an OSD unintentionally can result in data loss, which is to be prevented at all costs. Rook's functionality to remove OSDs automatically when a node was removed from the cluster CR has led on several occasions to users having OSDs unintentionally removed from their cluster. If the admin decides they really need to remove OSDs they should do it as a one-time action that they intentionally run rather than the operator attempting to manage the deletion with desired state. The risk of misinterpreting the desired state, even with a typo, is too great. Signed-off-by: Travis Nielsen --- PendingReleaseNotes.md | 1 + pkg/daemon/ceph/osd/agent.go | 58 -------- pkg/daemon/ceph/osd/agent_test.go | 36 ----- pkg/daemon/ceph/osd/daemon.go | 86 ++---------- pkg/daemon/ceph/osd/daemon_test.go | 67 ++-------- pkg/operator/ceph/cluster/osd/osd.go | 169 ------------------------ pkg/operator/ceph/cluster/osd/spec.go | 5 +- pkg/operator/ceph/cluster/osd/status.go | 80 ----------- 8 files changed, 19 insertions(+), 483 deletions(-) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index ef8c7c5df..697acdc68 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -26,6 +26,7 @@ - When the operator is upgraded, the mgr and OSDs (not running on PVC) won't be restarted if the Rook binary version changes - Rook is now able to create and manage Ceph clients [client crd](Documentation/ceph-client-crd.html). - OSDs: + - Rook will no longer automatically remove OSDs if nodes are removed from the cluster CR to avoid the risk of destroying OSDs unintentionally - Ceph OSD's admin socket is now placed under Ceph's default system location `/run/ceph`. - The on-host log directory for OSDs was set incorrectly to `//log`; fix this to be `/log/`, the same as other daemons. diff --git a/pkg/daemon/ceph/osd/agent.go b/pkg/daemon/ceph/osd/agent.go index 32d522f28..bf40205d3 100644 --- a/pkg/daemon/ceph/osd/agent.go +++ b/pkg/daemon/ceph/osd/agent.go @@ -126,30 +126,6 @@ func (a *OsdAgent) configureDirs(context *clusterd.Context, dirs map[string]int) return osds, lastErr } -func (a *OsdAgent) removeDirs(context *clusterd.Context, removedDirs map[string]int) error { - if len(removedDirs) == 0 { - return nil - } - - // walk through each of the OSD directories and remove them - var failedDirs []string - for dir, osdID := range removedDirs { - if err := a.removeOSDConfigDir(dir, osdID); err != nil { - errMsg := fmt.Sprintf("failed to remove osd.%d. %+v", osdID, err) - logger.Error(errMsg) - failedDirs = append(failedDirs, dir) - continue - } - } - - if len(failedDirs) > 0 { - // at least one OSD failed, return an overall error - return errors.Errorf("failed to cleanup directories: %+v", failedDirs) - } - - return nil -} - func (a *OsdAgent) configureDevices(context *clusterd.Context, devices *DeviceOsdMapping) ([]oposd.OSDInfo, error) { var cvSupported bool @@ -248,40 +224,6 @@ func getDeviceLVPath(context *clusterd.Context, deviceName string) (string, erro return output, nil } -func (a *OsdAgent) removeDevices(context *clusterd.Context, removedDevicesScheme *config.PerfScheme) error { - if removedDevicesScheme == nil || len(removedDevicesScheme.Entries) == 0 { - return nil - } - - var errorMessages []string - - // now start removing each OSD since they should now be running - for _, entry := range removedDevicesScheme.Entries { - - if err := a.removeOSDConfigDir(context.ConfigDir, entry.ID); err != nil { - errMsg := fmt.Sprintf("failed to remove osd.%d. %+v", entry.ID, err) - logger.Error(errMsg) - errorMessages = append(errorMessages, errMsg) - continue - } - - // remove OSD from partition scheme map - if err := config.RemoveFromScheme(entry, a.kv, config.GetConfigStoreName(a.nodeName)); err != nil { - errMsg := fmt.Sprintf("failed to remove osd.%d from scheme. %+v", entry.ID, err) - logger.Error(errMsg) - errorMessages = append(errorMessages, errMsg) - continue - } - } - - if len(errorMessages) > 0 { - // at least one OSD failed, return an overall error - return errors.Errorf("%s", strings.Join(errorMessages, "\n")) - } - - return nil -} - // computes a partitioning scheme for all the given desired devices. This could be devices already in use, // devices dedicated to metadata, and devices with all bluestore partitions collocated. func (a *OsdAgent) getPartitionPerfScheme(context *clusterd.Context, devices *DeviceOsdMapping, skipNewDevices bool) (*config.PerfScheme, *DeviceOsdMapping, error) { diff --git a/pkg/daemon/ceph/osd/agent_test.go b/pkg/daemon/ceph/osd/agent_test.go index 9736e32b6..5025572b9 100644 --- a/pkg/daemon/ceph/osd/agent_test.go +++ b/pkg/daemon/ceph/osd/agent_test.go @@ -340,42 +340,6 @@ func TestOSDAgentNoDevices(t *testing.T) { assert.Equal(t, 2, execWithOutputCount) } -func TestRemoveDevices(t *testing.T) { - configDir, _ := ioutil.TempDir("", "") - defer os.RemoveAll(configDir) - os.MkdirAll(configDir, 0755) - - nodeName := "node0347" - agent, mockExec, context := createTestAgent(t, "none", configDir, nodeName, &config.StoreConfig{StoreType: config.Bluestore}) - agent.devices[0].IsFilter = true - - _, removedDevices, _ := mockPartitionSchemeEntry(t, 1, "sdx", &agent.storeConfig, agent.kv, nodeName) - - osdUsageCallCount := 0 - mockExec.MockExecuteCommandWithOutputFile = func(debug bool, actionName, command, outputFile string, args ...string) (string, error) { - logger.Infof("Command: %s %+v", command, args) - if args[0] == "osd" && args[1] == "df" { - osdUsageCallCount++ - if osdUsageCallCount == 1 { - return `{"nodes":[{"id":1,"name":"osd.1","kb_used":100,"pgs":100}]}`, nil - } - // subsequent times return a value indicating that the OSD is no longer using any space and has no PGs (because its been migrated) - return `{"nodes":[{"id":1,"name":"osd.1","kb_used":0,"pgs":0}]}`, nil - } - if args[0] == "pg" && args[1] == "dump" && args[2] == "pgs_brief" { - return `[{"pgid":"1.ef","state":"active+clean","up":[5,6,7],"up_primary":5,"acting":[5,6,7],"acting_primary":5}]`, nil - } - if args[0] == "status" { - return `{"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil - } - - return "", nil - } - - err := agent.removeDevices(context, removedDevices) - assert.Nil(t, err) -} - func createTestAgent(t *testing.T, devices, configDir, nodeName string, storeConfig *config.StoreConfig) (*OsdAgent, *exectest.MockExecutor, *clusterd.Context) { forceFormat := false if storeConfig == nil { diff --git a/pkg/daemon/ceph/osd/daemon.go b/pkg/daemon/ceph/osd/daemon.go index 53c5bd629..f28fbb16e 100644 --- a/pkg/daemon/ceph/osd/daemon.go +++ b/pkg/daemon/ceph/osd/daemon.go @@ -218,12 +218,6 @@ func Provision(context *clusterd.Context, agent *OsdAgent, crushLocation string) return errors.Wrapf(err, "failed to get available devices") } - // determine the set of removed OSDs and the node's crush name (if needed) - removedDevicesScheme, _, err := getRemovedDevices(agent) - if err != nil { - return errors.Wrapf(err, "failed to get removed devices") - } - // orchestration is about to start, update the status status = oposd.OrchestrationStatus{Status: oposd.OrchestrationStatusOrchestrating, PvcBackedOSD: agent.pvcBacked} if err := oposd.UpdateNodeStatus(agent.kv, agent.nodeName, status); err != nil { @@ -245,7 +239,7 @@ func Provision(context *clusterd.Context, agent *OsdAgent, crushLocation string) // determine the set of directories that can/should be used for OSDs, with the default dir if no devices were specified. save off the node's crush name if needed. logger.Infof("devices = %+v", deviceOSDs) devicesConfigured := len(deviceOSDs) > 0 - dirs, removedDirs, err := getDataDirs(context, agent.kv, agent.directories, devicesConfigured, agent.nodeName) + dirs, err := getDataDirs(context, agent.kv, agent.directories, devicesConfigured, agent.nodeName) if err != nil { return errors.Wrapf(err, "failed to get data dirs") } @@ -257,17 +251,6 @@ func Provision(context *clusterd.Context, agent *OsdAgent, crushLocation string) return errors.Wrapf(err, "failed to configure dirs %+v", dirs) } - // now we can start removing OSDs from devices and directories - logger.Infof("removing osd devices: %+v", removedDevicesScheme) - if err := agent.removeDevices(context, removedDevicesScheme); err != nil { - return errors.Wrapf(err, "failed to remove devices") - } - - logger.Infof("removing osd dirs: %+v", removedDirs) - if err := agent.removeDirs(context, removedDirs); err != nil { - return errors.Wrapf(err, "failed to remove dirs") - } - logger.Info("saving osd dir map") if err := config.SaveOSDDirMap(agent.kv, agent.nodeName, dirs); err != nil { return errors.Wrapf(err, "failed to save osd dir map") @@ -299,12 +282,6 @@ func Provision(context *clusterd.Context, agent *OsdAgent, crushLocation string) func getAvailableDevices(context *clusterd.Context, desiredDevices []DesiredDevice, metadataDevice string, pvcBacked bool) (*DeviceOsdMapping, error) { available := &DeviceOsdMapping{Entries: map[string]*DeviceOsdIDEntry{}} - - if isRemovingNode(desiredDevices) { - // the node is being removed, just return an empty set - return available, nil - } - for _, device := range context.Devices { if device.Type == sys.PartType { continue @@ -385,15 +362,8 @@ func getAvailableDevices(context *clusterd.Context, desiredDevices []DesiredDevi return available, nil } -func isRemovingNode(devices []DesiredDevice) bool { - if len(devices) != 1 { - return false - } - return oposd.IsRemovingNode(devices[0].Name) -} - func getDataDirs(context *clusterd.Context, kv *k8sutil.ConfigMapKVStore, desiredDirs string, - devicesSpecified bool, nodeName string) (dirs, removedDirs map[string]int, err error) { + devicesSpecified bool, nodeName string) (dirs map[string]int, err error) { var dirList []string if desiredDirs != "" { @@ -405,34 +375,32 @@ func getDataDirs(context *clusterd.Context, kv *k8sutil.ConfigMapKVStore, desire // still important for determining when the fallback osd may be deleted. noDirsOrDevicesSpecified := len(dirList) == 0 && !devicesSpecified - removedDirs = make(map[string]int) - dirMap, err := config.LoadOSDDirMap(kv, nodeName) if err == nil { // we have an existing saved dir map, merge the user specified directories into it addDirsToDirMap(dirList, &dirMap) // determine which dirs are still active, which should be removed, then return them - activeDirs, removedDirs := getActiveAndRemovedDirs(dirList, dirMap, context.ConfigDir, noDirsOrDevicesSpecified) - return activeDirs, removedDirs, nil + activeDirs := getActiveDirs(dirList, dirMap, context.ConfigDir, noDirsOrDevicesSpecified) + return activeDirs, nil } if !kerrors.IsNotFound(err) { // real error when trying to load the osd dir map, return the err - return nil, nil, errors.Wrapf(err, "failed to load OSD dir map") + return nil, errors.Wrapf(err, "failed to load OSD dir map") } // the osd dirs map doesn't exist yet if len(dirList) == 0 { // no dirs should be used because the user has requested no dirs but they have requested devices - return map[string]int{}, removedDirs, nil + return map[string]int{}, nil } // add the specified dirs to the map and return it dirMap = make(map[string]int, len(dirList)) addDirsToDirMap(dirList, &dirMap) - return dirMap, removedDirs, nil + return dirMap, nil } func addDirsToDirMap(dirList []string, dirMap *map[string]int) { @@ -444,41 +412,8 @@ func addDirsToDirMap(dirList []string, dirMap *map[string]int) { } } -func getRemovedDevices(agent *OsdAgent) (*config.PerfScheme, *DeviceOsdMapping, error) { - removedDevicesScheme := config.NewPerfScheme() - removedDevicesMapping := &DeviceOsdMapping{Entries: map[string]*DeviceOsdIDEntry{}} - - if !isRemovingNode(agent.devices) { - // TODO: support more removed device scenarios beyond just entire node removal - return removedDevicesScheme, removedDevicesMapping, nil - } - - scheme, err := config.LoadScheme(agent.kv, config.GetConfigStoreName(agent.nodeName)) - if err != nil { - return nil, nil, errors.Wrapf(err, "failed to load agent's partition scheme") - } - - for _, entry := range scheme.Entries { - // determine which partition the data lives on for this entry - dataDetails, ok := entry.Partitions[entry.GetDataPartitionType()] - if !ok || dataDetails == nil { - return nil, nil, errors.Errorf("failed to find data partition for entry %+v", entry) - } - - // add the current scheme entry to the removed devices scheme and its device to the removed - // devices mapping - removedDevicesScheme.Entries = append(removedDevicesScheme.Entries, entry) - removedDevicesMapping.Entries[dataDetails.Device] = &DeviceOsdIDEntry{Data: entry.ID} - } - - return removedDevicesScheme, removedDevicesMapping, nil -} - -func getActiveAndRemovedDirs( - currentDirList []string, savedDirMap map[string]int, configDir string, noDirsOrDevicesSpecified bool, -) (activeDirs, removedDirs map[string]int) { +func getActiveDirs(currentDirList []string, savedDirMap map[string]int, configDir string, noDirsOrDevicesSpecified bool) (activeDirs map[string]int) { activeDirs = map[string]int{} - removedDirs = map[string]int{} for savedDir, id := range savedDirMap { foundSavedDir := false @@ -499,13 +434,10 @@ func getActiveAndRemovedDirs( if foundSavedDir { // the saved dir is still active activeDirs[savedDir] = id - } else { - // the saved dir was not found in the current dir list, meaning the user wants this dir removed - removedDirs[savedDir] = id } } - return activeDirs, removedDirs + return activeDirs } //releaseLVMDevice deactivates the LV to release the device. diff --git a/pkg/daemon/ceph/osd/daemon_test.go b/pkg/daemon/ceph/osd/daemon_test.go index 091a280c7..76153cbea 100644 --- a/pkg/daemon/ceph/osd/daemon_test.go +++ b/pkg/daemon/ceph/osd/daemon_test.go @@ -93,30 +93,26 @@ func TestGetDataDirs(t *testing.T) { nodeName := "node6046" // user has specified devices to use, no dirs should be returned - dirMap, removedDirMap, err := getDataDirs(context, kv, "", true, nodeName) + dirMap, err := getDataDirs(context, kv, "", true, nodeName) assert.Nil(t, err) assert.Equal(t, 0, len(dirMap)) - assert.Equal(t, 0, len(removedDirMap)) // user has no devices specified, should NO LONGER return default dir - dirMap, removedDirMap, err = getDataDirs(context, kv, "", false, nodeName) + dirMap, err = getDataDirs(context, kv, "", false, nodeName) assert.Nil(t, err) assert.Equal(t, 0, len(dirMap)) - assert.Equal(t, 0, len(removedDirMap)) // user has no devices specified but does specify dirs, those should be returned - dirMap, removedDirMap, err = getDataDirs(context, kv, "/rook/dir1", false, nodeName) + dirMap, err = getDataDirs(context, kv, "/rook/dir1", false, nodeName) assert.Nil(t, err) assert.Equal(t, 1, len(dirMap)) assert.Equal(t, unassignedOSDID, dirMap["/rook/dir1"]) - assert.Equal(t, 0, len(removedDirMap)) // user has devices specified and also specifies dirs, those should be returned - dirMap, removedDirMap, err = getDataDirs(context, kv, "/rook/dir1", true, nodeName) + dirMap, err = getDataDirs(context, kv, "/rook/dir1", true, nodeName) assert.Nil(t, err) assert.Equal(t, 1, len(dirMap)) assert.Equal(t, unassignedOSDID, dirMap["/rook/dir1"]) - assert.Equal(t, 0, len(removedDirMap)) // simulate an OSD ID being assigned to the dir dirMap["/rook/dir1"] = 1 @@ -125,12 +121,11 @@ func TestGetDataDirs(t *testing.T) { assert.Nil(t, err) // user has specified devices and also a new directory to use. it should be added to the dir map - dirMap, removedDirMap, err = getDataDirs(context, kv, "/rook/dir1,/tmp/mydir", true, nodeName) + dirMap, err = getDataDirs(context, kv, "/rook/dir1,/tmp/mydir", true, nodeName) assert.Nil(t, err) assert.Equal(t, 2, len(dirMap)) assert.Equal(t, 1, dirMap["/rook/dir1"]) assert.Equal(t, unassignedOSDID, dirMap["/tmp/mydir"]) - assert.Equal(t, 0, len(removedDirMap)) // simulate that the user's dir got an OSD by assigning it an ID dirMap["/tmp/mydir"] = 23 @@ -138,20 +133,17 @@ func TestGetDataDirs(t *testing.T) { assert.Nil(t, err) // user is still specifying the 2 directories, we should get back their IDs - dirMap, removedDirMap, err = getDataDirs(context, kv, "/rook/dir1,/tmp/mydir", true, nodeName) + dirMap, err = getDataDirs(context, kv, "/rook/dir1,/tmp/mydir", true, nodeName) assert.Nil(t, err) assert.Equal(t, 2, len(dirMap)) assert.Equal(t, 1, dirMap["/rook/dir1"]) assert.Equal(t, 23, dirMap["/tmp/mydir"]) - assert.Equal(t, 0, len(removedDirMap)) // user is now only specifying 1 of the dirs, the other 1 should be returned as removed - dirMap, removedDirMap, err = getDataDirs(context, kv, "/rook/dir1", true, nodeName) + dirMap, err = getDataDirs(context, kv, "/rook/dir1", true, nodeName) assert.Nil(t, err) assert.Equal(t, 1, len(dirMap)) assert.Equal(t, 1, dirMap["/rook/dir1"]) - assert.Equal(t, 1, len(removedDirMap)) - assert.Equal(t, 23, removedDirMap["/tmp/mydir"]) // clear the dir map and simulate the scenario where an OSD has been created in the default dir kv.ClearStore(config.GetConfigStoreName(nodeName)) @@ -162,18 +154,10 @@ func TestGetDataDirs(t *testing.T) { // when an OSD has been created in the default dir, no dirs are specified, and no devices are specified, // the default dir should still be in use (it should not come back as removed!) - dirMap, removedDirMap, err = getDataDirs(context, kv, "", false, nodeName) + dirMap, err = getDataDirs(context, kv, "", false, nodeName) assert.Nil(t, err) assert.Equal(t, 1, len(dirMap)) assert.Equal(t, osdID, dirMap[context.ConfigDir]) - assert.Equal(t, 0, len(removedDirMap)) - - // if devices are specified (but no dirs), the existing osd in the default dir will not be preserved - dirMap, removedDirMap, err = getDataDirs(context, kv, "", true, nodeName) - assert.Nil(t, err) - assert.Equal(t, 0, len(dirMap)) - assert.Equal(t, 1, len(removedDirMap)) - assert.Equal(t, osdID, removedDirMap[context.ConfigDir]) } func TestAvailableDevices(t *testing.T) { @@ -274,41 +258,6 @@ NAME="sdb1" SIZE="30" TYPE="part" PKNAME="sdb"`, nil assert.Equal(t, -1, mapping.Entries["sdd"].Data) } -func TestGetRemovedDevices(t *testing.T) { - testGetRemovedDevicesHelper(t, &config.StoreConfig{StoreType: config.Bluestore}) - testGetRemovedDevicesHelper(t, &config.StoreConfig{StoreType: config.Filestore}) -} - -func testGetRemovedDevicesHelper(t *testing.T, storeConfig *config.StoreConfig) { - configDir, _ := ioutil.TempDir("", "") - defer os.RemoveAll(configDir) - os.MkdirAll(configDir, 0755) - nodeName := "node3391" - agent, _, _ := createTestAgent(t, "none", configDir, nodeName, storeConfig) - agent.devices[0].IsFilter = true - - // mock the pre-existence of osd 1 on device sdx - _, _, _ = mockPartitionSchemeEntry(t, 1, "sdx", &agent.storeConfig, agent.kv, nodeName) - - // get the removed devices for this configuration (note we said to use devices "none" above), - // it should be osd 1 on device sdx - scheme, mapping, err := getRemovedDevices(agent) - assert.Nil(t, err) - assert.Equal(t, 1, len(mapping.Entries)) - assert.Equal(t, 1, len(scheme.Entries)) - - // assert the scheme has an entry for osd 1 and its data partition is on sdx - schemeEntry := scheme.Entries[0] - assert.Equal(t, 1, schemeEntry.ID) - assert.Equal(t, "sdx", schemeEntry.Partitions[schemeEntry.GetDataPartitionType()].Device) - - // assert the removed device mapping has an entry for device sdx and it points to osd 1 - mappingEntry, ok := mapping.Entries["sdx"] - assert.True(t, ok) - assert.NotNil(t, mappingEntry) - assert.Equal(t, 1, mappingEntry.Data) -} - func TestGetVolumeGroupName(t *testing.T) { validLVPath := "/dev/vgName1/lvName2" invalidLVPath1 := "/dev//vgName2" diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index c24551acb..52ab2f053 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -35,7 +35,6 @@ import ( opspec "github.com/rook/rook/pkg/operator/ceph/spec" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" - "github.com/rook/rook/pkg/util/display" apps "k8s.io/api/apps/v1" batch "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" @@ -411,12 +410,6 @@ func (c *Cluster) startProvisioningOverNodes(config *provisionConfig) { } logger.Infof("start osds after provisioning is completed, if needed") c.completeProvision(config) - - // start the OSD pods, waiting for the provisioning to be completed - // handle the removed nodes and rebalance the PGs - logger.Infof("checking if any nodes were removed") - c.handleRemovedNodes(config) - } func (c *Cluster) runJob(job *batch.Job, nodeName string, config *provisionConfig, action string) bool { @@ -601,111 +594,6 @@ func (c *Cluster) startOSDDaemonsOnNode(nodeName string, config *provisionConfig } } -func (c *Cluster) handleRemovedNodes(config *provisionConfig) { - // find all removed nodes (if any) and start orchestration to remove them from the cluster - removedNodes, err := c.findRemovedNodes() - if err != nil { - config.addError("failed to find removed nodes: %+v", err) - return - } - logger.Infof("processing %d removed nodes", len(removedNodes)) - - for removedNode, osdDeployments := range removedNodes { - logger.Infof("processing removed node %s", removedNode) - if err := c.isSafeToRemoveNode(removedNode, osdDeployments); err != nil { - logger.Warningf("skipping the removal of node %s because it is not safe to do so: %+v", removedNode, err) - continue - } - - logger.Infof("removing node %s from the cluster with %d OSDs", removedNode, len(osdDeployments)) - - var nodeCrushName string - errorOnCurrentNode := false - for _, dp := range osdDeployments { - - logger.Infof("processing removed osd %s", dp.Name) - id := getIDFromDeployment(dp) - if id == unknownID { - config.addError("cannot remove unknown osd %s", dp.Name) - continue - } - - // on the first osd, get the crush name of the host - if nodeCrushName == "" { - nodeCrushName, err = client.GetCrushHostName(c.context, c.Namespace, id) - if err != nil { - config.addError("failed to get crush host name for osd.%d: %+v", id, err) - } - } - - if err := c.removeOSD(dp.Name, id); err != nil { - config.addError("failed to remove osd %d. %+v", id, err) - errorOnCurrentNode = true - continue - } - } - - if err := c.updateOSDStatus(removedNode, OrchestrationStatus{Status: OrchestrationStatusCompleted}); err != nil { - config.addError("failed to set orchestration starting status for removed node %s: %+v", removedNode, err) - } - - if errorOnCurrentNode { - logger.Warningf("done processing %d osd removals on node %s with an error removing the osds. skipping node cleanup", len(osdDeployments), removedNode) - } else { - logger.Infof("succeeded processing %d osd removals on node %s. starting cleanup job on the node.", len(osdDeployments), removedNode) - c.cleanupRemovedNode(config, removedNode, nodeCrushName) - } - } - logger.Infof("done processing removed nodes") -} - -func (c *Cluster) cleanupRemovedNode(config *provisionConfig, nodeName, crushName string) { - // update the orchestration status of this removed node to the starting state - if err := c.updateOSDStatus(nodeName, OrchestrationStatus{Status: OrchestrationStatusStarting}); err != nil { - config.addError("failed to set orchestration starting status for removed node %s: %+v", nodeName, err) - return - } - - // trigger orchestration on the removed node by telling it not to use any storage at all. note that the directories are still passed in - // so that the pod will be able to mount them and migrate data from them. - osdProps := osdProperties{ - crushHostname: nodeName, - devices: []rookalpha.Device{}, - selection: rookalpha.Selection{DeviceFilter: "none"}, - resources: v1.ResourceRequirements{}, - storeConfig: osdconfig.StoreConfig{}, - } - job, err := c.makeJob(osdProps, config) - if err != nil { - message := fmt.Sprintf("failed to create prepare job node %s: %v", nodeName, err) - config.addError(message) - status := OrchestrationStatus{Status: OrchestrationStatusCompleted, Message: message} - if err := c.updateOSDStatus(nodeName, status); err != nil { - config.addError("failed to update node %s status. %+v", nodeName, err) - } - return - } - - if !c.runJob(job, nodeName, config, "remove") { - status := OrchestrationStatus{Status: OrchestrationStatusCompleted, Message: fmt.Sprintf("failed to cleanup osd config on node %s", nodeName)} - if err := c.updateOSDStatus(nodeName, status); err != nil { - config.addError("failed to update node %s status. %+v", nodeName, err) - } - return - } - - logger.Infof("waiting for removal cleanup on node %s", nodeName) - c.completeProvisionSkipOSDStart(config) - logger.Infof("done waiting for removal cleanup on node %s", nodeName) - - // after the batch job is finished, clean up all the resources related to the node - if crushName != "" { - if err := c.cleanUpNodeResources(nodeName, crushName); err != nil { - config.addError("failed to cleanup node resources for %s", crushName) - } - } -} - // discover nodes which currently have osds scheduled on them. Return a mapping of // node names -> a list of osd deployments on the node func (c *Cluster) discoverStorageNodes() (map[string][]*apps.Deployment, error) { @@ -738,63 +626,6 @@ func (c *Cluster) discoverStorageNodes() (map[string][]*apps.Deployment, error) return discoveredNodes, nil } -func (c *Cluster) isSafeToRemoveNode(nodeName string, osdDeployments []*apps.Deployment) error { - if err := client.IsClusterCleanError(c.context, c.Namespace); err != nil { - // the cluster isn't clean, it's not safe to remove this node - return err - } - - // get the current used space on all OSDs in the cluster - currUsage, err := client.GetOSDUsage(c.context, c.Namespace) - if err != nil { - return err - } - - // sum up the total OSD used space for the node by summing the used space of each OSD on the node - nodeUsage := int64(0) - for _, osdDeployment := range osdDeployments { - id := getIDFromDeployment(osdDeployment) - if id == unknownID { - continue - } - - osdUsage := currUsage.ByID(id) - if osdUsage != nil { - osdKB, err := osdUsage.UsedKB.Int64() - if err != nil { - logger.Warningf("osd.%d has invalid usage %+v: %+v", id, osdUsage.UsedKB, err) - continue - } - - nodeUsage += osdKB * 1024 - } - } - - // check to see if there is sufficient space left in the cluster to absorb all the migrated data from the node to be removed - clusterUsage, err := client.Usage(c.context, c.Namespace) - if err != nil { - return err - } - clusterAvailableBytes, err := clusterUsage.Stats.TotalAvailBytes.Int64() - if err != nil { - return err - } - clusterTotalBytes, err := clusterUsage.Stats.TotalBytes.Int64() - if err != nil { - return err - } - - if (clusterAvailableBytes - nodeUsage) < int64((float64(clusterTotalBytes) * clusterAvailableSpaceReserve)) { - // the remaining available space in the cluster after the space that this node is using gets moved elsewhere - // would be less than the cluster available space reserve, it's not safe to remove this node - return errors.Errorf("insufficient available space in the cluster to remove node %s. node usage: %s, cluster available: %s", - nodeName, display.BytesToString(uint64(nodeUsage)), display.BytesToString(uint64(clusterAvailableBytes))) - } - - // looks safe to remove the node - return nil -} - func getIDFromDeployment(deployment *apps.Deployment) int { if idstr, ok := deployment.Labels[OsdIdLabelKey]; ok { id, err := strconv.Atoi(idstr) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 44df4a68e..42a025673 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -822,10 +822,7 @@ func (c *Cluster) provisionOSDContainer(osdProps osdProperties, copyBinariesMoun } volumeMounts = append(volumeMounts, v1.VolumeMount{Name: k8sutil.PathToVolumeName(dpath), MountPath: dpath}) } - - if !IsRemovingNode(osdProps.selection.DeviceFilter) { - envVars = append(envVars, dataDirectoriesEnvVar(strings.Join(dirPaths, ","))) - } + envVars = append(envVars, dataDirectoriesEnvVar(strings.Join(dirPaths, ","))) } // elevate to be privileged if it is going to mount devices or if running in a restricted environment such as openshift diff --git a/pkg/operator/ceph/cluster/osd/status.go b/pkg/operator/ceph/cluster/osd/status.go index 0b5eae72f..2d7f0f5cb 100644 --- a/pkg/operator/ceph/cluster/osd/status.go +++ b/pkg/operator/ceph/cluster/osd/status.go @@ -26,7 +26,6 @@ import ( "github.com/rook/rook/pkg/operator/ceph/config" "github.com/rook/rook/pkg/operator/k8sutil" "github.com/rook/rook/pkg/util" - apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -283,82 +282,3 @@ func (c *Cluster) handleStatusConfigMapStatus(nodeName string, config *provision } return false } - -func IsRemovingNode(devices string) bool { - return devices == "none" -} - -func (c *Cluster) findRemovedNodes() (map[string][]*apps.Deployment, error) { - removedNodes := map[string][]*apps.Deployment{} - - // first discover the storage nodes that are still running - discoveredNodes, err := c.discoverStorageNodes() - if err != nil { - return nil, errors.Wrapf(err, "aborting search for removed nodes. failed to discover storage nodes") - } - - // c.ValidStorage.Nodes currently in the cluster `c` is only the subset of the user-defined - // nodes which are currently valid, and we want a list which can include nodes that are cordoned - // for maintenance or have automatic Kubernetes well-known taints added. - k8sNodes, err := k8sutil.GetKubernetesNodesMatchingRookNodes(c.DesiredStorage.Nodes, c.context.Clientset) - if err != nil { - return nil, errors.Wrapf(err, "aborting search for removed nodes. failed to list nodes from Kubernetes") - } - nodeMap := map[string]v1.Node{} - for _, n := range k8sNodes { - hostname := n.Labels[v1.LabelHostname] - nodeMap[hostname] = n - } - - for existingNode, osdDeployments := range discoveredNodes { - var nodeRef *v1.Node - if n, ok := nodeMap[existingNode]; !ok { - nodeRef = nil - } else { - nodeRef = &n - } - reason := "" - if c.nodeRemovedByUser(existingNode, nodeRef) { - logger.Infof("adding node %s to the removed nodes list. %s", existingNode, reason) - removedNodes[existingNode] = osdDeployments - } - } - - return removedNodes, nil -} - -// sample of conditions where we cannot determine if the user wants to remove a node as an osd host: -// - if the node is not schedulable, it may be cordoned for maintenance -// - if the node is not ready, it could be down temporarily -func (c *Cluster) nodeRemovedByUser(nodeName string, k8sNode *v1.Node) bool { - if c.DesiredStorage.UseAllNodes == false { - // for maximum Ceph data safety, when useAllNodes == false, the *only* way to get Rook to - // remove a node is by explicit removal from the cluster resource - if !c.DesiredStorage.NodeWithNameExists(nodeName) { - logger.Debugf("node removed by user. node %s was removed from the cluster definition", nodeName) - return true - } - } - if c.DesiredStorage.UseAllNodes == true { - // node nil means that the node does not exist in or has been deleted from kubernetes - if k8sNode == nil { - logger.Debugf("node removed by user. node %s does not exist in Kubernetes", nodeName) - return true - } - - // without deleting a node from Kubernetes, taints and affinities are the provided - // method for users to remove nodes from the cluster when useAllNodes == true - ignoreWellKnownTaints := true - placeable, err := k8sutil.NodeMeetsPlacementTerms(*k8sNode, c.placement, ignoreWellKnownTaints) - if err != nil { - logger.Errorf("assuming node is not removed to err on the side of caution."+ - " failed to determine if node %s meets Rook's placement terms. %+v", nodeName, err) - return false - } - if !placeable { - logger.Debugf("node removed by user. node %s has had taints or affinities modified.", nodeName) - return true - } - } - return false -} From ef4d5aea880aeabd66f9269db2d7e3a37abd12d6 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Thu, 12 Dec 2019 00:05:46 -0700 Subject: [PATCH 2/2] ceph: document manual deletion of osds Removal of OSDs is a sensitive area. If you get it wrong you could have data loss. Now a help topic is provided for manual steps to remove OSDs when absolutely necessary. Signed-off-by: Travis Nielsen --- Documentation/ceph-osd-mgmt.md | 130 +++++++++++++++++++++++++++++++++ PendingReleaseNotes.md | 3 +- 2 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 Documentation/ceph-osd-mgmt.md diff --git a/Documentation/ceph-osd-mgmt.md b/Documentation/ceph-osd-mgmt.md new file mode 100644 index 000000000..2a7d19e04 --- /dev/null +++ b/Documentation/ceph-osd-mgmt.md @@ -0,0 +1,130 @@ +--- +title: Ceph OSD Management +weight: 10610 +indent: true +--- + +# Ceph OSD Management + +Ceph Object Storage Daemons (OSDs) are the heart and soul of the Ceph storage platform. +Each OSD manages a local device and together they provide the distributed storage. Rook will automate creation and management of OSDs to hide the complexity +based on the desired state in the CephCluster CR as much as possible. This guide will walk through some of the scenarios +to configure OSDs where more configuration may be required. + +## OSD Health + +The [rook-ceph-tools pod](./ceph-toolbox.md) provides a simple environment to run Ceph tools. The `ceph` commands +mentioned in this document should be run from the toolbox. + +Once the is created, connect to the pod to execute the `ceph` commands to analyze the health of the cluster, +in particular the OSDs and placement groups (PGs). Some common commands to analyze OSDs include: +``` +ceph status +ceph osd tree +ceph osd status +ceph osd df +ceph osd utilization +``` + +```console +kubectl -n rook-ceph exec -it $(kubectl -n rook-ceph get pod -l "app=rook-ceph-tools" -o jsonpath='{.items[0].metadata.name}') bash +``` + +## Add an OSD + +The [QuickStart Guide](ceph-quickstart.md) will provide the basic steps to create a cluster and start some OSDs. For more details on the OSD +settings also see the [Cluster CRD](ceph-cluster-crd.md) documentation. If you are not seeing OSDs created, see the [Ceph Troubleshooting Guide](ceph-common-issues.md). + +To add more OSDs, Rook will automatically watch for new nodes and devices being added to your cluster. +If they match the filters or other settings in the `storage` section of the cluster CR, the operator +will create new OSDs. + +## Add an OSD on a PVC + +In more dynamic environments where storage can be dynamically provisioned with a raw block storage provider, the OSDs can be backed +by PVCs. See the `storageClassDeviceSets` documentation in the [Cluster CRD](ceph-cluster-crd.md#storage-class-device-sets) topic. + +To add more OSDs, you can either increase the `count` of the OSDs in an existing device set or you can +add more device sets to the cluster CR. The operator will then automatically create new OSDs according +to the updated cluster CR. + +## Remove an OSD + +Removal of OSDs is intentionally not automated. Rook's charter is to keep your data safe, not to delete it. If you are +sure you need to remove OSDs, it can be done. We just want you to be in control of this action. + +To remove an OSD due to a failed disk or other re-configuration, consider the following to ensure the health of the data +through the removal process: +- Confirm you will have enough space on your cluster after removing your OSDs to properly handle the deletion +- Confirm the remaining OSDs and their placement groups (PGs) are healthy in order to handle the rebalancing of the data +- Do not remove too many OSDs at once +- Wait for rebalancing between removing multiple OSDs + +If all the PGs are `active+clean` and there are no warnings about being low on space, this means the data is fully replicated +and it is safe to proceed. If an OSD is failing, the PGs will not be perfectly clean and you will need to proceed anyway. + +### From the Toolbox + +1. Determine the OSD ID for the OSD to be removed. The osd pod may be in an error state such as `CrashLoopBackoff` or the `ceph` commands +in the toolbox may show which OSD is `down`. +2. Mark the OSD as `out` if not already marked as such by Ceph. This signals Ceph to start moving (backfilling) the data that was on that OSD to another OSD. + - `ceph osd out osd.` (for example if the OSD ID is 23 this would be `ceph osd out osd.23`) +3. Wait for the data to finish backfilling to other OSDs. + - `ceph status` will indicate the backfilling is done when all of the PGs are `active+clean`. +4. Remove the disk from the node. +5. Update your CephCluster CR such that the operator won't create an OSD on the device anymore. +Depending on your CR settings, you may need to remove the device from the list or update the device filter. +If you are using `useAllDevices: true`, no change to the CR is necessary. +6. Remove the OSD from the Ceph cluster + - `ceph osd purge --yes-i-really-mean-it` +7. Verify the OSD is removed from the node in the CRUSH map + - `ceph osd tree` + +### Remove the OSD Deployment + +The operator can automatically remove OSD deployments that are considered "safe-to-destroy" by Ceph. +After the steps above, the OSD will be considered safe to remove since the data has all been moved +to other OSDs. But this will only be done automatically by the operator if you have this setting in the cluster CR: +``` +removeOSDsIfOutAndSafeToRemove: true +``` + +8. Otherwise, you will need to delete the deployment directly: + - `kubectl delete deployment -n rook-ceph rook-ceph-osd-` + +### Delete the underlying data + +9. If you want to clean the device where the OSD was running, see in the instructions to +wipe a disk on the [Cleaning up a Cluster](ceph-teardown.md#delete-the-data-on-hosts) topic. + +## Replace an OSD + +To replace a disk that has failed: + +1. Run the steps in the previous section to [Remove an OSD](#remove-an-osd). +2. Replace the physical device and verify the new device is attached. +3. Check if your cluster CR will find the new device. If you are using `useAllDevices: true` you can skip this step. +If your cluster CR lists individual devices or uses a device filter you may need to update the CR. +4. The operator ideally will automatically create the new OSD within a few minutes of adding the new device or updating the CR. +If you don't see a new OSD automatically created, restart the operator (by deleting the operator pod) to trigger the OSD creation. +5. Verify if the OSD is created on the node by running `ceph osd tree` from the toolbox. + +Note that the OSD might have a different ID than the previous OSD that was replaced. + +## Remove an OSD from a PVC + +If you have installed your OSDs on top of PVCs and you desire to reduce the size of your cluster by removing OSDs: + +1. Shrink the number of OSDs in the `storageClassDeviceSet` in the CephCluster CR. + - `kubectl -n rook-ceph edit cephcluster rook-ceph` + - Reduce the `count` of the OSDs to the desired number. Rook will not take any action to automatically remove the extra OSD(s), but will effectively stop managing the orphaned OSD. +2. Identify the orphaned PVC that belongs to the orphaned OSD. + - The orphaned PVC will have the highest index among the PVCs for the device set. + - `kubectl -n rook-ceph get pvc -l ceph.rook.io/DeviceSet=` + - For example if the device set is named `set1` and the `count` was reduced from `3` to `2`, the orphaned PVC would have the index `2` and might be named `set1-2-data-vbwcf` +3. Identify the orphaned OSD. + - The OSD assigned to the PVC can be found in the labels on the PVC + - `kubectl -n rook-ceph get pod -l ceph.rook.io/pvc= -o yaml | grep ceph-osd-id` + - For example, this might return: `ceph-osd-id: "0"` +4. Now proceed with the steps in the section above to [Remove an OSD](#remove-an-osd) for the orphaned OSD ID. +5. If desired, delete the orphaned PVC after the OSD is removed. diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 697acdc68..46a01727b 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -26,7 +26,8 @@ - When the operator is upgraded, the mgr and OSDs (not running on PVC) won't be restarted if the Rook binary version changes - Rook is now able to create and manage Ceph clients [client crd](Documentation/ceph-client-crd.html). - OSDs: - - Rook will no longer automatically remove OSDs if nodes are removed from the cluster CR to avoid the risk of destroying OSDs unintentionally + - Rook will no longer automatically remove OSDs if nodes are removed from the cluster CR to avoid the risk of destroying OSDs unintentionally. +To remove OSDs manually, see the new doc on [OSD Management](Documentation/ceph-osd-mgmt.md) - Ceph OSD's admin socket is now placed under Ceph's default system location `/run/ceph`. - The on-host log directory for OSDs was set incorrectly to `//log`; fix this to be `/log/`, the same as other daemons.