From aa0e5225246ce1ae85e564da5fd92ed0035e0c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Mon, 2 Sep 2019 22:03:06 +0200 Subject: [PATCH 1/4] ceph: remove dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These functions were not used anywhere, so removing them. Signed-off-by: Sébastien Han --- pkg/operator/ceph/config/config.go | 78 ------------------------------ 1 file changed, 78 deletions(-) diff --git a/pkg/operator/ceph/config/config.go b/pkg/operator/ceph/config/config.go index 0d0d65e2b..09664124f 100644 --- a/pkg/operator/ceph/config/config.go +++ b/pkg/operator/ceph/config/config.go @@ -24,13 +24,9 @@ import ( "strings" "github.com/coreos/pkg/capnslog" - "github.com/go-ini/ini" rookceph "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" - "github.com/rook/rook/pkg/operator/k8sutil" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var logger = capnslog.NewPackageLogger("github.com/rook/rook", "op-config") @@ -132,77 +128,3 @@ func SetDefaultAndUserConfigs( return nil } - -// LegacyConfigMapOverrideDefined checks to see if the legacy ConfigMap override exists with -// overrides defined. If it exists and has config overrides defined, this will return true. If it -// does not exist or exists without config overrides set, it will return false. -func LegacyConfigMapOverrideDefined(context *clusterd.Context, namespace string) (bool, error) { - cm, err := context.Clientset.CoreV1().ConfigMaps(namespace). - Get(k8sutil.ConfigOverrideName, metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - logger.Debugf("override configmap does not exist") - return false, nil - } - // if we can't determine existence or not, assume it exists - return true, fmt.Errorf("failed to see if legacy override ConfigMap exists. %+v", err) - } - - o, ok := cm.Data[k8sutil.ConfigOverrideVal] - if !ok || (ok && o == "") { - // if no override set or override is empty string - return false, nil - } - - return true, nil -} - -func configFileTextToOverrides(txt string) (rookceph.ConfigOverridesSpec, error) { - overrides := []rookceph.ConfigOverride{} - - f, err := configFileTxtToINI(txt) - if err != nil { - return nil, err - } - sections := f.Sections() - for _, s := range sections { - who := s.Name() - keys := s.Keys() - for _, k := range keys { - option := k.Name() - value := k.String() - overrides = append(overrides, configOverride(who, option, value)) - } - } - - return overrides, nil -} - -func configFileTxtToINI(txt string) (*ini.File, error) { - f := ini.Empty() - ovrTxt := []byte(txt) - if err := f.Append(ovrTxt); err != nil { - return nil, fmt.Errorf("failed to interpret text as INI file '''%s'''. %+v", ovrTxt, err) - } - return f, nil -} - -func getOverrideConfigFromConfigMap(context *clusterd.Context, namespace string) string { - cm, err := context.Clientset.CoreV1().ConfigMaps(namespace). - Get(k8sutil.ConfigOverrideName, metav1.GetOptions{}) - if err != nil { - if errors.IsNotFound(err) { - logger.Debugf("override configmap does not exist") - return "" // return empty string if cannot get override text - } - logger.Warningf("Error getting override configmap. %+v", err) - return "" // return empty string if cannot get override text - } - // the override config map exists - o, ok := cm.Data[k8sutil.ConfigOverrideVal] - if !ok { - logger.Warningf("The override config map does not have override text defined. %+v", o) - return "" // return empty string if cannot get override text - } - return o -} From 9c1bb7c62d4d32f452af17b8db1df99260999179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 30 Aug 2019 17:24:26 +0200 Subject: [PATCH 2/4] ceph: external be a bit more flexible on upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add various upgrade tests so that the local cluster does not fail if an upgrade is available on the external cluster. Also, add better upgrades handling on update and on add. We now rely as much as possible on a unique function that works both local and external cluster 'detectAndValidateCephVersion()'. So the code will act the same way wether a CR update is triggered or if the operator is restarted. The health check now reports any topology changes on the external cluster such as odd number of mons and low number. It will also detect if the external cluster has been upgraded and will log that information, helping the admin in his/her decision to trigger an image upgrade. Finally, we prevent the creation of new CRs if the external cluster version has a higher major number than the local one. Closes: https://github.com/rook/rook/issues/3731 Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/cluster.go | 120 ++++++++++++++-------- pkg/operator/ceph/cluster/controller.go | 73 ++++++------- pkg/operator/ceph/cluster/mon/health.go | 24 +++++ pkg/operator/ceph/file/controller.go | 19 ++++ pkg/operator/ceph/object/controller.go | 18 ++++ pkg/operator/ceph/version/version.go | 41 ++++++++ pkg/operator/ceph/version/version_test.go | 33 ++++++ 7 files changed, 242 insertions(+), 86 deletions(-) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index e34212a78..93bdeaa85 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -36,6 +36,7 @@ import ( "github.com/rook/rook/pkg/operator/ceph/cluster/mon" "github.com/rook/rook/pkg/operator/ceph/cluster/osd" "github.com/rook/rook/pkg/operator/ceph/cluster/rbd" + "github.com/rook/rook/pkg/operator/ceph/config" cephver "github.com/rook/rook/pkg/operator/ceph/version" "github.com/rook/rook/pkg/operator/k8sutil" v1 "k8s.io/api/core/v1" @@ -124,17 +125,19 @@ func (c *cluster) detectCephVersion(rookImage, cephImage string, timeout time.Du } func (c *cluster) validateCephVersion(version *cephver.CephVersion) error { - if !version.IsAtLeast(cephver.Minimum) { - return fmt.Errorf("the version does not meet the minimum version: %s", cephver.Minimum.String()) - } + // The external has its own supported versions + if !c.Spec.External.Enable { + if !version.IsAtLeast(cephver.Minimum) { + return fmt.Errorf("the version does not meet the minimum version: %s", cephver.Minimum.String()) + } - if !version.Supported() { - logger.Warningf("unsupported ceph version detected: %s.", version) - if !c.Spec.CephVersion.AllowUnsupported { - return fmt.Errorf("allowUnsupported must be set to true to run with this version: %v", version) + if !version.Supported() { + logger.Warningf("unsupported ceph version detected: %s.", version) + if !c.Spec.CephVersion.AllowUnsupported { + return fmt.Errorf("allowUnsupported must be set to true to run with this version: %v", version) + } } } - // The following tries to determine if the operator can proceed with an upgrade because we come from an OnAdd() call // If the cluster was unhealthy and someone injected a new image version, an upgrade was triggered but failed because the cluster is not healthy // Then after this, if the operator gets restarted we are not able to fail if the cluster is not healthy, the following tries to determine the @@ -156,6 +159,18 @@ func (c *cluster) validateCephVersion(version *cephver.CephVersion) error { return nil } + if c.Spec.External.Enable { + externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.Namespace) + if err != nil { + return fmt.Errorf("failed to get ceph mon version. %+v", err) + } + c.Info.CephVersion = *externalCephMonVersion + err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(*version, c.Info.CephVersion) + if err != nil { + return fmt.Errorf("failed to validate ceph version between external and local. %+v", err) + } + } + // Get cluster running versions versions, err := client.GetAllCephDaemonVersions(c.context, c.Namespace) if err != nil { @@ -231,51 +246,66 @@ func (c *cluster) doOrchestration(rookImage string, cephVersion cephver.CephVers return fmt.Errorf("failed to create override configmap %s. %+v", c.Namespace, err) } - // This gets triggered on CR update so let's not run that (mon/mgr/osd daemons) - // Start the mon pods - clusterInfo, err := c.mons.Start(c.Info, rookImage, cephVersion, *c.Spec, c.isUpgrade) - if err != nil { - return fmt.Errorf("failed to start the mons. %+v", err) - } - c.Info = clusterInfo // mons return the cluster's info + if c.Spec.External.Enable { + // Apply CRD ConfigOverrideName to the external cluster + err = config.SetDefaultAndUserConfigs(c.context, c.Namespace, c.Info, c.Spec.ConfigOverrides) + if err != nil { + // Mons are up, so something else is wrong + return fmt.Errorf("failed to set Rook and/or user-defined Ceph config options on the external cluster monitors. %+v", err) + } - // The cluster Identity must be established at this point - if !c.Info.IsInitialized() { - return fmt.Errorf("the cluster identity was not established: %+v", c.Info) - } + // The cluster Identity must be established at this point + if !c.Info.IsInitialized() { + return fmt.Errorf("the cluster identity was not established: %+v", c.Info) + } + } else { + // This gets triggered on CR update so let's not run that (mon/mgr/osd daemons) + // Start the mon pods + clusterInfo, err := c.mons.Start(c.Info, rookImage, cephVersion, *c.Spec, c.isUpgrade) + if err != nil { + return fmt.Errorf("failed to start the mons. %+v", err) + } + c.Info = clusterInfo // mons return the cluster's info - mgrs := mgr.New(c.Info, c.context, c.Namespace, rookImage, - spec.CephVersion, cephv1.GetMgrPlacement(spec.Placement), cephv1.GetMgrAnnotations(c.Spec.Annotations), - spec.Network, spec.Dashboard, spec.Monitoring, cephv1.GetMgrResources(spec.Resources), c.ownerRef, c.Spec.DataDirHostPath, c.isUpgrade) - err = mgrs.Start() - if err != nil { - return fmt.Errorf("failed to start the ceph mgr. %+v", err) - } + // The cluster Identity must be established at this point + if !c.Info.IsInitialized() { + return fmt.Errorf("the cluster identity was not established: %+v", c.Info) + } - // Start the OSDs - osds := osd.New(c.Info, c.context, c.Namespace, rookImage, spec.CephVersion, spec.Storage, spec.DataDirHostPath, - cephv1.GetOSDPlacement(spec.Placement), cephv1.GetOSDAnnotations(spec.Annotations), spec.Network, - cephv1.GetOSDResources(spec.Resources), c.ownerRef, c.isUpgrade) - err = osds.Start() - if err != nil { - return fmt.Errorf("failed to start the osds. %+v", err) - } + mgrs := mgr.New(c.Info, c.context, c.Namespace, rookImage, + spec.CephVersion, cephv1.GetMgrPlacement(spec.Placement), cephv1.GetMgrAnnotations(c.Spec.Annotations), + spec.Network, spec.Dashboard, spec.Monitoring, cephv1.GetMgrResources(spec.Resources), c.ownerRef, c.Spec.DataDirHostPath, c.isUpgrade) + err = mgrs.Start() + if err != nil { + return fmt.Errorf("failed to start the ceph mgr. %+v", err) + } - // Start the rbd mirroring daemon(s) - rbdmirror := rbd.New(c.Info, c.context, c.Namespace, rookImage, spec.CephVersion, cephv1.GetRBDMirrorPlacement(spec.Placement), - cephv1.GetRBDMirrorAnnotations(spec.Annotations), spec.Network, spec.RBDMirroring, - cephv1.GetRBDMirrorResources(spec.Resources), c.ownerRef, c.Spec.DataDirHostPath, c.isUpgrade) - err = rbdmirror.Start() - if err != nil { - return fmt.Errorf("failed to start the rbd mirrors. %+v", err) - } + // Start the OSDs + osds := osd.New(c.Info, c.context, c.Namespace, rookImage, spec.CephVersion, spec.Storage, spec.DataDirHostPath, + cephv1.GetOSDPlacement(spec.Placement), cephv1.GetOSDAnnotations(spec.Annotations), spec.Network, + cephv1.GetOSDResources(spec.Resources), c.ownerRef, c.isUpgrade) + err = osds.Start() + if err != nil { + return fmt.Errorf("failed to start the osds. %+v", err) + } - logger.Infof("Done creating rook instance in namespace %s", c.Namespace) - c.initCompleted = true + // Start the rbd mirroring daemon(s) + rbdmirror := rbd.New(c.Info, c.context, c.Namespace, rookImage, spec.CephVersion, cephv1.GetRBDMirrorPlacement(spec.Placement), + cephv1.GetRBDMirrorAnnotations(spec.Annotations), spec.Network, spec.RBDMirroring, + cephv1.GetRBDMirrorResources(spec.Resources), c.ownerRef, c.Spec.DataDirHostPath, c.isUpgrade) + err = rbdmirror.Start() + if err != nil { + return fmt.Errorf("failed to start the rbd mirrors. %+v", err) + } + + logger.Infof("Done creating rook instance in namespace %s", c.Namespace) + c.initCompleted = true + } // Notify the child controllers that the cluster spec might have changed + logger.Debug("notifying CR childs of the potential upgrade") for _, child := range c.childControllers { - child.ParentClusterChanged(*c.Spec, clusterInfo, c.isUpgrade) + child.ParentClusterChanged(*c.Spec, c.Info, c.isUpgrade) } return nil diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index c6aff70dd..800eb2f06 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -30,6 +30,7 @@ import ( "github.com/rook/rook/pkg/clusterd" "github.com/rook/rook/pkg/daemon/ceph/agent/flexvolume/attachment" "github.com/rook/rook/pkg/daemon/ceph/client" + cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" discoverDaemon "github.com/rook/rook/pkg/daemon/discover" "github.com/rook/rook/pkg/operator/ceph/cluster/mon" "github.com/rook/rook/pkg/operator/ceph/cluster/osd" @@ -61,6 +62,7 @@ const ( updateClusterInterval = 30 * time.Second updateClusterTimeout = 1 * time.Hour detectCephVersionTimeout = 15 * time.Minute + externalConnectionRetry = 60 * time.Second ) const ( @@ -254,19 +256,14 @@ func (c *ClusterController) configureExternalCephCluster(namespace, name string, c.updateClusterStatus(namespace, name, cephv1.ClusterStateConnecting, "") // loop until we find the secret necessary to connect to the external cluster - for { - var err error - cluster.Info, _, _, err = mon.LoadClusterInfo(c.context, namespace) - if err != nil { - logger.Warningf("waiting for the connection info of the external cluster. %+v", err) - time.Sleep(10 * time.Second) - continue - } else { - break - } - } + // then populate clusterInfo + cluster.Info = populateExternalClusterInfo(c.context, namespace) - logger.Infof("found the cluster info to connect to the external cluster. mons=%+v", cluster.Info.Monitors) + // Validate versions (local and external) + _, _, err = c.detectAndValidateCephVersion(cluster, cluster.Spec.CephVersion.Image) + if err != nil { + return fmt.Errorf("failed to detect and validate ceph version. %+v", err) + } // Write the rook-ceph-config configmap (used by various daemons to apply config overrides) // If we don't do this, daemons will never start, waiting forever for this configmap to be present @@ -275,36 +272,11 @@ func (c *ClusterController) configureExternalCephCluster(namespace, name string, return err } - // Let's write connection info (ceph config file and keyring) to the operator for health checks - err = mon.WriteConnectionConfig(cluster.context, cluster.Info) + // Apply CRD ConfigOverrideName to the external cluster + err = config.SetDefaultAndUserConfigs(cluster.context, namespace, cluster.Info, cluster.Spec.ConfigOverrides) if err != nil { - return fmt.Errorf("failed to write connection info %+v", err) - } - - // Get Ceph monitors version on the external cluster - cephMonVersion, err := client.GetCephMonVersion(c.context, namespace) - if err != nil { - return fmt.Errorf("failed to get ceph mon version. %+v", err) - } - - // We only support Nautilus or newer - if !cephMonVersion.IsAtLeastNautilus() { - return fmt.Errorf("unsupported ceph version %s, need at least nautilus", cephMonVersion.String()) - } - - logger.Infof("detecting the image version provided for the external cluster...") - specCephVersionImage, err := cluster.detectCephVersion(c.rookImage, cluster.Spec.CephVersion.Image, detectCephVersionTimeout) - if err != nil { - return fmt.Errorf("unknown ceph major version. %+v", err) - } - specCephVersion := *specCephVersionImage - - // Populate clusterInfo with the external cluster version - cluster.Info.CephVersion = *cephMonVersion - - // Make sure the external cluster version and the ceph version in the provided image are identical - if !cephver.IsIdentical(specCephVersion, cluster.Info.CephVersion) { - return fmt.Errorf("wrong ceph version %s, external cluster version is %s, they must match", specCephVersion.String(), cephMonVersion.String()) + // Mons are up, so something else is wrong + return fmt.Errorf("failed to set Rook and/or user-defined Ceph config options on the external cluster monitors. %+v", err) } // The cluster Identity must be established at this point @@ -981,3 +953,22 @@ func validateExternalClusterSpec(cluster *cluster) error { return nil } + +// Add validation in the code to fail if the external cluster has no OSDs keep waiting +func populateExternalClusterInfo(context *clusterd.Context, namespace string) *cephconfig.ClusterInfo { + var clusterInfo *cephconfig.ClusterInfo + for { + var err error + clusterInfo, _, _, err = mon.LoadClusterInfo(context, namespace) + if err != nil { + logger.Warningf("waiting for the connection info of the external cluster. retrying in %s.", externalConnectionRetry.String()) + time.Sleep(externalConnectionRetry) + continue + } else { + logger.Infof("found the cluster info to connect to the external cluster. mons=%+v", clusterInfo.Monitors) + break + } + } + + return clusterInfo +} diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index e47eab881..5ab8a4194 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -25,6 +25,7 @@ import ( "github.com/rook/rook/pkg/daemon/ceph/client" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" cephutil "github.com/rook/rook/pkg/daemon/ceph/util" + cephver "github.com/rook/rook/pkg/operator/ceph/version" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -297,6 +298,20 @@ func removeMonitorFromQuorum(context *clusterd.Context, clusterName, name string func (c *Cluster) handleExternalMonStatus(status client.MonStatusResponse) error { + // health check should tell us if the external cluster has been upgraded and display a message + externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.Namespace) + if err != nil { + return fmt.Errorf("failed to get ceph mon version. %+v", err) + } + + err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.ClusterInfo.CephVersion, *externalCephMonVersion) + if err != nil { + // We only want to display a warning + // Any CR update will be caught earlier and proper error will be return + // This only helps displaying potential upgrade from the external cluster + logger.Warning(err) + } + changed, err := c.addOrRemoveExternalMonitor(status) if err != nil { return fmt.Errorf("failed to add or remove external mon. %+v", err) @@ -321,6 +336,15 @@ func (c *Cluster) addOrRemoveExternalMonitor(status client.MonStatusResponse) (b monsNotFound[mon.Name] = struct{}{} } + monCount := len(status.MonMap.Mons) + if monCount%2 == 0 { + logger.Warningf("external cluster mon count is even (%d), should be uneven, continuing.", monCount) + } + + if monCount == 1 { + logger.Warning("external cluster mon count is 1, consider adding new monitors.") + } + // Iterate over the mons first and compare it with ClusterInfo for _, mon := range status.MonMap.Mons { inQuorum := monInQuorum(mon, status.Quorum) diff --git a/pkg/operator/ceph/file/controller.go b/pkg/operator/ceph/file/controller.go index 976aea2df..3f9e52360 100644 --- a/pkg/operator/ceph/file/controller.go +++ b/pkg/operator/ceph/file/controller.go @@ -26,7 +26,9 @@ import ( opkit "github.com/rook/operator-kit" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/daemon/ceph/client" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" + cephver "github.com/rook/rook/pkg/operator/ceph/version" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" @@ -105,6 +107,23 @@ func (c *FilesystemController) onAdd(obj interface{}) { c.acquireOrchestrationLock() defer c.releaseOrchestrationLock() + if c.clusterSpec.External.Enable { + // health check should tell us if the external cluster has been upgraded and display a message + externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.namespace) + if err != nil { + logger.Errorf("failed to get ceph mon version. %+v", err) + return + } + + err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.clusterInfo.CephVersion, *externalCephMonVersion) + if err != nil { + // This handles the case where the operator is running, the external cluster has been upgraded and a CR creation is called + // If that's a major version upgrade we fail, if it's a minor version, we continue, it's not ideal but not critical + logger.Errorf("refusing to run new crd. %+v", err) + return + } + } + err = createFilesystem(c.clusterInfo, c.context, *filesystem, c.rookVersion, c.clusterSpec, c.filesystemOwners(filesystem), c.clusterSpec.DataDirHostPath, c.isUpgrade) if err != nil { logger.Errorf("failed to create filesystem %s: %+v", filesystem.Name, err) diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index 79baaef92..61679a5b6 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -25,8 +25,10 @@ import ( opkit "github.com/rook/operator-kit" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/daemon/ceph/client" daemonconfig "github.com/rook/rook/pkg/daemon/ceph/config" cephconfig "github.com/rook/rook/pkg/operator/ceph/config" + cephver "github.com/rook/rook/pkg/operator/ceph/version" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" @@ -104,6 +106,22 @@ func (c *ObjectStoreController) onAdd(obj interface{}) { c.acquireOrchestrationLock() defer c.releaseOrchestrationLock() + if c.clusterSpec.External.Enable { + // health check should tell us if the external cluster has been upgraded and display a message + externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.namespace) + if err != nil { + logger.Errorf("failed to get ceph mon version. %+v", err) + return + } + + err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.clusterInfo.CephVersion, *externalCephMonVersion) + if err != nil { + // This handles the case where the operator is running, the external cluster has been upgraded and a CR creation is called + // If that's a major version upgrade we fail, if it's a minor version, we continue, it's not ideal but not critical + logger.Errorf("refusing to run new crd. %+v", err) + return + } + } c.createOrUpdateStore(objectstore) } diff --git a/pkg/operator/ceph/version/version.go b/pkg/operator/ceph/version/version.go index c95a06033..f7bc9483c 100644 --- a/pkg/operator/ceph/version/version.go +++ b/pkg/operator/ceph/version/version.go @@ -20,6 +20,8 @@ import ( "fmt" "regexp" "strconv" + + "github.com/coreos/pkg/capnslog" ) // CephVersion represents the Ceph version format @@ -51,6 +53,8 @@ var ( // for parsing the output of `ceph --version` versionPattern = regexp.MustCompile(`ceph version (\d+)\.(\d+)\.(\d+)`) + + logger = capnslog.NewPackageLogger("github.com/rook/rook", "cephver") ) func (v *CephVersion) String() string { @@ -214,3 +218,40 @@ func IsInferior(a, b CephVersion) bool { return false } + +// ValidateCephVersionsBetweenLocalAndExternalClusters makes sure an external cluster can be connected +// by checking the external ceph versions available and comparing it with the local image provided +func ValidateCephVersionsBetweenLocalAndExternalClusters(localVersion, externalVersion CephVersion) error { + logger.Debugf("local version is %s, external version is %s", localVersion.String(), externalVersion.String()) + + // We only support Nautilus or newer + if !externalVersion.IsAtLeastNautilus() { + return fmt.Errorf("unsupported ceph version %s, need at least nautilus, delete your cluster CR and create a new one with a correct ceph version", externalVersion.String()) + } + + // Identical version, regardless if other CRs are running, it's ok! + if IsIdentical(localVersion, externalVersion) { + return nil + } + + // Local version must never be higher than the external one + if IsSuperior(localVersion, externalVersion) { + return fmt.Errorf("local cluster ceph version is higher %s than the external cluster %s, this must never happen", externalVersion.String(), localVersion.String()) + } + + // External cluster was updated to a minor version higher, consider updating too! + if localVersion.Major == externalVersion.Major { + if IsSuperior(externalVersion, localVersion) { + logger.Warningf("external cluster ceph version is a minor version higher %s than the local cluster %s, consider upgrading", externalVersion.String(), localVersion.String()) + return nil + } + } + + // The external cluster was upgraded, consider upgrading too! + if localVersion.Major < externalVersion.Major { + logger.Errorf("external cluster ceph version is a major version higher %s than the local cluster %s, consider upgrading", externalVersion.String(), localVersion.String()) + return nil + } + + return nil +} diff --git a/pkg/operator/ceph/version/version_test.go b/pkg/operator/ceph/version/version_test.go index 0bd750dbb..92220311a 100644 --- a/pkg/operator/ceph/version/version_test.go +++ b/pkg/operator/ceph/version/version_test.go @@ -175,3 +175,36 @@ func TestIsInferior(t *testing.T) { assert.True(t, IsInferior(CephVersion{15, 1, 3}, CephVersion{15, 2, 2})) assert.True(t, IsInferior(CephVersion{15, 2, 1}, CephVersion{15, 2, 2})) } + +func TestValidateCephVersionsBetweenLocalAndExternalClusters(t *testing.T) { + // TEST 1: versions are identical + localCephVersion := CephVersion{Major: 14, Minor: 2, Extra: 1} + externalCephVersion := CephVersion{Major: 14, Minor: 2, Extra: 1} + err := ValidateCephVersionsBetweenLocalAndExternalClusters(localCephVersion, externalCephVersion) + assert.NoError(t, err) + + // TEST 2: local cluster version major is lower than external cluster version + localCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 1} + externalCephVersion = CephVersion{Major: 15, Minor: 2, Extra: 1} + err = ValidateCephVersionsBetweenLocalAndExternalClusters(localCephVersion, externalCephVersion) + assert.NoError(t, err) + + // TEST 3: local cluster version major is higher than external cluster version + localCephVersion = CephVersion{Major: 15, Minor: 2, Extra: 1} + externalCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 1} + err = ValidateCephVersionsBetweenLocalAndExternalClusters(localCephVersion, externalCephVersion) + assert.Error(t, err) + + // TEST 4: local version is > but from a minor release + // local version must never be higher + localCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 2} + externalCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 1} + err = ValidateCephVersionsBetweenLocalAndExternalClusters(localCephVersion, externalCephVersion) + assert.Error(t, err) + + // TEST 5: external version is > but from a minor release + localCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 1} + externalCephVersion = CephVersion{Major: 14, Minor: 2, Extra: 2} + err = ValidateCephVersionsBetweenLocalAndExternalClusters(localCephVersion, externalCephVersion) + assert.NoError(t, err) +} From bc1a5a90371b6995c6df543da98d081719a1a001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 4 Sep 2019 12:12:43 +0200 Subject: [PATCH 3/4] ceph: external create 'rook-config-override' cm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This configmap is needed by RGW and MDS pods, if it's not present we won't be able to start any of them. Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/cluster.go | 19 +++--------------- pkg/operator/ceph/cluster/controller.go | 26 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 93bdeaa85..4b5735c6b 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -38,9 +38,6 @@ import ( "github.com/rook/rook/pkg/operator/ceph/cluster/rbd" "github.com/rook/rook/pkg/operator/ceph/config" cephver "github.com/rook/rook/pkg/operator/ceph/version" - "github.com/rook/rook/pkg/operator/k8sutil" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -231,19 +228,9 @@ func (c *cluster) createInstance(rookImage string, cephVersion cephver.CephVersi func (c *cluster) doOrchestration(rookImage string, cephVersion cephver.CephVersion, spec *cephv1.ClusterSpec) error { // Create a configmap for overriding ceph config settings // These settings should only be modified by a user after they are initialized - placeholderConfig := map[string]string{ - k8sutil.ConfigOverrideVal: "", - } - cm := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: k8sutil.ConfigOverrideName, - }, - Data: placeholderConfig, - } - k8sutil.SetOwnerRef(&cm.ObjectMeta, &c.ownerRef) - _, err := c.context.Clientset.CoreV1().ConfigMaps(c.Namespace).Create(cm) - if err != nil && !errors.IsAlreadyExists(err) { - return fmt.Errorf("failed to create override configmap %s. %+v", c.Namespace, err) + err := populateConfigOverrideConfigMap(c.context, c.Namespace, c.ownerRef) + if err != nil { + return fmt.Errorf("failed to populate config override config map. %+v", err) } if c.Spec.External.Enable { diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index 800eb2f06..397c36b47 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -265,6 +265,11 @@ func (c *ClusterController) configureExternalCephCluster(namespace, name string, return fmt.Errorf("failed to detect and validate ceph version. %+v", err) } + err = populateConfigOverrideConfigMap(cluster.context, namespace, cluster.ownerRef) + if err != nil { + return fmt.Errorf("failed to populate config override config map. %+v", err) + } + // Write the rook-ceph-config configmap (used by various daemons to apply config overrides) // If we don't do this, daemons will never start, waiting forever for this configmap to be present err = config.GetStore(cluster.context, namespace, &cluster.ownerRef).CreateOrUpdate(cluster.Info) @@ -972,3 +977,24 @@ func populateExternalClusterInfo(context *clusterd.Context, namespace string) *c return clusterInfo } + +func populateConfigOverrideConfigMap(context *clusterd.Context, namespace string, ownerRef metav1.OwnerReference) error { + placeholderConfig := map[string]string{ + k8sutil.ConfigOverrideVal: "", + } + + cm := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: k8sutil.ConfigOverrideName, + }, + Data: placeholderConfig, + } + + k8sutil.SetOwnerRef(&cm.ObjectMeta, &ownerRef) + _, err := context.Clientset.CoreV1().ConfigMaps(namespace).Create(cm) + if err != nil && !errors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create override configmap %s. %+v", namespace, err) + } + + return nil +} From 533ac5b693b266714ef89a824ce41c656287f5af Mon Sep 17 00:00:00 2001 From: travisn Date: Wed, 4 Sep 2019 19:02:05 -0600 Subject: [PATCH 4/4] ceph: refactor version detection of external cluster Reduce the duplication of detecting the version of the external cluster by refactoring to a single method that retrieves and validates the version Signed-off-by: travisn --- pkg/operator/ceph/cluster/cluster.go | 8 ++---- pkg/operator/ceph/cluster/mon/health.go | 16 ++--------- pkg/operator/ceph/file/controller.go | 12 ++------ pkg/operator/ceph/object/controller.go | 12 ++------ pkg/operator/ceph/spec/version.go | 37 +++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 39 deletions(-) create mode 100644 pkg/operator/ceph/spec/version.go diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 4b5735c6b..b064a5789 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -37,6 +37,7 @@ import ( "github.com/rook/rook/pkg/operator/ceph/cluster/osd" "github.com/rook/rook/pkg/operator/ceph/cluster/rbd" "github.com/rook/rook/pkg/operator/ceph/config" + cephspec "github.com/rook/rook/pkg/operator/ceph/spec" cephver "github.com/rook/rook/pkg/operator/ceph/version" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -157,12 +158,7 @@ func (c *cluster) validateCephVersion(version *cephver.CephVersion) error { } if c.Spec.External.Enable { - externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.Namespace) - if err != nil { - return fmt.Errorf("failed to get ceph mon version. %+v", err) - } - c.Info.CephVersion = *externalCephMonVersion - err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(*version, c.Info.CephVersion) + c.Info.CephVersion, err = cephspec.ValidateCephVersionsBetweenLocalAndExternalClusters(c.context, c.Namespace, *version) if err != nil { return fmt.Errorf("failed to validate ceph version between external and local. %+v", err) } diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index 5ab8a4194..d5cebbe1d 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -25,7 +25,7 @@ import ( "github.com/rook/rook/pkg/daemon/ceph/client" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" cephutil "github.com/rook/rook/pkg/daemon/ceph/util" - cephver "github.com/rook/rook/pkg/operator/ceph/version" + cephspec "github.com/rook/rook/pkg/operator/ceph/spec" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -297,19 +297,9 @@ func removeMonitorFromQuorum(context *clusterd.Context, clusterName, name string } func (c *Cluster) handleExternalMonStatus(status client.MonStatusResponse) error { - - // health check should tell us if the external cluster has been upgraded and display a message - externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.Namespace) + _, err := cephspec.ValidateCephVersionsBetweenLocalAndExternalClusters(c.context, c.Namespace, c.ClusterInfo.CephVersion) if err != nil { - return fmt.Errorf("failed to get ceph mon version. %+v", err) - } - - err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.ClusterInfo.CephVersion, *externalCephMonVersion) - if err != nil { - // We only want to display a warning - // Any CR update will be caught earlier and proper error will be return - // This only helps displaying potential upgrade from the external cluster - logger.Warning(err) + return fmt.Errorf("failed to validate external ceph version. %+v", err) } changed, err := c.addOrRemoveExternalMonitor(status) diff --git a/pkg/operator/ceph/file/controller.go b/pkg/operator/ceph/file/controller.go index 3f9e52360..175c22cf3 100644 --- a/pkg/operator/ceph/file/controller.go +++ b/pkg/operator/ceph/file/controller.go @@ -26,9 +26,8 @@ import ( opkit "github.com/rook/operator-kit" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" - "github.com/rook/rook/pkg/daemon/ceph/client" cephconfig "github.com/rook/rook/pkg/daemon/ceph/config" - cephver "github.com/rook/rook/pkg/operator/ceph/version" + cephspec "github.com/rook/rook/pkg/operator/ceph/spec" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" @@ -108,14 +107,7 @@ func (c *FilesystemController) onAdd(obj interface{}) { defer c.releaseOrchestrationLock() if c.clusterSpec.External.Enable { - // health check should tell us if the external cluster has been upgraded and display a message - externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.namespace) - if err != nil { - logger.Errorf("failed to get ceph mon version. %+v", err) - return - } - - err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.clusterInfo.CephVersion, *externalCephMonVersion) + _, err := cephspec.ValidateCephVersionsBetweenLocalAndExternalClusters(c.context, c.namespace, c.clusterInfo.CephVersion) if err != nil { // This handles the case where the operator is running, the external cluster has been upgraded and a CR creation is called // If that's a major version upgrade we fail, if it's a minor version, we continue, it's not ideal but not critical diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index 61679a5b6..4fe8ad614 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -25,10 +25,9 @@ import ( opkit "github.com/rook/operator-kit" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" - "github.com/rook/rook/pkg/daemon/ceph/client" daemonconfig "github.com/rook/rook/pkg/daemon/ceph/config" cephconfig "github.com/rook/rook/pkg/operator/ceph/config" - cephver "github.com/rook/rook/pkg/operator/ceph/version" + cephspec "github.com/rook/rook/pkg/operator/ceph/spec" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" @@ -107,14 +106,7 @@ func (c *ObjectStoreController) onAdd(obj interface{}) { defer c.releaseOrchestrationLock() if c.clusterSpec.External.Enable { - // health check should tell us if the external cluster has been upgraded and display a message - externalCephMonVersion, err := client.GetCephMonVersion(c.context, c.namespace) - if err != nil { - logger.Errorf("failed to get ceph mon version. %+v", err) - return - } - - err = cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(c.clusterInfo.CephVersion, *externalCephMonVersion) + _, err := cephspec.ValidateCephVersionsBetweenLocalAndExternalClusters(c.context, c.namespace, c.clusterInfo.CephVersion) if err != nil { // This handles the case where the operator is running, the external cluster has been upgraded and a CR creation is called // If that's a major version upgrade we fail, if it's a minor version, we continue, it's not ideal but not critical diff --git a/pkg/operator/ceph/spec/version.go b/pkg/operator/ceph/spec/version.go new file mode 100644 index 000000000..4ad36a4a8 --- /dev/null +++ b/pkg/operator/ceph/spec/version.go @@ -0,0 +1,37 @@ +/* +Copyright 2018 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package spec + +import ( + "fmt" + + "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/daemon/ceph/client" + cephver "github.com/rook/rook/pkg/operator/ceph/version" +) + +// ValidateCephVersionsBetweenLocalAndExternalClusters makes sure an external cluster can be connected +// by checking the external ceph versions available and comparing it with the local image provided +func ValidateCephVersionsBetweenLocalAndExternalClusters(context *clusterd.Context, namespace string, localVersion cephver.CephVersion) (cephver.CephVersion, error) { + // health check should tell us if the external cluster has been upgraded and display a message + externalVersion, err := client.GetCephMonVersion(context, namespace) + if err != nil { + return cephver.CephVersion{}, fmt.Errorf("failed to get ceph mon version. %+v", err) + } + + return *externalVersion, cephver.ValidateCephVersionsBetweenLocalAndExternalClusters(localVersion, *externalVersion) +}