SUSE / DeepSea

A collection of Salt files for deploying, managing and automating Ceph.
GNU General Public License v3.0
162 stars 75 forks source link

populate: don't overwrite fsid if present (bsc#1140932) #1707

Closed jan--f closed 5 years ago

jan--f commented 5 years ago

Superseeds #1706

I think its better to query the pillar for an existing fsid. This way we're independent from how the fsid get into the pillar.

swiftgist commented 5 years ago

Since this one references #1706, I'll just comment here.

The code looks fine for either, but I do not recall why the default tree is getting overwritten (i.e. reinitialized). Was available_roles causing an issue?

The fsid will be the same for either solution since the yaml file is a salt '*' saltutil.pillar_refresh from updating Salt. If the Salt call fails, then we will end up in the same predicament.

One other thought: the public and cluster networks are no longer static if the default tree is reinitialized. If some Ceph admin relied on the defaults, but added network interfaces to their Ceph cluster (e.g. second management network) which then changed the discovery, then the cluster.yml would have new defaults. Eventually, this would update the ceph.conf.

jan--f commented 5 years ago

Since this one references #1706, I'll just comment here.

The code looks fine for either, but I do not recall why the default tree is getting overwritten (i.e. reinitialized). Was available_roles causing an issue? To be honest, I'm not sure anymore. I think it was a case where the network change was intended and it was hard to figure out for the user what happened.

Seems like #1545 was not a good solution to this though. I guess its better to rethink this. How about I alter this PR to default to overwrite=False. This way we'll get the old behaviour back and still have the overwrite functionality in case we decide to use it later.