Add the WAL and DB devices for the disk-add command #222
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Luciano Lo Giudice <luciano.logiudice@canonical.com>
Signed-off-by: Luciano Lo Giudice <luciano.logiudice@canonical.com>
Signed-off-by: Peter Sabaini <peter.sabaini@canonical.com>
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 <luciano.logiudice@canonical.com>
Signed-off-by: Luciano Lo Giudice <luciano.logiudice@canonical.com>
sabaini
requested changes
Sep 27, 2023
lmlg
force-pushed
the
wal-db
branch
2 times, most recently
from
4c94287
to
09127e2
Compare
sabaini
requested changes
Oct 4, 2023
microceph/ceph/osd.go
Outdated
| logger.Debugf("Adding OSD %s", path) | ||
| // 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 { |
There was a problem hiding this comment.
Hm, is there a reason to pass data as by value and wal / db by ref?
There was a problem hiding this comment.
wal and db may be nil, whereas data cannot.
microceph/api/disks.go
Outdated
| @@ -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) | |||
|
|
|||
| data := types.DiskParameter{req.Path, req.Encrypt, req.Wipe} | |||
There was a problem hiding this comment.
NIT: "data" definition should be consistent with wal/db object initialisation.
There was a problem hiding this comment.
Data can never be nil, hence why it's not a pointer.
UtkarshBhatthere
requested changes
Oct 4, 2023
sabaini
approved these changes
Oct 5, 2023
yes, of course. |
lmlg
changed the title
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 <luciano.logiudice@canonical.com>
|
Note: CI tests for this change will be taken up in a separate PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to add the needed support for write-ahead log (WAL) and database devices when adding OSDs to the cluster.