From d00323da5c12253b2401f22043d8d80a380b22c6 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Thu, 21 Sep 2023 19:18:20 -0300 Subject: [PATCH 1/6] Add the WAL and DB devices for the disk-add command Signed-off-by: Luciano Lo Giudice --- microceph/api/disks.go | 2 +- microceph/api/types/disks.go | 8 +++++--- microceph/ceph/osd.go | 12 ++++++++++-- microceph/cmd/microceph/disk_add.go | 12 ++++++++++++ 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/microceph/api/disks.go b/microceph/api/disks.go index 86e32751..51c4392d 100644 --- a/microceph/api/disks.go +++ b/microceph/api/disks.go @@ -56,7 +56,7 @@ func cmdDisksPost(s *state.State, r *http.Request) response.Response { mu.Lock() defer mu.Unlock() - err = ceph.AddOSD(s, req.Path, req.Wipe, req.Encrypt) + err = ceph.AddOSD(s, req.Path, req.Wipe, req.Encrypt, req.WALDev, req.DBDev) if err != nil { return response.SmartError(err) } diff --git a/microceph/api/types/disks.go b/microceph/api/types/disks.go index c9afbdce..c84e05a1 100644 --- a/microceph/api/types/disks.go +++ b/microceph/api/types/disks.go @@ -3,9 +3,11 @@ package types // DisksPost hold a path and a flag for enabling device wiping type DisksPost struct { - Path string `json:"path" yaml:"path"` - Wipe bool `json:"wipe" yaml:"wipe"` - Encrypt bool `json:"encrypt" yaml:"encrypt"` + Path string `json:"path" yaml:"path"` + Wipe bool `json:"wipe" yaml:"wipe"` + Encrypt bool `json:"encrypt" yaml:"encrypt"` + WALDev *string `json:"waldev" yaml:"waldev"` + DBDev *string `json:"dbdev" yaml:"dbdev"` } // DisksDelete holds an OSD number and a flag for forcing the removal diff --git a/microceph/ceph/osd.go b/microceph/ceph/osd.go index 7b93d6e0..1b810f3c 100644 --- a/microceph/ceph/osd.go +++ b/microceph/ceph/osd.go @@ -265,7 +265,7 @@ func updateFailureDomain(s *state.State) error { } // AddOSD adds an OSD to the cluster, given a device path and a flag for wiping -func AddOSD(s *state.State, path string, wipe bool, encrypt bool) error { +func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string, dbdev *string) error { logger.Debugf("Adding OSD %s", path) revert := revert.New() @@ -416,7 +416,15 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool) error { } // Bootstrap OSD. - _, err = processExec.RunCommand("ceph-osd", "--mkfs", "--no-mon-config", "-i", fmt.Sprintf("%d", nr)) + args := []string{"--mkfs", "--no-mon-config", "-i", fmt.Sprintf("%d", nr)} + if waldev != nil { + args = append(args, *waldev) + } + if dbdev != nil { + args = append(args, *dbdev) + } + + _, err = processExec.RunCommand("ceph-osd", args...) if err != nil { return fmt.Errorf("Failed to bootstrap OSD: %w", err) } diff --git a/microceph/cmd/microceph/disk_add.go b/microceph/cmd/microceph/disk_add.go index 016df83d..53b4e0ce 100644 --- a/microceph/cmd/microceph/disk_add.go +++ b/microceph/cmd/microceph/disk_add.go @@ -16,6 +16,8 @@ type cmdDiskAdd struct { flagWipe bool flagEncrypt bool + walDevice string + dbDevice string } func (c *cmdDiskAdd) Command() *cobra.Command { @@ -27,6 +29,8 @@ func (c *cmdDiskAdd) Command() *cobra.Command { cmd.PersistentFlags().BoolVar(&c.flagWipe, "wipe", false, "Wipe the disk prior to use") cmd.PersistentFlags().BoolVar(&c.flagEncrypt, "encrypt", false, "Encrypt the disk prior to use") + cmd.PersistentFlags().StringVar(&c.walDevice, "wal-device", "", "The device used for WAL") + cmd.PersistentFlags().StringVar(&c.dbDevice, "db-device", "", "The device used for the DB") return cmd } @@ -52,6 +56,14 @@ func (c *cmdDiskAdd) Run(cmd *cobra.Command, args []string) error { Encrypt: c.flagEncrypt, } + if c.walDevice != "" { + req.WALDev = &c.walDevice + } + + if c.dbDevice != "" { + req.DBDev = &c.dbDevice + } + err = client.AddDisk(context.Background(), cli, req) if err != nil { return err From 2cf7ddf086baf427500e2192b65c11578682da47 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Thu, 21 Sep 2023 20:01:42 -0300 Subject: [PATCH 2/6] Add the parameter names Signed-off-by: Luciano Lo Giudice --- microceph/ceph/osd.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/microceph/ceph/osd.go b/microceph/ceph/osd.go index 1b810f3c..abb8dbfc 100644 --- a/microceph/ceph/osd.go +++ b/microceph/ceph/osd.go @@ -418,10 +418,10 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string // Bootstrap OSD. args := []string{"--mkfs", "--no-mon-config", "-i", fmt.Sprintf("%d", nr)} if waldev != nil { - args = append(args, *waldev) + args = append(args, []string{"--bluestore-block-wal-path", *waldev}...) } if dbdev != nil { - args = append(args, *dbdev) + args = append(args, []string{"--bluestore-block-db-path", *dbdev}...) } _, err = processExec.RunCommand("ceph-osd", args...) From c9ae181b1b72a728520e8732dd9c18f22f4fa864 Mon Sep 17 00:00:00 2001 From: Peter Sabaini Date: Fri, 22 Sep 2023 11:35:12 +0200 Subject: [PATCH 3/6] Fix: failure domain unit tests Signed-off-by: Peter Sabaini --- microceph/ceph/osd_test.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/microceph/ceph/osd_test.go b/microceph/ceph/osd_test.go index 00f1f6bb..423f34e4 100644 --- a/microceph/ceph/osd_test.go +++ b/microceph/ceph/osd_test.go @@ -113,20 +113,18 @@ func (s *osdSuite) SetupTest() { // TestSwitchHostFailureDomain tests the switchFailureDomain function func (s *osdSuite) TestSwitchHostFailureDomain() { r := mocks.NewRunner(s.T()) - // list and create two crush rules - addCrushRuleLsExpectations(r) - addCrushRuleCreateExpectations(r) - addCrushRuleCreateExpectations(r) - // dump crush rules + + // dump crush rules to resolve names addCrushRuleDumpExpectations(r) // set default crush rule addSetDefaultRuleExpectations(r) - // list and dump crush rule + // list to check if crush rule exists addCrushRuleLsExpectations(r) + // dump crush rules to resolve names addCrushRuleDumpExpectations(r) - // list crush rule json + // list pools addCrushRuleLsJsonExpectations(r) - // set osd pool + // set pool crush rule addOsdPoolSetExpectations(r) processExec = r @@ -149,20 +147,18 @@ func (s *osdSuite) TestUpdateFailureDomain() { } r := mocks.NewRunner(s.T()) - // list and create two crush rules - addCrushRuleLsExpectations(r) - addCrushRuleCreateExpectations(r) - addCrushRuleCreateExpectations(r) - // dump crush rules + + // dump crush rules to resolve names addCrushRuleDumpExpectations(r) // set default crush rule addSetDefaultRuleExpectations(r) - // list and dump crush rule + // list to check if crush rule exists addCrushRuleLsExpectations(r) + // dump crush rules to resolve names addCrushRuleDumpExpectations(r) - // list crush rule json + // list pools addCrushRuleLsJsonExpectations(r) - // set osd pool + // set pool crush rule addOsdPoolSetExpectations(r) processExec = r From 369643598bf042e8eb38bb0b4063b3f6769b5abd Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 25 Sep 2023 18:20:20 -0300 Subject: [PATCH 4/6] Factor out common functionality This commit moves the process of wiping, encrypting and symlinking devices so that it can be reused for the data, WAL and DB devices. Signed-off-by: Luciano Lo Giudice --- microceph/api/disks.go | 13 ++- microceph/api/types/disks.go | 20 +++-- microceph/ceph/osd.go | 129 +++++++++++++++------------- microceph/cmd/microceph/disk_add.go | 8 ++ 4 files changed, 103 insertions(+), 67 deletions(-) diff --git a/microceph/api/disks.go b/microceph/api/disks.go index 51c4392d..5e951666 100644 --- a/microceph/api/disks.go +++ b/microceph/api/disks.go @@ -47,6 +47,8 @@ func cmdDisksGet(s *state.State, r *http.Request) response.Response { func cmdDisksPost(s *state.State, r *http.Request) response.Response { var req types.DisksPost + var wal *types.DiskParameter + var db *types.DiskParameter logger.Debugf("cmdDisksPost: %v", req) err := json.NewDecoder(r.Body).Decode(&req) @@ -56,7 +58,16 @@ func cmdDisksPost(s *state.State, r *http.Request) response.Response { mu.Lock() defer mu.Unlock() - err = ceph.AddOSD(s, req.Path, req.Wipe, req.Encrypt, req.WALDev, req.DBDev) + + data := types.DiskParameter{req.Path, req.Wipe, req.Encrypt} + if req.WALDev != nil { + wal = &types.DiskParameter{*req.WALDev, req.WALWipe, req.WALEncrypt} + } + if req.DBDev != nil { + db = &types.DiskParameter{*req.DBDev, req.DBWipe, req.DBEncrypt} + } + + err = ceph.AddOSD(s, data, wal, db) if err != nil { return response.SmartError(err) } diff --git a/microceph/api/types/disks.go b/microceph/api/types/disks.go index c84e05a1..dac857e8 100644 --- a/microceph/api/types/disks.go +++ b/microceph/api/types/disks.go @@ -3,11 +3,15 @@ package types // DisksPost hold a path and a flag for enabling device wiping type DisksPost struct { - Path string `json:"path" yaml:"path"` - Wipe bool `json:"wipe" yaml:"wipe"` - Encrypt bool `json:"encrypt" yaml:"encrypt"` - WALDev *string `json:"waldev" yaml:"waldev"` - DBDev *string `json:"dbdev" yaml:"dbdev"` + Path string `json:"path" yaml:"path"` + Wipe bool `json:"wipe" yaml:"wipe"` + Encrypt bool `json:"encrypt" yaml:"encrypt"` + WALDev *string `json:"waldev" yaml:"waldev"` + WALWipe bool `json:"walwipe" yaml:"walwipe"` + WALEncrypt bool `json:"walencrypt" yaml:"walencrypt"` + DBDev *string `json:"dbdev" yaml:"dbdev"` + DBWipe bool `json:"dbwipe" yaml:"dbwipe"` + DBEncrypt bool `json:"dbencrypt" yaml:"dbencrypt"` } // DisksDelete holds an OSD number and a flag for forcing the removal @@ -27,3 +31,9 @@ type Disk struct { Path string `json:"path" yaml:"path"` Location string `json:"location" yaml:"location"` } + +type DiskParameter struct { + Path string + Encrypt bool + Wipe bool +} diff --git a/microceph/ceph/osd.go b/microceph/ceph/osd.go index abb8dbfc..a905df4b 100644 --- a/microceph/ceph/osd.go +++ b/microceph/ceph/osd.go @@ -80,12 +80,34 @@ func nextOSD(s *state.State) (int64, error) { } } +func prepareDisk(disk *types.DiskParameter, suffix string, osdPath string, osdID int64) error { + if disk.Wipe { + err := timeoutWipe(disk.Path) + if err != nil { + return fmt.Errorf("Failed to wipe device %s: %w", disk.Path, err) + } + } + if disk.Encrypt { + err := checkEncryptSupport() + if err != nil { + return fmt.Errorf("Encryption unsupported on this machine: %w", err) + } + path, err := setupEncryptedOSD(disk.Path, osdPath, osdID, suffix) + if err != nil { + return fmt.Errorf("Failed to encrypt device %s: %w", disk.Path, err) + } + disk.Path = path + } + return os.Symlink(disk.Path, filepath.Join(osdPath, "block"+suffix)) +} + // setupEncryptedOSD sets up an encrypted OSD on the given disk. // -// Takes a path to the disk device as well as the osd data path and the osd id. +// Takes a path to the disk device as well as the OSD data path, the OSD id and +// a suffix (to differentiate invocations between data, WAL and DB devices). // Returns the path to the encrypted device and an error if any. -func setupEncryptedOSD(devicePath string, osdDataPath string, osdID int64) (string, error) { - if err := os.Symlink(devicePath, filepath.Join(osdDataPath, "unencrypted")); err != nil { +func setupEncryptedOSD(devicePath string, osdDataPath string, osdID int64, suffix string) (string, error) { + if err := os.Symlink(devicePath, filepath.Join(osdDataPath, "unencrypted"+suffix)); err != nil { return "", fmt.Errorf("Failed to add unencrypted block symlink: %w", err) } @@ -96,7 +118,7 @@ func setupEncryptedOSD(devicePath string, osdDataPath string, osdID int64) (stri } // Store key in ceph key value store - if err = storeKey(key, osdID); err != nil { + if err = storeKey(key, osdID, suffix); err != nil { return "", fmt.Errorf("Key store error: %w", err) } @@ -106,7 +128,7 @@ func setupEncryptedOSD(devicePath string, osdDataPath string, osdID int64) (stri } // Open the encrypted device - encryptedDevicePath, err := openEncryptedDevice(devicePath, osdID, key) + encryptedDevicePath, err := openEncryptedDevice(devicePath, osdID, key, suffix) if err != nil { return "", fmt.Errorf("Failed to open: %w", err) } @@ -151,9 +173,9 @@ func encryptDevice(path string, key []byte) error { } // Store the key in the ceph key value store, under a name that derives from the osd id. -func storeKey(key []byte, osdID int64) error { +func storeKey(key []byte, osdID int64, suffix string) error { // Run the ceph config-key set command - _, err := processExec.RunCommand("ceph", "config-key", "set", fmt.Sprintf("microceph:osd.%d/key", osdID), string(key)) + _, err := processExec.RunCommand("ceph", "config-key", "set", fmt.Sprintf("microceph:osd%s.%d/key", suffix, osdID), string(key)) if err != nil { return fmt.Errorf("Failed to store key: %w", err) } @@ -161,7 +183,7 @@ func storeKey(key []byte, osdID int64) error { } // Open the encrypted device and return its path. -func openEncryptedDevice(path string, osdID int64, key []byte) (string, error) { +func openEncryptedDevice(path string, osdID int64, key []byte, suffix string) (string, error) { // Run the cryptsetup open command, expect key on stdin cmd := exec.Command( "cryptsetup", @@ -169,7 +191,7 @@ func openEncryptedDevice(path string, osdID int64, key []byte) (string, error) { "--key-file", "-", "luksOpen", path, - fmt.Sprintf("luksosd-%d", osdID), + fmt.Sprintf("luksosd%s-%d", suffix, osdID), ) stdin, err := cmd.StdinPipe() if err != nil { @@ -187,7 +209,7 @@ NOTE: OSD Encryption requires a snapd >= 2.59.1 Verify your version of snapd by running "snap version" `, path, err, out) } - return fmt.Sprintf("/dev/mapper/luksosd-%d", osdID), nil + return fmt.Sprintf("/dev/mapper/luksosd%s-%d", suffix, osdID), nil } // checkEncryptSupport checks if the kernel supports encryption. @@ -265,24 +287,18 @@ func updateFailureDomain(s *state.State) error { } // AddOSD adds an OSD to the cluster, given a device path and a flag for wiping -func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string, dbdev *string) error { - logger.Debugf("Adding OSD %s", path) +func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, db *types.DiskParameter) error { + logger.Debugf("Adding OSD %s", data.Path) revert := revert.New() defer revert.Fail() // Validate the path. - if !shared.IsBlockdevPath(path) { - return fmt.Errorf("Invalid disk path: %s", path) - } - // Check if we need to support encryption - if encrypt { - if err := checkEncryptSupport(); err != nil { - return fmt.Errorf("Encryption unsupported on this machine: %w", err) - } + if !shared.IsBlockdevPath(data.Path) { + return fmt.Errorf("Invalid disk path: %s", data.Path) } - _, _, major, minor, _, _, err := shared.GetFileStat(path) + _, _, major, minor, _, _, err := shared.GetFileStat(data.Path) if err != nil { return fmt.Errorf("Invalid disk path: %w", err) } @@ -302,11 +318,11 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string // check if candidate exists if shared.PathExists(candidate) && !shared.IsDir(candidate) { - path = candidate + data.Path = candidate } else { candidate = fmt.Sprintf("/dev/disk/by-path/%s", disk.DevicePath) if shared.PathExists(candidate) && !shared.IsDir(candidate) { - path = candidate + data.Path = candidate } } @@ -319,11 +335,11 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string if part.Device == dev { candidate := fmt.Sprintf("/dev/disk/by-id/%s-part%d", disk.DeviceID, part.Partition) if shared.PathExists(candidate) { - path = candidate + data.Path = candidate } else { candidate = fmt.Sprintf("/dev/disk/by-path/%s-part%d", disk.DevicePath, part.Partition) if shared.PathExists(candidate) { - path = candidate + data.Path = candidate } } @@ -337,24 +353,16 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string // Fallthrough. We didn't find a /dev/disk path for this device, use the original path. } - // Wipe the block device if requested. - if wipe { - err = timeoutWipe(path) - if err != nil { - return fmt.Errorf("Failed to wipe the device: %w", err) - } - } - // Get a OSD number. nr, err := nextOSD(s) if err != nil { return fmt.Errorf("Failed to find next OSD number: %w", err) } - logger.Debugf("nextOSD number is %d for disk %s", nr, path) + logger.Debugf("nextOSD number is %d for disk %s", nr, data.Path) // Record the disk. err = s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { - _, err := database.CreateDisk(ctx, tx, database.Disk{Member: s.Name(), Path: path, OSD: int(nr)}) + _, err := database.CreateDisk(ctx, tx, database.Disk{Member: s.Name(), Path: data.Path, OSD: int(nr)}) if err != nil { return fmt.Errorf("Failed to record disk: %w", err) } @@ -370,42 +378,33 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string dataPath := filepath.Join(os.Getenv("SNAP_COMMON"), "data") osdDataPath := filepath.Join(dataPath, "osd", fmt.Sprintf("ceph-%d", nr)) + // Keep the old path in case it changes after encrypting. + oldPath := data.Path + + // Create directory. + err = os.MkdirAll(osdDataPath, 0700) + if err != nil { + return fmt.Errorf("Failed to bootstrap monitor: %w", err) + } + + // Wipe and/or encrypt the disk if needed. + err = prepareDisk(&data, "", osdDataPath, nr) + // if we fail later, make sure we free up the record revert.Add(func() { os.RemoveAll(osdDataPath) s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { - database.DeleteDisk(ctx, tx, s.Name(), path) + database.DeleteDisk(ctx, tx, s.Name(), oldPath) return nil }) }) - // Create directory. - err = os.MkdirAll(osdDataPath, 0700) - if err != nil { - return fmt.Errorf("Failed to bootstrap monitor: %w", err) - } - // Generate keyring. err = genAuth(filepath.Join(osdDataPath, "keyring"), fmt.Sprintf("osd.%d", nr), []string{"mgr", "allow profile osd"}, []string{"mon", "allow profile osd"}, []string{"osd", "allow *"}) if err != nil { return fmt.Errorf("Failed to generate OSD keyring: %w", err) } - var blockPath string - if encrypt { - blockPath, err = setupEncryptedOSD(path, osdDataPath, nr) - if err != nil { - return err - } - } else { - blockPath = path - } - - // Setup device symlink. - if err = os.Symlink(blockPath, filepath.Join(osdDataPath, "block")); err != nil { - return fmt.Errorf("Failed to add block symlink: %w", err) - } - // Generate OSD uuid. fsid := uuid.NewRandom().String() @@ -417,11 +416,19 @@ func AddOSD(s *state.State, path string, wipe bool, encrypt bool, waldev *string // Bootstrap OSD. args := []string{"--mkfs", "--no-mon-config", "-i", fmt.Sprintf("%d", nr)} - if waldev != nil { - args = append(args, []string{"--bluestore-block-wal-path", *waldev}...) + if wal != nil { + err = prepareDisk(wal, ".wal", osdDataPath, nr) + if err != nil { + return fmt.Errorf("Failed to set up WAL device: %w", err) + } + args = append(args, []string{"--bluestore-block-wal-path", wal.Path}...) } - if dbdev != nil { - args = append(args, []string{"--bluestore-block-db-path", *dbdev}...) + if db != nil { + err = prepareDisk(db, ".db", osdDataPath, nr) + if err != nil { + return fmt.Errorf("Failed to set up DB device: %w", err) + } + args = append(args, []string{"--bluestore-block-db-path", db.Path}...) } _, err = processExec.RunCommand("ceph-osd", args...) diff --git a/microceph/cmd/microceph/disk_add.go b/microceph/cmd/microceph/disk_add.go index 53b4e0ce..34176b97 100644 --- a/microceph/cmd/microceph/disk_add.go +++ b/microceph/cmd/microceph/disk_add.go @@ -17,7 +17,11 @@ type cmdDiskAdd struct { flagWipe bool flagEncrypt bool walDevice string + walEncrypt bool + walWipe bool dbDevice string + dbEncrypt bool + dbWipe bool } func (c *cmdDiskAdd) Command() *cobra.Command { @@ -30,7 +34,11 @@ func (c *cmdDiskAdd) Command() *cobra.Command { cmd.PersistentFlags().BoolVar(&c.flagWipe, "wipe", false, "Wipe the disk prior to use") cmd.PersistentFlags().BoolVar(&c.flagEncrypt, "encrypt", false, "Encrypt the disk prior to use") cmd.PersistentFlags().StringVar(&c.walDevice, "wal-device", "", "The device used for WAL") + cmd.PersistentFlags().BoolVar(&c.walWipe, "wal-wipe", false, "Wipe the WAL device prior to use") + cmd.PersistentFlags().BoolVar(&c.walEncrypt, "wal-encrypt", false, "Encrypt the WAL device prior to use") cmd.PersistentFlags().StringVar(&c.dbDevice, "db-device", "", "The device used for the DB") + cmd.PersistentFlags().BoolVar(&c.dbWipe, "db-wipe", false, "Wipe the DB device prior to use") + cmd.PersistentFlags().BoolVar(&c.dbEncrypt, "db-encrypt", false, "Encrypt the DB device prior to use") return cmd } From 3cfab9862d05c922d3bde343a6bed25c3488753c Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Mon, 25 Sep 2023 19:55:47 -0300 Subject: [PATCH 5/6] Fill out missing members and check error Signed-off-by: Luciano Lo Giudice --- microceph/ceph/osd.go | 3 +++ microceph/cmd/microceph/disk_add.go | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/microceph/ceph/osd.go b/microceph/ceph/osd.go index a905df4b..b5e0cfd6 100644 --- a/microceph/ceph/osd.go +++ b/microceph/ceph/osd.go @@ -389,6 +389,9 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, // Wipe and/or encrypt the disk if needed. err = prepareDisk(&data, "", osdDataPath, nr) + if err != nil { + return fmt.Errorf("Failed to prepare data device: %w", err) + } // if we fail later, make sure we free up the record revert.Add(func() { diff --git a/microceph/cmd/microceph/disk_add.go b/microceph/cmd/microceph/disk_add.go index 34176b97..02b335e6 100644 --- a/microceph/cmd/microceph/disk_add.go +++ b/microceph/cmd/microceph/disk_add.go @@ -66,10 +66,14 @@ func (c *cmdDiskAdd) Run(cmd *cobra.Command, args []string) error { if c.walDevice != "" { req.WALDev = &c.walDevice + req.WALWipe = c.walWipe + req.WALEncrypt = c.walEncrypt } if c.dbDevice != "" { req.DBDev = &c.dbDevice + req.DBWipe = c.dbWipe + req.DBEncrypt = c.dbEncrypt } err = client.AddDisk(context.Background(), cli, req) From 61cab699eacc18a12c6653d282314f11cc693357 Mon Sep 17 00:00:00 2001 From: Luciano Lo Giudice Date: Wed, 27 Sep 2023 13:37:40 -0300 Subject: [PATCH 6/6] Cleanup code and improve error messages. This commit does the following: - Reorder struct variables and move up cleanup. - Fixes OSD cleanup. - Skips symlinking WAL and DB devices since Ceph does it for us. - Properly order script arguments. - Factor out stable disk path management. Signed-off-by: Luciano Lo Giudice --- microceph/api/disks.go | 7 ++-- microceph/ceph/osd.go | 93 +++++++++++++++++++++++++----------------- 2 files changed, 59 insertions(+), 41 deletions(-) diff --git a/microceph/api/disks.go b/microceph/api/disks.go index 5e951666..5f47792b 100644 --- a/microceph/api/disks.go +++ b/microceph/api/disks.go @@ -49,6 +49,7 @@ func cmdDisksPost(s *state.State, r *http.Request) response.Response { var req types.DisksPost var wal *types.DiskParameter var db *types.DiskParameter + var data types.DiskParameter logger.Debugf("cmdDisksPost: %v", req) err := json.NewDecoder(r.Body).Decode(&req) @@ -59,12 +60,12 @@ func cmdDisksPost(s *state.State, r *http.Request) response.Response { mu.Lock() defer mu.Unlock() - data := types.DiskParameter{req.Path, req.Wipe, req.Encrypt} + data = types.DiskParameter{req.Path, req.Encrypt, req.Wipe} if req.WALDev != nil { - wal = &types.DiskParameter{*req.WALDev, req.WALWipe, req.WALEncrypt} + wal = &types.DiskParameter{*req.WALDev, req.WALEncrypt, req.WALWipe} } if req.DBDev != nil { - db = &types.DiskParameter{*req.DBDev, req.DBWipe, req.DBEncrypt} + db = &types.DiskParameter{*req.DBDev, req.DBEncrypt, req.DBWipe} } err = ceph.AddOSD(s, data, wal, db) diff --git a/microceph/ceph/osd.go b/microceph/ceph/osd.go index b5e0cfd6..8e56e001 100644 --- a/microceph/ceph/osd.go +++ b/microceph/ceph/osd.go @@ -17,6 +17,7 @@ import ( "syscall" "time" + "github.com/canonical/lxd/shared/api" "github.com/canonical/lxd/shared/logger" "github.com/canonical/microceph/microceph/common" @@ -98,7 +99,12 @@ func prepareDisk(disk *types.DiskParameter, suffix string, osdPath string, osdID } disk.Path = path } - return os.Symlink(disk.Path, filepath.Join(osdPath, "block"+suffix)) + // Only the data device needs to be symlinked (suffix != ""). + // Other devices (WAL and DB) are automatically handled by Ceph itself. + if suffix != "" { + return nil + } + return os.Symlink(disk.Path, filepath.Join(osdPath, "block")) } // setupEncryptedOSD sets up an encrypted OSD on the given disk. @@ -286,31 +292,19 @@ func updateFailureDomain(s *state.State) error { return nil } -// AddOSD adds an OSD to the cluster, given a device path and a flag for wiping -func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, db *types.DiskParameter) error { - logger.Debugf("Adding OSD %s", data.Path) - - revert := revert.New() - defer revert.Fail() - +func setStablePath(storage *api.ResourcesStorage, param *types.DiskParameter) error { // Validate the path. - if !shared.IsBlockdevPath(data.Path) { - return fmt.Errorf("Invalid disk path: %s", data.Path) + if !shared.IsBlockdevPath(param.Path) { + return fmt.Errorf("Invalid disk path: %s", param.Path) } - _, _, major, minor, _, _, err := shared.GetFileStat(data.Path) + _, _, major, minor, _, _, err := shared.GetFileStat(param.Path) if err != nil { return fmt.Errorf("Invalid disk path: %w", err) } dev := fmt.Sprintf("%d:%d", major, minor) - // Lookup a stable path for it. - storage, err := resources.GetStorage() - if err != nil { - return fmt.Errorf("Unable to list system disks: %w", err) - } - for _, disk := range storage.Disks { // Check if full disk. if disk.Device == dev { @@ -318,11 +312,11 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, // check if candidate exists if shared.PathExists(candidate) && !shared.IsDir(candidate) { - data.Path = candidate + param.Path = candidate } else { candidate = fmt.Sprintf("/dev/disk/by-path/%s", disk.DevicePath) if shared.PathExists(candidate) && !shared.IsDir(candidate) { - data.Path = candidate + param.Path = candidate } } @@ -330,27 +324,42 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, } // Check if partition. - found := false for _, part := range disk.Partitions { if part.Device == dev { candidate := fmt.Sprintf("/dev/disk/by-id/%s-part%d", disk.DeviceID, part.Partition) if shared.PathExists(candidate) { - data.Path = candidate + param.Path = candidate } else { candidate = fmt.Sprintf("/dev/disk/by-path/%s-part%d", disk.DevicePath, part.Partition) if shared.PathExists(candidate) { - data.Path = candidate + param.Path = candidate } } break } } + } - if found { - break - } - // Fallthrough. We didn't find a /dev/disk path for this device, use the original path. + return nil +} + +// AddOSD adds an OSD to the cluster, given the data, WAL and DB devices and their respective +// flags for wiping and encrypting. +func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, db *types.DiskParameter) error { + logger.Debugf("Adding OSD %s", data.Path) + + revert := revert.New() + defer revert.Fail() + + // Lookup a stable path for it. + storage, err := resources.GetStorage() + if err != nil { + return fmt.Errorf("Unable to list system disks: %w", err) + } + + if err := setStablePath(storage, &data); err != nil { + return fmt.Errorf("Failed to set stable disk path: %w", err) } // Get a OSD number. @@ -375,16 +384,25 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, logger.Debugf("Created disk record for osd.%d", nr) + // Keep the old path in case it changes after encrypting. + oldPath := data.Path + dataPath := filepath.Join(os.Getenv("SNAP_COMMON"), "data") osdDataPath := filepath.Join(dataPath, "osd", fmt.Sprintf("ceph-%d", nr)) - // Keep the old path in case it changes after encrypting. - oldPath := data.Path + // if we fail later, make sure we free up the record + revert.Add(func() { + os.RemoveAll(osdDataPath) + s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { + database.DeleteDisk(ctx, tx, s.Name(), oldPath) + return nil + }) + }) // Create directory. err = os.MkdirAll(osdDataPath, 0700) if err != nil { - return fmt.Errorf("Failed to bootstrap monitor: %w", err) + return fmt.Errorf("Failed to create OSD directory: %w", err) } // Wipe and/or encrypt the disk if needed. @@ -393,15 +411,6 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, return fmt.Errorf("Failed to prepare data device: %w", err) } - // if we fail later, make sure we free up the record - revert.Add(func() { - os.RemoveAll(osdDataPath) - s.Database.Transaction(s.Context, func(ctx context.Context, tx *sql.Tx) error { - database.DeleteDisk(ctx, tx, s.Name(), oldPath) - return nil - }) - }) - // Generate keyring. err = genAuth(filepath.Join(osdDataPath, "keyring"), fmt.Sprintf("osd.%d", nr), []string{"mgr", "allow profile osd"}, []string{"mon", "allow profile osd"}, []string{"osd", "allow *"}) if err != nil { @@ -420,6 +429,10 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, // Bootstrap OSD. args := []string{"--mkfs", "--no-mon-config", "-i", fmt.Sprintf("%d", nr)} if wal != nil { + if err = setStablePath(storage, wal); err != nil { + return fmt.Errorf("Failed to set stable path for WAL: %w", err) + } + err = prepareDisk(wal, ".wal", osdDataPath, nr) if err != nil { return fmt.Errorf("Failed to set up WAL device: %w", err) @@ -427,6 +440,10 @@ func AddOSD(s *state.State, data types.DiskParameter, wal *types.DiskParameter, args = append(args, []string{"--bluestore-block-wal-path", wal.Path}...) } if db != nil { + if err = setStablePath(storage, db); err != nil { + return fmt.Errorf("Failed to set stable path for DB: %w", err) + } + err = prepareDisk(db, ".db", osdDataPath, nr) if err != nil { return fmt.Errorf("Failed to set up DB device: %w", err)