-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ncm-ceph: support for luminous, bluestore, ceph-volume and per host osd deployment #1230
Conversation
Why the new PR? |
because some browsers were having difficulties loading the page :) |
@@ -30,6 +30,8 @@ type ceph_osd = { | |||
'journal_path' ? string | |||
'crush_weight' : double(0..) = 1.0 | |||
'labels' ? string[1..] | |||
'storetype'? string with match(SELF, '^(blue|file)store$') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use choice? but this is probably not correct (new entry in old schema)? also panlint fixes and remove next empty line?
'filestore_xattr_use_omap' ? boolean | ||
'fsid' : type_uuid | ||
'mon_cluster_log_to_syslog' : boolean = true | ||
'mon_initial_members' : type_network_name [1..] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no space before [
|
||
@documentation{ overarching ceph cluster type, with osds, mons and msds } | ||
type ceph_cluster = { | ||
'monitors' : ceph_monitor {3..} # with match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no space before{
'max_add_osd_failures' : long(0..) = 0 | ||
}; | ||
|
||
type ceph_supported_version = string with match(SELF, '[0-9]+\.[0-9]+(\.[0-9]+)?'); # TODO minimum 12.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\d
instead of [0-9]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support globs?
}; | ||
|
||
type ceph_supported_version = string with match(SELF, '[0-9]+\.[0-9]+(\.[0-9]+)?'); # TODO minimum 12.2.2 | ||
type ceph_deploy_supported_version = string with match(SELF, '[0-9]+\.[0-9]+\.[0-9]+'); # TODO minimum 1.5.39 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
return; | ||
} | ||
} | ||
$self->debug(1, "Node ready to receive ceph-commands"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this mean?
return $success || $self->write_init_cfg(); | ||
|
||
} | ||
sub deploy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
insert newline above
|
||
$self->debug(5, "deploy hash:", Dumper($map)); | ||
$self->verbose("Running ceph-deploy commands. This can take some time when adding new daemons."); | ||
foreach my $hostname (sort keys(%{$map})) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no {}
required
my ($self, $map) = @_; | ||
|
||
$self->debug(5, "deploy hash:", Dumper($map)); | ||
$self->verbose("Running ceph-deploy commands. This can take some time when adding new daemons."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
report names of hosts
my ($ec, $jstr) = $self->{Cluster}->run_ceph_command([qw(mon dump)], 'get mon map', nostderr => 1) or return; | ||
my $monsh = decode_json($jstr); | ||
foreach my $mon (@{$monsh->{mons}}){ | ||
$self->add_existing('mon', $mon->{name}, { addr => $mon->{addr}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove space before addr
* Copy bootstrap-osd key for ncm-download | ||
|
||
Do for each OSDServer (If deployhosts is osd server, start with this one) | ||
* remove osds from a server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way to check ceph state that removing osds does not cause real issues? no dataloss should occur before zapping the disks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added to wait before rebalance is ok (after ceph osd out)
my $mdshs = decode_json($jstr); | ||
my $fsmap = $mdshs->{fsmap}; | ||
foreach my $fs (@{$fsmap->{filesystems}}){ | ||
foreach my $mds (values %{$fs->{mdsmap}->{info}}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loop over sorted keys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why keys? I dont need them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can loop over sorted values if you want, as long as it reproducable. if you don;t need the keys, use a list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is json output of ceph, keys are pids or someting. I'll sort the values then
my ($self, $hostname, $mon) = @_; | ||
$self->debug(3, "Comparing mon $hostname"); | ||
my $ceph_mon = $self->{ceph}->{$hostname}->{mon}; | ||
if ($ceph_mon->{addr} =~ /^0\.0\.0\.0:0/) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use eq
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's no othe rway to check this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a corner case seen with an earlier version. running the ceph-deploy create initial command sometimes went wrong and the ceph-mon is in a weird existing not configured state. Don't know if this can still happen or how to reproduce this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add this in a comment
$self->{quattor}->{$name}->{daemons}->{$type} = $daemon; | ||
$self->{quattor}->{$name}->{fqdn} = $daemon->{fqdn}; | ||
return 1; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
{ | ||
my ($self) = @_; | ||
my @actions = (\&mon_hash, \&mgr_hash, \&mds_hash); | ||
foreach my $type (@actions){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
space before {
my ($self) = @_; | ||
my $quattor = $self->{Cluster}->{cluster}; | ||
$self->debug(2, "Building information from quattor"); | ||
while (my ($hostname, $mon) = each(%{$quattor->{monitors}})) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no each, loop over sorted keys
$self->add_quattor('mon', $hostname, $mon); | ||
$self->add_quattor('mgr', $hostname, $mon); # add a mgr for each mon | ||
} | ||
while (my ($hostname, $mds) = each(%{$quattor->{mdss}})) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no each, loop over sorted keys
=item 6. Run the component again to start the configuration of the new cluster | ||
|
||
=item 7. When the component now runs on OSD servers, it will deploy the local OSDs | ||
=back |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add newline
'max_add_osd_failures' : long(0..) = 0 | ||
}; | ||
|
||
type ceph_supported_version = string with match(SELF, '[0-9]+\.[0-9]+(\.[0-9]+)?'); # TODO minimum 12.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support globs?
} | ||
# ceph-volume lvm create --bluestore --data /dev/sdk | ||
my $devpath = "/dev/" . unescape($name); | ||
my $succes = $self->run_command([qw(ceph-volume lvm create), "--$attrs->{storetype}", "--data", $devpath], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
succes -> success
|
||
|
||
} | ||
sub deploy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
insert newline
} | ||
$self->debug(1, "Deployed osd $name"); | ||
return 1; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove newlines
variable CEPH_SCHEMA_VERSION ?= 'v1'; | ||
|
||
include format('components/${project.artifactId}/%s/schema', CEPH_SCHEMA_VERSION); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add site overrid like https://github.com/quattor/maven-tools/pull/170/files#diff-fb9fe79c1696b94dcb0e714774c01f61R71
5bcddc0
to
3d0ae26
Compare
@stdweird what's the likelihood of this being ready for merge in 18.3? |
@jrha need a new release of the buildtools first, and i probably need to rereview this. but we use this in production for new and upgraded ceph clusters |
Also add Commands helper module
Also ClusterMap helper Module
@jrha needs maven-tools 1.58 for building 😄 |
@hpcugentbot retest this please |
Merged as is, we can open more issues if they crop up. |
Recreate component for luminous support, with bluestore, ceph-volume instead of ceph-deploy for osd creation.
This component will only work with Luminous > 12.2.2 and beyond. For Jewel and earlier there is still the v1 of component
Replaces #1224 (which was getting far too large).
Fixes #740, fixes #741, fixes #742, fixes #743, fixes #776.