From 051125ff3faed9d059037c7cc66cbee0e193ed37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 29 Apr 2022 10:37:07 +0200 Subject: [PATCH 1/2] core: tidy up go deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tidy our dependencies. Signed-off-by: Sébastien Han From 6ef491d9fc95f533fe5b14268b79f09c3946016b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 29 Apr 2022 10:29:14 +0200 Subject: [PATCH 2/2] osd: close the encrypted disk after cleanup is done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once we are done cleaning up the content of the encrypted osd, let's close the main LUKS device. This avoids having dm devices on the system after the cleanup. Closes: https://github.com/rook/rook/issues/10181 Signed-off-by: Sébastien Han --- .github/workflows/canary-integration-test.yml | 22 ++++ pkg/daemon/ceph/cleanup/disk.go | 22 +++- pkg/daemon/ceph/osd/encryption.go | 2 +- pkg/daemon/ceph/osd/encryption_test.go | 2 +- pkg/daemon/ceph/osd/volume.go | 117 ++++++++++-------- pkg/operator/ceph/cluster/osd/osd.go | 1 + tests/scripts/github-action-helper.sh | 10 ++ 7 files changed, 118 insertions(+), 58 deletions(-) diff --git a/.github/workflows/canary-integration-test.yml b/.github/workflows/canary-integration-test.yml index 54b337156..882c393cb 100644 --- a/.github/workflows/canary-integration-test.yml +++ b/.github/workflows/canary-integration-test.yml @@ -207,6 +207,17 @@ jobs: - name: check-ownerreferences run: tests/scripts/github-action-helper.sh check_ownerreferences + - name: teardown cluster with cleanup policy + run: | + kubectl -n rook-ceph patch cephcluster rook-ceph --type merge -p '{"spec":{"cleanupPolicy":{"confirmation":"yes-really-destroy-data"}}}' + kubectl -n rook-ceph delete cephcluster rook-ceph + kubectl -n rook-ceph logs deploy/rook-ceph-operator + tests/scripts/github-action-helper.sh wait_for_cleanup_pod + lsblk + BLOCK=$(sudo lsblk --paths|awk '/14G/ {print $1}'| head -1) + sudo head --bytes=60 ${BLOCK}1 + sudo head --bytes=60 ${BLOCK}2 + - name: collect common logs if: always() uses: ./.github/workflows/collect-logs @@ -363,6 +374,17 @@ jobs: kubectl -n rook-ceph delete deploy/rook-ceph-osd-0 tests/scripts/github-action-helper.sh wait_for_ceph_to_be_ready osd 2 + - name: teardown cluster with cleanup policy + run: | + kubectl -n rook-ceph patch cephcluster rook-ceph --type merge -p '{"spec":{"cleanupPolicy":{"confirmation":"yes-really-destroy-data"}}}' + kubectl -n rook-ceph delete cephcluster rook-ceph + kubectl -n rook-ceph logs deploy/rook-ceph-operator + tests/scripts/github-action-helper.sh wait_for_cleanup_pod + BLOCK=$(sudo lsblk --paths|awk '/14G/ {print $1}'| head -1) + sudo head --bytes=60 ${BLOCK}1 + sudo head --bytes=60 ${BLOCK}2 + sudo lsblk + - name: collect common logs if: always() uses: ./.github/workflows/collect-logs diff --git a/pkg/daemon/ceph/cleanup/disk.go b/pkg/daemon/ceph/cleanup/disk.go index 18116b7c6..59c6b4173 100644 --- a/pkg/daemon/ceph/cleanup/disk.go +++ b/pkg/daemon/ceph/cleanup/disk.go @@ -87,7 +87,7 @@ func (s *DiskSanitizer) sanitizeRawDisk(osdRawList []oposd.OSDInfo) { wg.Add(1) // Put each sanitize in a go routine to speed things up - go s.executeSanitizeCommand(osd.BlockPath, &wg) + go s.executeSanitizeCommand(osd, &wg) } wg.Wait() @@ -115,7 +115,7 @@ func (s *DiskSanitizer) sanitizeLVMDisk(osdLVMList []oposd.OSDInfo) { // // purge remaining LVM2 metadata from PV for _, pv := range pvs { wg2.Add(1) - go s.executeSanitizeCommand(pv, &wg2) + go s.executeSanitizeCommand(oposd.OSDInfo{BlockPath: pv}, &wg2) } wg2.Wait() } @@ -175,15 +175,25 @@ func (s *DiskSanitizer) buildShredArgs(disk string) []string { return shredArgs } -func (s *DiskSanitizer) executeSanitizeCommand(disk string, wg *sync.WaitGroup) { +func (s *DiskSanitizer) executeSanitizeCommand(osdInfo oposd.OSDInfo, wg *sync.WaitGroup) { // On return, notify the WaitGroup that we’re done defer wg.Done() - output, err := s.context.Executor.ExecuteCommandWithCombinedOutput(shredUtility, s.buildShredArgs(disk)...) + output, err := s.context.Executor.ExecuteCommandWithCombinedOutput(shredUtility, s.buildShredArgs(osdInfo.BlockPath)...) if err != nil { - logger.Errorf("failed to sanitize osd disk %q. %s. %v", disk, output, err) + logger.Errorf("failed to sanitize osd disk %q. %s. %v", osdInfo.BlockPath, output, err) } logger.Infof("%s\n", output) - logger.Infof("successfully sanitized osd disk %q", disk) + logger.Infof("successfully sanitized osd disk %q", osdInfo.BlockPath) + + // If the device is encrypted let's close it after sanitizing its content + if osdInfo.Encrypted { + err := osd.CloseEncryptedDevice(s.context, osdInfo.BlockPath) + if err != nil { + logger.Errorf("failed to close encrypted osd disk %q. %v", osdInfo.BlockPath, err) + } else { + logger.Infof("successfully closed encrypted osd disk %q", osdInfo.BlockPath) + } + } } diff --git a/pkg/daemon/ceph/osd/encryption.go b/pkg/daemon/ceph/osd/encryption.go index 513ebd1d7..283aa46c7 100644 --- a/pkg/daemon/ceph/osd/encryption.go +++ b/pkg/daemon/ceph/osd/encryption.go @@ -39,7 +39,7 @@ var ( luksLabelCephFSID = regexp.MustCompile("ceph_fsid=(.*)") ) -func closeEncryptedDevice(context *clusterd.Context, dmName string) error { +func CloseEncryptedDevice(context *clusterd.Context, dmName string) error { args := []string{"--verbose", "luksClose", dmName} cryptsetupOut, err := context.Executor.ExecuteCommandWithCombinedOutput(cryptsetupBinary, args...) if err != nil { diff --git a/pkg/daemon/ceph/osd/encryption_test.go b/pkg/daemon/ceph/osd/encryption_test.go index 359d13085..b6dc88d8a 100644 --- a/pkg/daemon/ceph/osd/encryption_test.go +++ b/pkg/daemon/ceph/osd/encryption_test.go @@ -79,7 +79,7 @@ func TestCloseEncryptedDevice(t *testing.T) { } context := &clusterd.Context{Executor: executor} - err := closeEncryptedDevice(context, "/dev/mapper/ceph-43e9efed-0676-4731-b75a-a4c42ece1bb1-xvdbr-block-dmcrypt") + err := CloseEncryptedDevice(context, "/dev/mapper/ceph-43e9efed-0676-4731-b75a-a4c42ece1bb1-xvdbr-block-dmcrypt") assert.NoError(t, err) } diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go index 52fe3e91f..ff5c18cf9 100644 --- a/pkg/daemon/ceph/osd/volume.go +++ b/pkg/daemon/ceph/osd/volume.go @@ -938,7 +938,12 @@ func readCVLogContent(cvLogFilePath string) string { return string(b) } -// GetCephVolumeRawOSDs list OSD prepared with raw mode +// GetCephVolumeRawOSDs list OSD prepared with raw mode. +// Sometimes this function called against a device, sometimes it's not. For instance, in the cleanup +// scenario, we don't pass any block because we are looking for all the OSDs present on the machine. +// On the other hand, the PVC scenario always uses the PVC block as a block to check whether the +// disk is an OSD or not. +// The same goes for "metadataBlock" and "walBlock" they are only used in the prepare job. func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.ClusterInfo, cephfsid, block, metadataBlock, walBlock string, lvBackedPV, skipDeviceClass bool) ([]oposd.OSDInfo, error) { // lv can be a block device if raw mode is used cvMode := "raw" @@ -960,55 +965,57 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster // // isCephEncryptedBlock() returns false if the disk is not a LUKS device with: // Device /dev/sdc is not a valid LUKS device. - if isCephEncryptedBlock(context, cephfsid, block) { - childDevice, err := sys.ListDevicesChild(context.Executor, block) - if err != nil { - return nil, err - } - - var encryptedBlock string - // Find the encrypted block as part of the output - // Most of the time we get 2 devices, the parent and the child but we don't want to guess - // which one is the child by looking at the index, instead the iterate over the list - // Our encrypted device **always** have "-dmcrypt" in the name. - for _, device := range childDevice { - if strings.Contains(device, "-dmcrypt") { - encryptedBlock = device - break + if block != "" { + if isCephEncryptedBlock(context, cephfsid, block) { + childDevice, err := sys.ListDevicesChild(context.Executor, block) + if err != nil { + return nil, err } - } - if encryptedBlock == "" { - // The encrypted block is not opened, this is an extreme corner case - // The OSD deployment has been removed manually AND the node rebooted - // So we need to re-open the block to re-hydrate the OSDInfo. - // - // Handling this case would mean, writing the encryption key on a temporary file, then call - // luksOpen to open the encrypted block and then call ceph-volume to list against the opened - // encrypted block. - // We don't implement this, yet and return an error. - return nil, errors.Errorf("failed to find the encrypted block device for %q, not opened?", block) - } - // If we have one child device, it should be the encrypted block but still verifying it - isDeviceEncrypted, err := sys.IsDeviceEncrypted(context.Executor, encryptedBlock) - if err != nil { - return nil, err - } + var encryptedBlock string + // Find the encrypted block as part of the output + // Most of the time we get 2 devices, the parent and the child but we don't want to guess + // which one is the child by looking at the index, instead the iterate over the list + // Our encrypted device **always** have "-dmcrypt" in the name. + for _, device := range childDevice { + if strings.Contains(device, "-dmcrypt") { + encryptedBlock = device + break + } + } + if encryptedBlock == "" { + // The encrypted block is not opened, this is an extreme corner case + // The OSD deployment has been removed manually AND the node rebooted + // So we need to re-open the block to re-hydrate the OSDInfo. + // + // Handling this case would mean, writing the encryption key on a temporary file, then call + // luksOpen to open the encrypted block and then call ceph-volume to list against the opened + // encrypted block. + // We don't implement this, yet and return an error. + return nil, errors.Errorf("failed to find the encrypted block device for %q, not opened?", block) + } - // All good, now we set the right disk for ceph-volume to list against - // The ceph-volume will look like: - // [root@bde85e6b23ec /]# ceph-volume raw list /dev/mapper/ocs-deviceset-thin-1-data-0hmfgp-block-dmcrypt - // { - // "4": { - // "ceph_fsid": "fea59c09-2d35-4096-bc46-edb0fd39ab86", - // "device": "/dev/mapper/ocs-deviceset-thin-1-data-0hmfgp-block-dmcrypt", - // "osd_id": 4, - // "osd_uuid": "fcff2062-e653-4d42-84e5-e8de639bed4b", - // "type": "bluestore" - // } - // } - if isDeviceEncrypted { - block = encryptedBlock + // If we have one child device, it should be the encrypted block but still verifying it + isDeviceEncrypted, err := sys.IsDeviceEncrypted(context.Executor, encryptedBlock) + if err != nil { + return nil, err + } + + // All good, now we set the right disk for ceph-volume to list against + // The ceph-volume will look like: + // [root@bde85e6b23ec /]# ceph-volume raw list /dev/mapper/ocs-deviceset-thin-1-data-0hmfgp-block-dmcrypt + // { + // "4": { + // "ceph_fsid": "fea59c09-2d35-4096-bc46-edb0fd39ab86", + // "device": "/dev/mapper/ocs-deviceset-thin-1-data-0hmfgp-block-dmcrypt", + // "osd_id": 4, + // "osd_uuid": "fcff2062-e653-4d42-84e5-e8de639bed4b", + // "type": "bluestore" + // } + // } + if isDeviceEncrypted { + block = encryptedBlock + } } } @@ -1077,6 +1084,7 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster LVBackedPV: lvBackedPV, CVMode: cvMode, Store: "bluestore", + Encrypted: strings.Contains(blockPath, "-dmcrypt"), } if !skipDeviceClass { @@ -1089,6 +1097,15 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster } // If this is an encrypted OSD + // Always rely on the env variable and NOT what we find as a block name, this function is + // called by: + // * the prepare job, which pass the env variable for encryption or not + // * the cleanup job which lists **all** devices and cleans them up + // They do different things, in the case of the prepare job we want to close the encrypted + // device because the device is going to be detached from the pod and re-attached to the OSD + // pod + // For the cleanup pod we don't want to close the encrypted block since it will sanitize it + // first and then close it if os.Getenv(oposd.CephVolumeEncryptedKeyEnvVarName) != "" { // If label and subsystem are not set on the encrypted block let's set it // They will be set if the OSD deployment has been removed manually and the prepare job @@ -1104,7 +1121,7 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster } // Close encrypted device - err = closeEncryptedDevice(context, block) + err = CloseEncryptedDevice(context, block) if err != nil { return nil, errors.Wrapf(err, "failed to close encrypted device %q for osd %d", block, osdID) } @@ -1112,7 +1129,7 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster // If there is a metadata block if metadataBlock != "" { // Close encrypted device - err = closeEncryptedDevice(context, metadataBlock) + err = CloseEncryptedDevice(context, metadataBlock) if err != nil { return nil, errors.Wrapf(err, "failed to close encrypted db device %q for osd %d", metadataBlock, osdID) } @@ -1121,7 +1138,7 @@ func GetCephVolumeRawOSDs(context *clusterd.Context, clusterInfo *client.Cluster // If there is a wal block if walBlock != "" { // Close encrypted device - err = closeEncryptedDevice(context, walBlock) + err = CloseEncryptedDevice(context, walBlock) if err != nil { return nil, errors.Wrapf(err, "failed to close encrypted wal device %q for osd %d", walBlock, osdID) } diff --git a/pkg/operator/ceph/cluster/osd/osd.go b/pkg/operator/ceph/cluster/osd/osd.go index 25ba0da9c..8f8889f5f 100644 --- a/pkg/operator/ceph/cluster/osd/osd.go +++ b/pkg/operator/ceph/cluster/osd/osd.go @@ -108,6 +108,7 @@ type OSDInfo struct { Store string `json:"store"` // Ensure the OSD daemon has affinity with the same topology from the OSD prepare pod TopologyAffinity string `json:"topologyAffinity"` + Encrypted bool `json:"encrypted"` } // OrchestrationStatus represents the status of an OSD orchestration diff --git a/tests/scripts/github-action-helper.sh b/tests/scripts/github-action-helper.sh index bf54f1352..d043f40d7 100755 --- a/tests/scripts/github-action-helper.sh +++ b/tests/scripts/github-action-helper.sh @@ -250,6 +250,16 @@ function wait_for_prepare_pod() { kubectl -n rook-ceph describe deployment/rook-ceph-osd-0 || true } +function wait_for_cleanup_pod() { + timeout 180 bash <