clean cephFS omap details #237

yati1998 · 2024-02-06T12:50:03Z

Remove the omap object for the given subvolume.

yati1998 · 2024-02-06T14:50:03Z

https://github.com/rook/kubectl-rook-ceph/actions/runs/7799941151/job/21272736952?pr=237#step:12:20

did not load config file, using default settings.
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 Errors while parsing config file!
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 can't open ceph.conf: (2) No such file or directory
unable to get monitor info from DNS SRV with service name: ceph-mon
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 failed for service _ceph-mon._tcp
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 monclient: get_monmap_and_config cannot identify monitors to contact
failed to fetch mon config (--no-mon-config to skip)```

yati1998 · 2024-02-06T14:59:16Z

cc: @travisn @Madhu-1

travisn · 2024-02-06T18:51:19Z

https://github.com/rook/kubectl-rook-ceph/actions/runs/7799941151/job/21272736952?pr=237#step:12:20

did not load config file, using default settings.
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 Errors while parsing config file!
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 can't open ceph.conf: (2) No such file or directory
unable to get monitor info from DNS SRV with service name: ceph-mon
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 failed for service _ceph-mon._tcp
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 monclient: get_monmap_and_config cannot identify monitors to contact
failed to fetch mon config (--no-mon-config to skip)```

The plugin may not understand that it needs to append the mon and cluster commands to the rados command. Looks like you need to add the rados command here.

yati1998 · 2024-02-07T04:41:27Z

https://github.com/rook/kubectl-rook-ceph/actions/runs/7799941151/job/21272736952?pr=237#step:12:20
did not load config file, using default settings.
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 Errors while parsing config file!
2024-02-06T10:08:53.016+0000 7fa11474ae00 -1 can't open ceph.conf: (2) No such file or directory
unable to get monitor info from DNS SRV with service name: ceph-mon
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 failed for service _ceph-mon._tcp
2024-02-06T10:08:53.018+0000 7fa11474ae00 -1 monclient: get_monmap_and_config cannot identify monitors to contact
failed to fetch mon config (--no-mon-config to skip)```
The plugin may not understand that it needs to append the mon and cluster commands to the rados command. Looks like you need to add the rados command here.

That works @travisn . Thanks.

subhamkrai

Also, please update commit/PR title

.github/workflows/go-test.yaml

pkg/filesystem/subvolume.go

travisn · 2024-02-15T23:02:58Z

The CI is failing:

+ kubectl rook-ceph --operator-namespace test-operator -n test-cluster subvolume delete myfs test-subvol group-a
Info: subvolume "test-subvol" deleted
Error: error getting omap keys myfs-metadata/test.subvolume: (2) No such file or directory
. failed to run command. command terminated with exit code 1%!(EXTRA string=failed to list omapval)
Error: failed to get omap details: "error getting omap keys myfs-metadata/test.subvolume: (2) No such file or directory\n. failed to run command. command terminated with exit code 1"

yati1998 · 2024-02-16T05:54:53Z

The CI is failing:

+ kubectl rook-ceph --operator-namespace test-operator -n test-cluster subvolume delete myfs test-subvol group-a
Info: subvolume "test-subvol" deleted
Error: error getting omap keys myfs-metadata/test.subvolume: (2) No such file or directory
. failed to run command. command terminated with exit code 1%!(EXTRA string=failed to list omapval)
Error: failed to get omap details: "error getting omap keys myfs-metadata/test.subvolume: (2) No such file or directory\n. failed to run command. command terminated with exit code 1"

Yes, this error is expected. Updated the test to include error in output.

.github/workflows/go-test.yaml

Madhu-1

i think we should start testing all these functionalities in the CI to ensure it works, Please add test cases for it

Madhu-1 · 2024-02-13T14:47:36Z

docs/subvolume.md

+# Error: error getting omap keys ocs-storagecluster-cephfilesystem-metadata/test.subvolume:
+# (2) No such file or directory
+# . failed to run command. command terminated with exit code 1%!(EXTRA string=failed to list omapval)


why do we have this error?

Incase the subvolume is created from backend and it's stale, there will be no omap created for it. In that case if we try to delete the omap, it will through this error.

let's document the happy case output.

pkg/filesystem/subvolume.go

subhamkrai · 2024-02-19T12:33:43Z

docs/subvolume.md

+# Error: error getting omap keys ocs-storagecluster-cephfilesystem-metadata/test.subvolume:
+# (2) No such file or directory
+# . failed to run command. command terminated with exit code 1%!(EXTRA string=failed to list omapval)


let's document the happy case output.

pkg/filesystem/subvolume.go

yati1998 · 2024-02-26T08:25:18Z

@travisn @Madhu-1 I have addressed all the comments above, please do have a look once you are free.
@Madhu-1 about adding the test for stale rados omap, we need to create sc with retain feature and a few other steps need to be followed, I am working on it to check how we can add that here in ci.

nixpanic · 2024-03-18T12:48:24Z

pkg/filesystem/subvolume_test.go

+	tests := []struct {
+		name string
+		want string
+	}{


Maybe consider adding negative tests as well? An empty string, a string with only no, one or two - in there probably are problematic and cause issues. It may be unlikely that these get passed to the function, but you should be prepared for weird issues in a cleanup process, some values may be unexpectedly missing or somehow got corrupted.

I'm not sure if the added tests provide the needed or expected results to whatever calls getOmapVal(). But this at least confirms that there are no panics happening.

My feedback has been addressed. After CI is passing LGTM.

nixpanic

Looks good to me. If there are any missed corner-cases where cleanup isn't working as expected, this can always be improved later.

subhamkrai

@yati1998 please rebase your PR to get latest ci fixes.

yati1998 · 2024-03-19T14:29:49Z

@Madhu-1 waiting for your review here. Please feel free to add your comments.

Madhu-1

LGTM,
small nits

pkg/filesystem/subvolume.go

Madhu-1 · 2024-03-19T14:43:45Z

pkg/filesystem/subvolume.go

+	if err != nil || poolName == "" {
+		logging.Fatal(fmt.Errorf("pool name not found: %q", err))


same here as well

yes, it's not required.

pkg/filesystem/subvolume.go

tests/github-action-helper.sh

subhamkrai

Also, since this clean cephfs OMAP please update the commit title and PR title to mention cephFS OMAP

docs/subvolume.md

pkg/filesystem/subvolume.go

this commit deletes the omap presence of the stale subvolume Signed-off-by: yati1998 <ypadia@redhat.com>

yati1998 force-pushed the omapcleanup branch from 4814d36 to dfb8a62 Compare February 7, 2024 10:36

yati1998 marked this pull request as ready for review February 9, 2024 05:03

yati1998 force-pushed the omapcleanup branch 4 times, most recently from 63b1880 to 013ee98 Compare February 12, 2024 08:17

subhamkrai reviewed Feb 13, 2024

View reviewed changes

.github/workflows/go-test.yaml Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

yati1998 force-pushed the omapcleanup branch 3 times, most recently from c52d3b6 to d881f18 Compare February 15, 2024 07:45

yati1998 force-pushed the omapcleanup branch from d881f18 to 8197c19 Compare February 16, 2024 05:53

yati1998 requested review from subhamkrai, travisn and Madhu-1 February 16, 2024 05:55

travisn reviewed Feb 16, 2024

View reviewed changes

.github/workflows/go-test.yaml Outdated Show resolved Hide resolved

Madhu-1 requested changes Feb 16, 2024

View reviewed changes

Madhu-1 reviewed Feb 16, 2024

View reviewed changes

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

yati1998 force-pushed the omapcleanup branch 2 times, most recently from 978ee47 to 07b74fd Compare February 19, 2024 11:55

subhamkrai requested changes Feb 19, 2024

View reviewed changes

yati1998 force-pushed the omapcleanup branch 2 times, most recently from 6ffcf46 to b9336f1 Compare February 26, 2024 05:49

yati1998 mentioned this pull request Feb 26, 2024

add new command to delete stale omap objects #251

Open

yati1998 force-pushed the omapcleanup branch 3 times, most recently from 40198f5 to 40a2196 Compare March 18, 2024 11:08

nixpanic reviewed Mar 18, 2024

View reviewed changes

yati1998 force-pushed the omapcleanup branch from 40a2196 to 8bd4dd4 Compare March 18, 2024 13:43

yati1998 force-pushed the omapcleanup branch from 8bd4dd4 to a41ebba Compare March 18, 2024 14:06

yati1998 requested a review from nixpanic March 18, 2024 14:07

nixpanic approved these changes Mar 18, 2024

View reviewed changes

subhamkrai reviewed Mar 19, 2024

View reviewed changes

yati1998 force-pushed the omapcleanup branch from a41ebba to 6ac167f Compare March 19, 2024 10:12

Madhu-1 reviewed Mar 19, 2024

View reviewed changes

yati1998 force-pushed the omapcleanup branch from 6ac167f to 2ef2bb8 Compare March 19, 2024 15:34

yati1998 requested a review from Madhu-1 March 19, 2024 16:15

Madhu-1 approved these changes Mar 19, 2024

View reviewed changes

yati1998 requested a review from subhamkrai March 20, 2024 03:51

subhamkrai requested changes Mar 20, 2024

View reviewed changes

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

tests/github-action-helper.sh Outdated Show resolved Hide resolved

subhamkrai requested changes Mar 20, 2024

View reviewed changes

docs/subvolume.md Outdated Show resolved Hide resolved

docs/subvolume.md Outdated Show resolved Hide resolved

docs/subvolume.md Outdated Show resolved Hide resolved

yati1998 force-pushed the omapcleanup branch from 2ef2bb8 to 937a382 Compare March 20, 2024 09:42

yati1998 changed the title ~~clean omap details~~ clean cephFS omap details Mar 20, 2024

yati1998 force-pushed the omapcleanup branch from 937a382 to 5f116aa Compare March 20, 2024 09:57

yati1998 requested a review from subhamkrai March 20, 2024 09:57

subhamkrai requested changes Mar 20, 2024

View reviewed changes

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

pkg/filesystem/subvolume.go Outdated Show resolved Hide resolved

csi: clean cephFS omap details

d86b70d

this commit deletes the omap presence of the stale subvolume Signed-off-by: yati1998 <ypadia@redhat.com>

yati1998 force-pushed the omapcleanup branch from 5f116aa to d86b70d Compare March 20, 2024 10:23

subhamkrai approved these changes Mar 20, 2024

View reviewed changes

subhamkrai merged commit 2bcbb3e into rook:master Mar 20, 2024
6 checks passed

travisn mentioned this pull request Apr 1, 2024

rados command should be available in the plugin #268

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clean cephFS omap details #237

clean cephFS omap details #237

yati1998 commented Feb 6, 2024

yati1998 commented Feb 6, 2024 •

edited

Loading

yati1998 commented Feb 6, 2024

travisn commented Feb 6, 2024

yati1998 commented Feb 7, 2024

subhamkrai left a comment

travisn commented Feb 15, 2024

yati1998 commented Feb 16, 2024

Madhu-1 left a comment

Madhu-1 Feb 13, 2024

yati1998 Feb 16, 2024

subhamkrai Feb 19, 2024

subhamkrai Feb 19, 2024

yati1998 commented Feb 26, 2024

nixpanic Mar 18, 2024

nixpanic Mar 18, 2024 •

edited

Loading

nixpanic left a comment

subhamkrai left a comment

yati1998 commented Mar 19, 2024

Madhu-1 left a comment

Madhu-1 Mar 19, 2024

yati1998 Mar 19, 2024

subhamkrai left a comment

		if err != nil \|\| poolName == "" {
		logging.Fatal(fmt.Errorf("pool name not found: %q", err))

clean cephFS omap details #237

clean cephFS omap details #237

Conversation

yati1998 commented Feb 6, 2024

yati1998 commented Feb 6, 2024 • edited Loading

yati1998 commented Feb 6, 2024

travisn commented Feb 6, 2024

yati1998 commented Feb 7, 2024

subhamkrai left a comment

Choose a reason for hiding this comment

travisn commented Feb 15, 2024

yati1998 commented Feb 16, 2024

Madhu-1 left a comment

Choose a reason for hiding this comment

Madhu-1 Feb 13, 2024

Choose a reason for hiding this comment

yati1998 Feb 16, 2024

Choose a reason for hiding this comment

subhamkrai Feb 19, 2024

Choose a reason for hiding this comment

subhamkrai Feb 19, 2024

Choose a reason for hiding this comment

yati1998 commented Feb 26, 2024

nixpanic Mar 18, 2024

Choose a reason for hiding this comment

nixpanic Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

nixpanic left a comment

Choose a reason for hiding this comment

subhamkrai left a comment

Choose a reason for hiding this comment

yati1998 commented Mar 19, 2024

Madhu-1 left a comment

Choose a reason for hiding this comment

Madhu-1 Mar 19, 2024

Choose a reason for hiding this comment

yati1998 Mar 19, 2024

Choose a reason for hiding this comment

subhamkrai left a comment

Choose a reason for hiding this comment

yati1998 commented Feb 6, 2024 •

edited

Loading

nixpanic Mar 18, 2024 •

edited

Loading