Skip to content

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

Merged
sebastian-philipp merged 2 commits intoceph:masterfrom
sebastian-philipp:cephadm-notify_config-ceph-conf-race
Jul 30, 2020
Merged

mgr/cephadm: revamp ceph.conf distribution scheduling #36286
sebastian-philipp merged 2 commits intoceph:masterfrom
sebastian-philipp:cephadm-notify_config-ceph-conf-race

Conversation

@sebastian-philipp
Copy link
Contributor

@sebastian-philipp sebastian-philipp commented Jul 24, 2020

Fixes

Traceback (most recent call last):
  File "mgr/cephadm/module.py", line 271, in __init__
    self.config_notify()
  File "mgr/cephadm/module.py", line 525, in config_notify
    self.config_notify_one(what)
  File "mgr/cephadm/module.py", line 531, in config_notify_one
    self.cache.distribute_new_etc_ceph_ceph_conf()
AttributeError: 'CephadmOrchestrator' object has no attribute 'cache'

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

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 sebastian-philipp requested a review from a team as a code owner July 24, 2020 15:36
mgfritch
mgfritch previously approved these changes Jul 24, 2020
View reviewed changes
@ricardoasmarques ricardoasmarques mentioned this pull request Jul 24, 2020
Merged
@tchaikov
Copy link
Contributor

tchaikov commented Jul 26, 2020

jenkins test make check

tchaikov
tchaikov previously requested changes Jul 26, 2020
View reviewed changes
Copy link
Contributor

@tchaikov tchaikov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to set self.ssh_config_file before referencing it. before this change self.config_notify() creates a set of member variables with the names and values of MODULE_OPTIONS. but after this change they are initialized at the end of constructor.

and we have

2020-07-26T08:52:31.889+0000 7f4d3c56e700 -1 mgr load Traceback (most recent call last):
  File "/usr/share/ceph/mgr/cephadm/module.py", line 281, in __init__
    self._reconfig_ssh()
  File "/usr/share/ceph/mgr/cephadm/module.py", line 592, in _reconfig_ssh
    ssh_config_fname = self.ssh_config_file
AttributeError: 'CephadmOrchestrator' object has no attribute 'ssh_config_file'

see https://pulpito.ceph.com/kchai-2020-07-26_08:18:41-rados-wip-kefu-testing-2020-07-26-1454-distro-basic-smithi/5257575/

@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Jul 26, 2020

Thanks kefu!

@ricardoasmarques
Copy link
Contributor

ricardoasmarques commented Jul 27, 2020

This PR fixes an issue introduced in #35576

@sebastian-philipp sebastian-philipp force-pushed the cephadm-notify_config-ceph-conf-race branch from d5d9e42 to b239f1d Compare July 27, 2020 12:27
@sebastian-philipp sebastian-philipp added wip-swagner-testing My Teuthology tests wip-swagner3-testing and removed wip-swagner-testing My Teuthology tests labels Jul 27, 2020
@sebastian-philipp sebastian-philipp changed the title mgr/cephadm: Call config_notify after self.cache is set mgr/cephadm: revamp ceph.conf distribution scheduling Jul 27, 2020
@sebastian-philipp sebastian-philipp dismissed mgfritch’s stale review July 27, 2020 13:05

the PR has little in common with the original PR here you approved. dismiss as outdated.

@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Jul 27, 2020 

test_orchestrator/module.py volumes/__init__.py
cephadm/module.py: note: In member "config_notify" of class "CephadmOrchestrator":
cephadm/module.py:522: error: Argument 2 to "setattr" has incompatible type "Dict[Any, Any]"; expected "str"
cephadm/module.py:524: error: Argument 2 to "getattr" has incompatible type "Dict[Any, Any]"; expected "str"

@sebastian-philipp
mgr/cephadm: Add test verifying the initializaiton order
537412c 
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Jul 28, 2020

green: http://pulpito.ceph.com/swagner-2020-07-28_08:28:33-rados:cephadm-wip-swagner3-testing-2020-07-27-1524-distro-basic-smithi/ (before the rebase)

@sebastian-philipp
mgr/cephadm: revamp ceph.conf distribution scheduling
aa4c3b2 
Having an in-memeory list doesn't work properly: Especially
when loading the mgr module, we didn't knwo if we should
deploy confs or not.

Now we only distribute ceph.confs, if there is a new mon_map.
We also store that info now in the config store

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp sebastian-philipp force-pushed the cephadm-notify_config-ceph-conf-race branch from b239f1d to aa4c3b2 Compare July 28, 2020 10:27
@sebastian-philipp sebastian-philipp mentioned this pull request Jul 29, 2020
Merged
3 tasks
@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Jul 30, 2020

https://pulpito.ceph.com/swagner-2020-07-30_07:40:52-rados:cephadm-wip-swagner-testing-2020-07-29-1507-distro-basic-smithi/

@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Jul 30, 2020

ping @tchaikov

@tchaikov tchaikov dismissed their stale review July 30, 2020 13:47

dismissed.

ricardoasmarques
ricardoasmarques reviewed Jul 30, 2020
View reviewed changes
Copy link
Contributor

@ricardoasmarques ricardoasmarques left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sebastian-philipp sebastian-philipp merged commit 6dbcd8f into ceph:master Jul 30, 2020
@sebastian-philipp sebastian-philipp mentioned this pull request Aug 4, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tchaikov tchaikov tchaikov left review comments

@mgfritch mgfritch mgfritch left review comments

+1 more reviewer

@ricardoasmarques ricardoasmarques ricardoasmarques approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cephadm needs-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sebastian-philipp @tchaikov @ricardoasmarques @mgfritch