From b79c223986cd94899be273cb7a66de1a5e7518a2 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Mon, 16 Dec 2019 17:21:50 -0700 Subject: [PATCH] ceph: only set isUpgrade when the image changes The isUpgrade flag was being set every time the operator was started. This caused the upgrade checks to be run every time the operator restarted. Thus, if any mons were down, you likely would get stuck on the upgrade checks since another mon could not be stopped. We need to rely on the upgrade being detected either by a different image name from the cluster CR update, or from different versions of the daemons running in case the operator was restarted in the middle of an upgrade. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/cluster.go | 5 ++--- pkg/operator/ceph/cluster/cluster_test.go | 2 +- pkg/operator/ceph/cluster/controller.go | 5 +++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 546703f1b..3d1f87bed 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -379,7 +379,7 @@ func (c *cluster) checkSetOrchestrationStatus() bool { } // This function compare the Ceph spec image and the cluster running version -// It returns false if the image is different and true if identical +// It returns true if the image is different and false if identical func diffImageSpecAndClusterRunningVersion(imageSpecVersion cephver.CephVersion, runningVersions client.CephDaemonsVersions) (bool, error) { numberOfCephVersions := len(runningVersions.Overall) if numberOfCephVersions == 0 { @@ -403,10 +403,9 @@ func diffImageSpecAndClusterRunningVersion(imageSpecVersion cephver.CephVersion, clusterRunningVersion := *version // If this is the same version - // We return 'true' so that we restart using the Ceph checks if cephver.IsIdentical(clusterRunningVersion, imageSpecVersion) { logger.Debugf("both cluster and image spec versions are identical, doing nothing %s", imageSpecVersion.String()) - return true, nil + return false, nil } if cephver.IsSuperior(imageSpecVersion, clusterRunningVersion) { diff --git a/pkg/operator/ceph/cluster/cluster_test.go b/pkg/operator/ceph/cluster/cluster_test.go index 16265d471..d8ecb7839 100755 --- a/pkg/operator/ceph/cluster/cluster_test.go +++ b/pkg/operator/ceph/cluster/cluster_test.go @@ -109,7 +109,7 @@ func TestDiffImageSpecAndClusterRunningVersion(t *testing.T) { m, err = diffImageSpecAndClusterRunningVersion(fakeImageVersion, dummyRunningVersions5) assert.NoError(t, err) - assert.True(t, m) + assert.False(t, m) } func TestMinVersion(t *testing.T) { diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index 9d7d4b23f..e07a4ab77 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -624,6 +624,7 @@ func (c *ClusterController) onUpdate(oldObj, newObj interface{}) { // Also we make sure there is actually an upgrade to perform // It's not because the image spec changed that the ceph version did // Someone could use the same Ceph version but with a different base OS content + cluster.isUpgrade = false if versionChanged { // we compare against cluster.Info.CephVersion since it received the new spec version earlier // so don't get confused by the name of the function and its arguments @@ -645,9 +646,9 @@ func (c *ClusterController) onUpdate(oldObj, newObj interface{}) { return } } - // If Ceph is healthy let's start the upgrade! - cluster.isUpgrade = true } + // If Ceph is healthy let's start the upgrade! + cluster.isUpgrade = true } else { logger.Infof("ceph daemons running versions are: %+v", runningVersions) }