mgr/cephadm: Manage /etc/ceph/ceph.conf#35576

Merged

sebastian-philipp merged 6 commits intoceph:masterfrom

sebastian-philipp:cephadm-etc-ceph-ceph-conf

Jun 30, 2020

Contributor

sebastian-philipp commented Jun 15, 2020 •

edited

Loading

TODO

replace ceph.conf handling in qa/tasks/cephadm.py

ceph/qa/tasks/cephadm.py

Lines 961 to 964 in b2de27b

    
           teuthology.sudo_write_file( 
        
               remote=remote, 
        
               path='/etc/ceph/{}.conf'.format(cluster_name), 
        
               data=ctx.ceph[cluster_name].config_file)

pytest

Considerations

This is now embedded using the usual check_* methods, which means it's using a different code path than the daemon's reconfig calls. otoh, we're not dealing with daemons here.
I don't like error_ok=True, as id detours the normal python error handling. Making it necessary to add special code for it.

To enable the management of ceph.conf

ceph config set mgr mgr/cephadm/manage_etc_ceph_ceph_conf true

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

sebastian-philipp added the cephadm label

sebastian-philipp requested a review from a team as a code owner

June 15, 2020 16:19

sebastian-philipp changed the title ~~mgr/cephadm: Manage /etc/ceph/ceph.conf~~ [WIP] mgr/cephadm: Manage /etc/ceph/ceph.conf

jmolmo reviewed

View reviewed changes

src/pybind/mgr/cephadm/inventory.py Outdated Show resolved Hide resolved

jschmid1 reviewed

View reviewed changes

src/pybind/mgr/cephadm/inventory.py Outdated Show resolved Hide resolved

jschmid1 reviewed

View reviewed changes

src/pybind/mgr/cephadm/module.py Show resolved Hide resolved

jschmid1 reviewed

View reviewed changes

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

jschmid1 reviewed

View reviewed changes

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

jschmid1 reviewed

View reviewed changes

src/pybind/mgr/cephadm/module.py Show resolved Hide resolved

sebastian-philipp force-pushed the cephadm-etc-ceph-ceph-conf branch from 094c4e4 to dd53952 Compare

June 16, 2020 13:38

mgfritch reviewed

View reviewed changes

Contributor

mgfritch left a comment

Couple thoughts/questions:

seems like this would break any host with a multi-cluster setup?
what about ceph.client.admin.keyring or ceph.pub ?
if the action fails should it be retried on the next host check?
what if the ceph.conf file was changed manually? detect this?
add an action specific to sync'ing ceph.conf on demand?

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

src/pybind/mgr/cephadm/module.py

                           self.prometheus_alerts_path = ''
                           self.migration_current = None
                           self.config_dashboard = True
+                          self.manage_etc_ceph_ceph_conf = True

Contributor

mgfritch Jun 16, 2020

Suggested change

      
                        self.manage_etc_ceph_ceph_conf = True
          
                        self.manage_etc_ceph_ceph_conf = False

change this to match the default value in the MODULE_OPTION ?

Contributor Author

sebastian-philipp Jun 17, 2020

that kills the multi-cluster support. don't know if we want that.

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

src/pybind/mgr/cephadm/module.py

Comment on lines +1284 to +1313

+                              out, err, code = remoto.process.check(
+                                  conn,
+                                  ['mkdir', '-p', '/etc/ceph'])

Contributor

mgfritch Jun 16, 2020

what if a different output dir was used during bootstrap (e.g. vstart)?

$ cephadm bootstrap ... --output-dir ~/ceph/build/

Contributor Author

sebastian-philipp Jun 17, 2020

Under no circumstance, I'd automatically enable manage_etc_ceph_ceph_conf if output-dir is something different than /etc/ceph

src/pybind/mgr/cephadm/inventory.py Outdated Show resolved Hide resolved

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved

Contributor Author

sebastian-philipp commented Jun 16, 2020

Couple thoughts/questions:

seems like this would break any host with a multi-cluster setup?

definitely.

what about ceph.client.admin.keyring or ceph.pub ?

deploying the admin keyring? hm. maybe. might be an idea, but on which hosts? Placement spec? @ricardoasmarques + @fmount wdyt?

add an action specific to sync'ing ceph.conf on demand?

I think we need this in any case. Maybe ceph cephadm deploy-etc-ceph-conf [<host>]?

if the action fails should it be retried on the next host check?

I#d make this a manual step. I really don#t want to head into the realm of automatically deploying non-containerized stuff.

what if the ceph.conf file was changed manually? detect this?

I'd simply overwrite it.

Contributor

fmount commented Jun 17, 2020

Couple thoughts/questions:

seems like this would break any host with a multi-cluster setup?

definitely.

what about ceph.client.admin.keyring or ceph.pub ?

deploying the admin keyring? hm. maybe. might be an idea, but on which hosts? Placement spec? @ricardoasmarques + @fmount wdyt?

Speaking about the OpenStack context with Director deployed Ceph cluster, the mons/mgrs are 3 and they are collocated into the Controller nodes.
I should login to one of the three controllers and exec a Ceph client to run commands against the Ceph cluster, but if the Controller[0] (which is the bootstrap node in cephadm terminology) goes down, I should be able to login on the second Controller, get a ceph client (like [1]) and work with my cluster.
The ceph-ansible deployment gives this kind of experience, and I think having the keyring and ceph.conf is the minimal amount of actions that should be taken by cephadm when a new cluster is deployed or new monitors are added/scheduled to new hosts via spec.

add an action specific to sync'ing ceph.conf on demand?

I think we need this in any case. Maybe ceph cephadm deploy-etc-ceph-conf [<host>]?

Right now we have this role [2] that is able to get the relevant data from the first controller (or mon) and sync them to the other existing monitors, but we still have the problem of updating ceph.conf with the list of monitors, so I'm not sure it's just a matter of copying keyring and conf but also be consistent (in terms of ceph.conf) with the state of the cluster (e.g., the mon_host param should be updated accordingly).
This role is useful also for clusters that are external to OpenStack and we need a client to be able to access the Ceph Cluster.

if the action fails should it be retried on the next host check?

I#d make this a manual step. I really don#t want to head into the realm of automatically deploying non-containerized stuff.

Agree with that

what if the ceph.conf file was changed manually? detect this?

I'd simply overwrite it.

Manual operations should be avoided and we need a way to make the config consistent, but there are useful options [3] that can be configured on a new deployment (e.g. HCI environments).
Is there a plan to have a way to customize ceph.conf somehow?

[1] https://github.com/fmount/tripleo-ceph/blob/master/roles/tripleo_cluster_set_container_cli/tasks/set_container_cli.yaml#L13
[2] https://github.com/fmount/tripleo-ceph/blob/master/roles/ceph_client/tasks/sync.yml
[3] https://github.com/ceph/ceph-ansible/blob/master/roles/ceph-config/templates/ceph.conf.j2

Contributor Author

sebastian-philipp commented Jun 17, 2020

Couple thoughts/questions:

what about ceph.client.admin.keyring or ceph.pub ?

deploying the admin keyring? hm. maybe. might be an idea, but on which hosts? Placement spec? @ricardoasmarques + @fmount wdyt?

Speaking about the OpenStack context with Director deployed Ceph cluster, the mons/mgrs are 3 and they are collocated into the Controller nodes.
I should login to one of the three controllers and exec a Ceph client to run commands against the Ceph cluster, but if the Controller[0] (which is the bootstrap node in cephadm terminology) goes down, I should be able to login on the second Controller, get a ceph client (like [1]) and work with my cluster.
The ceph-ansible deployment gives this kind of experience, and I think having the keyring and ceph.conf is the minimal amount of actions that should be taken by cephadm when a new cluster is deployed or new monitors are added/scheduled to new hosts via spec.

Interesting. Onto which hosts should the admin keyring distributed? At least I'd think this might be independent of the MONs.

add an action specific to sync'ing ceph.conf on demand?

I think we need this in any case. Maybe ceph cephadm deploy-etc-ceph-conf [<host>]?

Right now we have this role [2] that is able to get the relevant data from the first controller (or mon) and sync them to the other existing monitors, but we still have the problem of updating ceph.conf with the list of monitors, so I'm not sure it's just a matter of copying keyring and conf but also be consistent (in terms of ceph.conf) with the state of the cluster (e.g., the mon_host param should be updated accordingly).

Actually you can't properly manage the ceph.conf at all, simply as you're not getting notified when a new MON enters the cluster.

Which files are you interested in, other than the ceph.conf?

This role is useful also for clusters that are external to OpenStack and we need a client to be able to access the Ceph Cluster.

OK, things get interesing now. If you add a new host to the cluster, cephamd will:

deploy node-exporter
deploy ceph-crash
and now, it will also deploy the ceph.conf.

Is this sufficient and ok for a client machine? Otherwise you'll need to distribute the ceph.conf yourself.

what if the ceph.conf file was changed manually? detect this?

I'd simply overwrite it.

Manual operations should be avoided and we need a way to make the config consistent, but there are useful options [3] that can be configured on a new deployment (e.g. HCI environments).
Is there a plan to have a way to customize ceph.conf somehow?

By this point, ceph.conf should be nearly empty and should contain only the information to access the other MONs. If you need anything, please use ceph config for it.

[1] https://github.com/fmount/tripleo-ceph/blob/master/roles/tripleo_cluster_set_container_cli/tasks/set_container_cli.yaml#L13
[2] https://github.com/fmount/tripleo-ceph/blob/master/roles/ceph_client/tasks/sync.yml
[3] https://github.com/ceph/ceph-ansible/blob/master/roles/ceph-config/templates/ceph.conf.j2

Contributor Author

sebastian-philipp commented Jun 17, 2020

@jdurgin do you know, if we will need support for named ceph clusters in qa/tasks/cephadm.py? Maybe for rbd-mirror or RGW multizone?

something like

ceph/qa/tasks/cephadm.py

Lines 961 to 964 in b2de27b

    
           teuthology.sudo_write_file( 
        
               remote=remote, 
        
               path='/etc/ceph/{}.conf'.format(cluster_name), 
        
               data=ctx.ceph[cluster_name].config_file)

I'd like to avoid adding this to cephadm itself.

sebastian-philipp force-pushed the cephadm-etc-ceph-ceph-conf branch 2 times, most recently from fd9426e to 46b9dfb Compare

June 18, 2020 10:32

sebastian-philipp requested a review from ricardoasmarques

June 18, 2020 10:33

sebastian-philipp added the needs-qa label

Contributor Author

sebastian-philipp commented Jun 18, 2020

right now, this is IMO safe to merge, as this is disabled by default using a feature-flag. enables us to improve it later on (e.g. enable it by default, change the behaviour etc)

Contributor

fmount commented Jun 18, 2020

Couple thoughts/questions:

what about ceph.client.admin.keyring or ceph.pub ?

deploying the admin keyring? hm. maybe. might be an idea, but on which hosts? Placement spec? @ricardoasmarques + @fmount wdyt?

Speaking about the OpenStack context with Director deployed Ceph cluster, the mons/mgrs are 3 and they are collocated into the Controller nodes.
I should login to one of the three controllers and exec a Ceph client to run commands against the Ceph cluster, but if the Controller[0] (which is the bootstrap node in cephadm terminology) goes down, I should be able to login on the second Controller, get a ceph client (like [1]) and work with my cluster.
The ceph-ansible deployment gives this kind of experience, and I think having the keyring and ceph.conf is the minimal amount of actions that should be taken by cephadm when a new cluster is deployed or new monitors are added/scheduled to new hosts via spec.

Interesting. Onto which hosts should the admin keyring distributed? At least I'd think this might be independent of the MONs.

In general OpenStack collocates monitors and mgrs in controller nodes where the ctlplane is found, so at least monitors should have all the relevant data to help operators interact w/ Ceph cluster. The undercloud isn't able to reach the StorageNetwork on the overcloud, so monitors are the first entrypoint for Director deployed clusters.
However, any node that is able to reach the Ceph cluster public network (which is set by [1]) can be configured as a client, so yes, this not depends-on the MONs and that's the reason we have a sync role [2] to copy both conf and keyring.

add an action specific to sync'ing ceph.conf on demand?

I think we need this in any case. Maybe ceph cephadm deploy-etc-ceph-conf [<host>]?

Right now we have this role [2] that is able to get the relevant data from the first controller (or mon) and sync them to the other existing monitors, but we still have the problem of updating ceph.conf with the list of monitors, so I'm not sure it's just a matter of copying keyring and conf but also be consistent (in terms of ceph.conf) with the state of the cluster (e.g., the mon_host param should be updated accordingly).

Actually you can't properly manage the ceph.conf at all, simply as you're not getting notified when a new MON enters the cluster.

Which files are you interested in, other than the ceph.conf?

Not sure right now, ceph.conf and keys are required when a new mon is scheduled, but I need to understand which is the gap compared to ceph-ansible to answer your question.
For now I think generate a proper ceph.conf and sync it across the mons is the minimal amount of actions that we need to make the clients working.

This role is useful also for clusters that are external to OpenStack and we need a client to be able to access the Ceph Cluster.

OK, things get interesing now. If you add a new host to the cluster, cephamd will:
* deploy node-exporter

* deploy ceph-crash

* and now, it will also deploy the ceph.conf.
Is this sufficient and ok for a client machine? Otherwise you'll need to distribute the ceph.conf yourself.

For a client node != mons it's ok, I'd like to copy and sync stuff myself since it's an extra logic for cephadm that cannot cover all the potential scenarios, what I'm not sure right now it's the reason why you deploy node-exporter and ceph-crash by default in a client node (it can be external to the Ceph cluster)

what if the ceph.conf file was changed manually? detect this?

I'd simply overwrite it.

Manual operations should be avoided and we need a way to make the config consistent, but there are useful options [3] that can be configured on a new deployment (e.g. HCI environments).
Is there a plan to have a way to customize ceph.conf somehow?

By this point, ceph.conf should be nearly empty and should contain only the information to access the other MONs. If you need anything, please use ceph config for it.

Ack, I'll try to push config using this approach

[1] https://github.com/fmount/tripleo-ceph/blob/master/roles/tripleo_cluster_set_container_cli/tasks/set_container_cli.yaml#L13
[2] https://github.com/fmount/tripleo-ceph/blob/master/roles/ceph_client/tasks/sync.yml
[3] https://github.com/ceph/ceph-ansible/blob/master/roles/ceph-config/templates/ceph.conf.j2

[1] https://github.com/fmount/tripleo-ceph/blob/master/roles/tripleo_cluster_mon_config/tasks/set_monitor_public_network.yaml
[2] https://github.com/fmount/tripleo-ceph/blob/master/roles/ceph_client/tasks/sync.yml

ricardoasmarques mentioned this pull request

potentially invalid ceph.conf after moving monitors ceph/ceph-salt#199

Closed

sebastian-philipp added the wip-swagner3-testing label

sebastian-philipp changed the title ~~[WIP] mgr/cephadm: Manage /etc/ceph/ceph.conf~~ mgr/cephadm: Manage /etc/ceph/ceph.conf

Contributor Author

sebastian-philipp commented Jun 24, 2020

green: http://pulpito.ceph.com/swagner-2020-06-23_13:15:09-rados:cephadm-wip-swagner3-testing-2020-06-23-1058-distro-basic-smithi/

failures:

https://tracker.ceph.com/issues/46046 (NFS)
https://tracker.ceph.com/issues/45628 (upgrade)

Contributor Author

sebastian-philipp commented Jun 24, 2020

In general OpenStack collocates monitors and mgrs in controller nodes where the ctlplane is found, so at least monitors should have all the relevant data to help operators interact w/ Ceph cluster. The undercloud isn't able to reach the StorageNetwork on the overcloud, so monitors are the first entrypoint for Director deployed clusters.
However, any node that is able to reach the Ceph cluster public network (which is set by [1]) can be configured as a client, so yes, this not depends-on the MONs and that's the reason we have a sync role [2] to copy both conf and keyring.

sounds reasonable.

Not sure right now, ceph.conf and keys are required when a new mon is scheduled, but I need to understand which is the gap compared to ceph-ansible to answer your question.
For now I think generate a proper ceph.conf and sync it across the mons is the minimal amount of actions that we need to make the clients working.

ok. I'm going to merge this as it is.

I think we can consider adding new things to distribute later on.

This role is useful also for clusters that are external to OpenStack and we need a client to be able to access the Ceph Cluster.

For a client node != mons it's ok, I'd like to copy and sync stuff myself since it's an extra logic for cephadm that cannot cover all the potential scenarios, what I'm not sure right now it's the reason why you deploy node-exporter and ceph-crash by default in a client node (it can be external to the Ceph cluster)

that can be configured by changing the placement specification of the services. Deploying them on all known hosts is the default right now.

Contributor Author

sebastian-philipp commented Jun 24, 2020

jenkins retest this

sebastian-philipp removed the wip-swagner3-testing label

asettle mentioned this pull request

[doc] 6.5.3.2 Deploy Ceph OSDs SUSE/doc-ses#391

Closed

2 tasks

sebastian-philipp removed the needs-qa label

sebastian-philipp added the needs-review label

Contributor

mgfritch commented Jun 24, 2020

jenkins test make check

Contributor

mgfritch commented Jun 24, 2020

jenkins test docs

Contributor

mgfritch commented Jun 24, 2020

jenkins test make check

sebastian-philipp added the needs-rebase label

sebastian-philipp added 6 commits

June 25, 2020 15:55


          mgr/cephadm: add type hints for _refresh_host_*()

c37412d

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>


          python-common: Add simple str(HostSpec)

e60a2a5

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>


          mgr/cephadm: refactor _run_cephadm

a0c39f3

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>


          mgr/cephadm: config_notify now provides a hook

1cd4d0f

reason is, we want to use this hook to schedule a
ceph.conf update for all hosts.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>


          mgr/tests: _ceph_get_module_option returns proper type now.

acdd26a

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>


          mgr/cephadm: Manage /etc/ceph/ceph.conf

c18ad7c

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

sebastian-philipp force-pushed the cephadm-etc-ceph-ceph-conf branch from 46b9dfb to c18ad7c Compare

June 25, 2020 13:57

sebastian-philipp added needs-qa and removed needs-review labels

mgfritch added wip-mgfritch-testing and removed needs-rebase labels

ricardoasmarques mentioned this pull request

[WIP] Ceph config should not be managed by 'ceph-salt' ceph/ceph-salt#203

Closed

sebastian-philipp added the wip-swagner-testing label

Contributor Author

sebastian-philipp commented Jun 29, 2020

https://pulpito.ceph.com/swagner-2020-06-29_09:26:42-rados:cephadm-wip-swagner-testing-2020-06-26-1524-distro-basic-smithi/

Failure: https://tracker.ceph.com/issues/46178

sebastian-philipp added needs-review and removed needs-qa wip-swagner-testing labels

mgfritch approved these changes

View reviewed changes

Contributor

mgfritch left a comment

this still somewhat abuses the host refresh when I think a dedicated orch host update .. or similar command might be better ...

otherwise, lgtm

jschmid1 self-requested a review

June 30, 2020 08:44

jschmid1 approved these changes

View reviewed changes

sebastian-philipp merged commit e5d8df1 into ceph:master

sebastian-philipp mentioned this pull request

octopus: cephadm batch backport July (1) #35898

Merged

ricardoasmarques mentioned this pull request

Enable 'ceph.conf' management by cephadm ceph/ceph-salt#291

Merged

sebastian-philipp mentioned this pull request

update deployment backend to cephadm batrick/ceph-linode#57

Merged

ricardoasmarques mentioned this pull request

mgr/cephadm: revamp ceph.conf distribution scheduling #36286

Merged

3 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephadm needs-review wip-mgfritch-testing