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
mgr/cephadm: redeploy monitoring stack daemons if their image changes #40507
Conversation
…ner image config setting Fixes: https://tracker.ceph.com/issues/50061 Signed-off-by: Adam King <adking@redhat.com>
|
I like the simplicity. IIUC we would update the default config value and as soon as the mgr is upgraded the monitoring daemons would redeploy. The upgrade sequence would end up being:
It's an odd spot to put the monitoring upgrades in the sequence, but harmless. One thing that might be a bit confusing: changing hte container_image ceph option doesn't have any immediate impact unless you explicitly redeploy; you have to do ceph orch upgrade start to trigger a ceph upgrade. OTOH, just changing the mgr/cephadm/*_image option and it'll immediately upgrade the non-ceph daemon. |
|
jenkins test api |
| if not action and dd.daemon_type in MONITORING_STACK_TYPES and not self.mgr.spec_store[dd.service_name()].deleted: | ||
| if dd.container_image_name != self.mgr._get_container_image(dd.name()): | ||
| self.log.info('Redeploying %s. Daemon\'s current image does not match mgr/cephadm/container_image_%s config setting.' | ||
| % (dd.name(), dd.daemon_type)) |
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.
Maybe Redeploying %s (container image changed)...' % dd.name() so this is consistent with the other messages.
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.
nit, no need to use % for formatting the logging message, just
self.log.info('Redeploying %s. Daemon\'s current image does not match mgr/cephadm/container_image_%s config setting.',
dd.name(), dd.daemon_type)|
I'm a bit concerned that we're introducing a special case for the monitoring daemons here. This already contains the Monitoring daemons: ceph/src/pybind/mgr/cephadm/upgrade.py Line 488 in 502409c
On the other hand, the upgrade is odd actually, as it does not follow the declarative approach. Imagine the upgrade would work like for each daemon:
Which would be great and make cephadm even more declarative. I see two good ways here:: Either we opt for making cephadm more declarative by redeploying daemons, if the container_image does not match. (Not just monitoring, but all) Or we opt for keeping the current behavior and keep the existing upgrade logic. |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
Fixes: https://tracker.ceph.com/issues/50061
Signed-off-by: Adam King adking@redhat.com
Idea is to automatically redeploy the relevant monitoring stack daemon if the user changes the image we're using for it using 'ceph config set . . .'
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox