From 5520ee35bb4d6ca442cc5dde78466924f70088cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 5 Jun 2020 15:06:35 +0200 Subject: [PATCH 1/2] ceph: check if ok-to-stop even on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if we cannot determine wheter all the OSDs are running on the host, we still need to perform the ok-to-stop check. It is far more common to have more than 3 OSDs anyways. Signed-off-by: Sébastien Han --- pkg/daemon/ceph/client/upgrade.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/daemon/ceph/client/upgrade.go b/pkg/daemon/ceph/client/upgrade.go index 117704b32..a3928688d 100644 --- a/pkg/daemon/ceph/client/upgrade.go +++ b/pkg/daemon/ceph/client/upgrade.go @@ -390,10 +390,9 @@ func osdDoNothing(context *clusterd.Context, clusterName string) bool { // aio means all in one aio, err := allOSDsSameHost(context, clusterName) if err != nil { - // That's tricky, we are about to perform an update so it's difficult to break the update for this - // let's consider this is not a problem but log what happened - logger.Warningf("not able to determine if all OSDs are running on the same host, not performing upgrade check, running in best-effort") - return true + // If calling osd list fails, we assume there are more than 3 OSDs and we check if ok-to-stop + logger.Warningf("failed to determine if all osds are running on the same host, performing upgrade check anyways. %v", err) + return false } if aio { From 3c7a25286245d165c5d68d16a66591f80c634038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 5 Jun 2020 15:13:50 +0200 Subject: [PATCH 2/2] ceph: fix osd number detection on upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the cluster has 3 OSDs, during the upgrade when the OSD restarts it will temporarily disappear from the "ceph versions" list so we need a more robust way to list the number of OSDs and always returns the total number of OSDs. Closes: https://github.com/rook/rook/issues/5521 Signed-off-by: Sébastien Han --- pkg/daemon/ceph/client/upgrade.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/daemon/ceph/client/upgrade.go b/pkg/daemon/ceph/client/upgrade.go index a3928688d..3769ce285 100644 --- a/pkg/daemon/ceph/client/upgrade.go +++ b/pkg/daemon/ceph/client/upgrade.go @@ -370,21 +370,18 @@ func buildHostListFromTree(tree OsdTree) (OsdTree, error) { // it checks for various cluster info like number of OSD and their placement // it returns 'true' if we need to do nothing and false and we should pre-check/post-check func osdDoNothing(context *clusterd.Context, clusterName string) bool { - versions, err := GetAllCephDaemonVersions(context, clusterName) + osds, err := OsdListNum(context, clusterName) if err != nil { - logger.Warningf("failed to get ceph daemons versions, this likely means there is no cluster yet. %v", err) - return true + logger.Warningf("failed to determine the total number of osds. will check if the osd is ok-to-stop anyways. %v", err) + // If calling osd list fails, we assume there are more than 3 OSDs and we check if ok-to-stop + // If there are less than 3 OSDs, the ok-to-stop call will fail + // this can still be controlled by setting continueUpgradeAfterChecksEvenIfNotHealthy + // At least this will happen for a single OSD only, which means 2 OSDs will restart in a small interval + return false } - - if len(versions.Osd) == 1 { - // now trying to parse and find how many osds are presents - // if we have less than 3 osds we skip the check and do best-effort - for _, osdCount := range versions.Osd { - if osdCount < 3 { - logger.Warningf("the cluster has less than 3 OSDs, not performing upgrade check, running in best-effort") - return true - } - } + if len(osds) < 3 { + logger.Warningf("the cluster has less than 3 osds, not performing upgrade check, running in best-effort") + return true } // aio means all in one