osddaemon: run vgchange -ay during init #3771

galexrt · 2019-09-04T17:48:20Z

Description of your changes:

Running vgchange -ay will enable all VGs on the host. This will remove
the need to have the lvm* services running on the host. In such a case
with a host, this would lead to all OSDs on that host to be unavailable
and the OSD Pods crashlooping as they can't find their VG and / or LV.

Signed-off-by: Alexander Trost galexrt@googlemail.com

Which issue is resolved by this Pull Request:
Resolves #3640

Checklist:

Reviewed the developer guide on Submitting a Pull Request
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.
Pending release notes updated with breaking and/or notable changes, if necessary.
Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
Code generation (make codegen) has been run to update object specifications, if necessary.
Comments have been added or updated based on the standards set in CONTRIBUTING.md
Add the flag for skipping the CI if this PR does not require a build. See here for more details.

[test ceph min]

BlaineEXE · 2019-09-04T17:53:40Z

pkg/daemon/ceph/osd/daemon.go

 	}

+	// Activate all VGs on the server
+	context.Executor.ExecuteCommand(false, "", "/sbin/vgchange", "-ay")


I think we should consider using just vgchange and letting exec.Cmd search the PATH for where vgchange actually lives so we can support environments where this might be a different path.

Will implement to search through PATH, though to note this will be executed inside the Rook Ceph container image used in the OSD init container.

That is [probably] not necessary. If ExecuteCommand uses exec.Cmd, then Go will automatically search PATH. https://godoc.org/os/exec#LookPath

travisn · 2019-09-04T17:55:14Z

pkg/daemon/ceph/osd/daemon.go

@@ -55,10 +55,11 @@ func StartOSD(context *clusterd.Context, osdType, osdID, osdUUID string, pvcBack
 		}

 		context.Executor.ExecuteCommand(false, "", "/sbin/vgchange", "-an")
-
-		context.Executor.ExecuteCommand(false, "", "/sbin/vgchange", "-ay")


#3755 is working on a change here for the specific vg for the osd being started. Let's keep this line as is, and wait for #3755 to change it.

The vgname related change is part of this PR - #3779

travisn · 2019-09-04T17:55:36Z

pkg/daemon/ceph/osd/daemon.go

 	}

+	// Activate all VGs on the server


Let's add this in an else statement, related to the change coming in #3755

travisn · 2019-09-24T15:20:31Z

@galexrt Related changes may have already covered the fix? Or is there something else after the rebase?

leseb

Is this change still relevant?

galexrt · 2019-10-12T20:17:16Z

Yes, this PR is still relevant as the VGs are only activated when the OSDs are backed by PVCs, see

rook/pkg/daemon/ceph/osd/daemon.go

Lines 61 to 75 in 24fc94c

    
           if pvcBackedOSD { 
        
           	volumeGroupName, err = getVolumeGroupName(lvPath) 
        
           	if err != nil { 
        
           		return fmt.Errorf("error fetching volume group name for OSD %s. %+v", osdID, err) 
        
           	} 
        
           	go handleTerminate(context, lvPath, volumeGroupName) 
        
           	if err := context.Executor.ExecuteCommand(false, "", "/sbin/vgchange", "-an", volumeGroupName); err != nil { 
        
           		return fmt.Errorf("failed to deactivate volume group for lv %+v. Error: %+v", lvPath, err) 
        
           	} 
        
           	if err := context.Executor.ExecuteCommand(false, "", "/sbin/vgchange", "-ay", volumeGroupName); err != nil { 
        
           		return fmt.Errorf("failed to activate volume group for lv %+v. Error: %+v", lvPath, err) 
        
           	} 
        
           }

.

I have updated the PR to enable the VGs all the time. Does anything speak against that?

pkg/daemon/ceph/osd/daemon.go

galexrt · 2019-10-18T09:36:32Z

@leseb @travisn @BlaineEXE PTAL

leseb · 2019-10-18T11:36:07Z

pkg/daemon/ceph/osd/daemon.go

-			return fmt.Errorf("failed to activate volume group for lv %+v. Error: %+v", lvPath, err)
-		}
+	if err := context.Executor.ExecuteCommand(false, "", "vgchange", "-ay", volumeGroupName); err != nil {
+		return fmt.Errorf("failed to activate volume group for lv %+v. %+v", lvPath, err)


Isn't lvPath a string? If so please use %q instead. Thanks.

Fixed it now.

travisn · 2019-10-18T16:01:50Z

pkg/daemon/ceph/osd/daemon.go

-		if err := context.Executor.ExecuteCommand(false, "", "vgchange", "-ay", volumeGroupName); err != nil {
-			return fmt.Errorf("failed to activate volume group for lv %+v. Error: %+v", lvPath, err)
-		}
+	if err := context.Executor.ExecuteCommand(false, "", "vgchange", "-ay", volumeGroupName); err != nil {


The volumeGroupName won't be set if it's not a PVC, if you could take another look per discussion

Running `vgchange -ay` will enable all VGs on the host. This will remove the need to have the lvm* services running on the host. In such a case with a host, this would lead to all OSDs on that host to be unavailable and the OSD Pods crashlooping as they can't find their VG and / or LV. Signed-off-by: Alexander Trost <galexrt@googlemail.com>

galexrt · 2019-10-18T18:02:19Z

The volumeGroupName (LvPath) is not available for non-PVC backed OSDs.

So it'll be either just enabling all VGs or finding a working way to get the LVPath from the non-PVC backed OSDs as well.

If anyone knows a good way to get the LVPath for existing OSDs, please let me know. I'll ask in the huddle on Tuesday.

mykaul · 2019-10-19T12:12:30Z

I'm not sure it's a good idea to blindly enable all VGs on a host. If anything, we should have a filter of some sort. There might be VGs we don't own.

leseb · 2019-12-12T16:15:30Z

c-v should incorporate the necessary de-activate call soon, so we should wait for that instead of running this in Rook. @galexrt can we close this? Thanks.

leseb · 2020-02-12T10:26:34Z

Closing this due to inactivity. Feel free to re-open. Thanks. Also, we are moving away from LVM and the current implementation is stable so we probably can skip this.

andrewrynhard · 2020-02-12T15:04:24Z

@leseb Any idea on the timeline for moving away from LVM? I'd like to deploy rook but this is the one thing holding me up.

leseb · 2020-02-12T15:58:05Z

@leseb Any idea on the timeline for moving away from LVM? I'd like to deploy rook but this is the one thing holding me up.

@andrewrynhard If you're using PVC, it'll be in 1.3, if not it's a bit difficult because a lot of things need to be implemented in raw mode.
There is this #4768 to bring a minimal support of raw when not running on PVC. However, it means you will only be able to bootstrap OSD in a full disk or a partition. Basically, Bluestore block/wal/db will be on that device, so no dedicated device, no more than one osd on one device and no encryption. That might work for most of the people out there though.

galexrt added ceph main ceph tag ceph-osd labels Sep 4, 2019

galexrt requested review from travisn and BlaineEXE September 4, 2019 17:48

BlaineEXE requested changes Sep 4, 2019

View reviewed changes

travisn requested changes Sep 4, 2019

View reviewed changes

leseb reviewed Sep 25, 2019

View reviewed changes

galexrt force-pushed the activate_vg_osds branch from c347db7 to f03e7af Compare October 12, 2019 20:16

leseb requested changes Oct 14, 2019

View reviewed changes

pkg/daemon/ceph/osd/daemon.go Outdated Show resolved Hide resolved

pkg/daemon/ceph/osd/daemon.go Outdated Show resolved Hide resolved

galexrt force-pushed the activate_vg_osds branch from f03e7af to 7f4ac30 Compare October 18, 2019 09:35

leseb reviewed Oct 18, 2019

View reviewed changes

galexrt force-pushed the activate_vg_osds branch from 7f4ac30 to 73ec92f Compare October 18, 2019 12:33

leseb approved these changes Oct 18, 2019

View reviewed changes

travisn approved these changes Oct 18, 2019

View reviewed changes

travisn added the backport-release-1.1 label Oct 18, 2019

leseb requested a review from BlaineEXE October 18, 2019 14:51

BlaineEXE approved these changes Oct 18, 2019

View reviewed changes

travisn requested changes Oct 18, 2019

View reviewed changes

galexrt force-pushed the activate_vg_osds branch from 73ec92f to d353544 Compare October 18, 2019 16:07

leseb removed the backport-release-1.1 label Dec 12, 2019

leseb closed this Feb 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

osddaemon: run vgchange -ay during init #3771

osddaemon: run vgchange -ay during init #3771

galexrt commented Sep 4, 2019 •

edited by leseb

BlaineEXE Sep 4, 2019

galexrt Sep 4, 2019

BlaineEXE Sep 6, 2019 •

edited

travisn Sep 4, 2019

sp98 Sep 5, 2019

travisn Sep 4, 2019

travisn commented Sep 24, 2019

leseb left a comment

galexrt commented Oct 12, 2019

galexrt commented Oct 18, 2019

leseb Oct 18, 2019

galexrt Oct 18, 2019

travisn Oct 18, 2019

galexrt commented Oct 18, 2019 •

edited

mykaul commented Oct 19, 2019

leseb commented Dec 12, 2019

leseb commented Feb 12, 2020

andrewrynhard commented Feb 12, 2020

leseb commented Feb 12, 2020 •

edited

osddaemon: run vgchange -ay during init #3771

osddaemon: run vgchange -ay during init #3771

Conversation

galexrt commented Sep 4, 2019 • edited by leseb

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlaineEXE Sep 6, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisn commented Sep 24, 2019

leseb left a comment

Choose a reason for hiding this comment

galexrt commented Oct 12, 2019

galexrt commented Oct 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

galexrt commented Oct 18, 2019 • edited

mykaul commented Oct 19, 2019

leseb commented Dec 12, 2019

leseb commented Feb 12, 2020

andrewrynhard commented Feb 12, 2020

leseb commented Feb 12, 2020 • edited

galexrt commented Sep 4, 2019 •

edited by leseb

BlaineEXE Sep 6, 2019 •

edited

galexrt commented Oct 18, 2019 •

edited

leseb commented Feb 12, 2020 •

edited