From 9efce968a096b00a27d37c945697233752088719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 10 Dec 2019 14:59:14 +0100 Subject: [PATCH 1/5] ceph: expose build number on ceph version label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pod label now shows the build number too. Signed-off-by: Sébastien Han --- pkg/operator/ceph/spec/label.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/operator/ceph/spec/label.go b/pkg/operator/ceph/spec/label.go index 749da7009..3a95e4931 100644 --- a/pkg/operator/ceph/spec/label.go +++ b/pkg/operator/ceph/spec/label.go @@ -38,8 +38,8 @@ const ( func addCephVersionLabel(cephVersion version.CephVersion, labels map[string]string) { // cephVersion.String() returns a string with a space in it, and labels in k8s are limited to // alphanum characters plus '-', '_', '.' - labels[CephVersionLabelKey] = fmt.Sprintf("%d.%d.%d", - cephVersion.Major, cephVersion.Minor, cephVersion.Extra) + labels[CephVersionLabelKey] = fmt.Sprintf("%d.%d.%d-%d", + cephVersion.Major, cephVersion.Minor, cephVersion.Extra, cephVersion.Build) } // AddCephVersionLabelToDeployment adds a label reporting the Ceph version which Rook has detected is From e91cf1ac1df576d98597611eab8a21c638a34a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 10 Dec 2019 15:00:09 +0100 Subject: [PATCH 2/5] ceph: crash retry in seconds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop wasn't using seconds for its retry so the crash controller would simply crash since the version is used later in the pod spec. Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/crash/reconcile.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/operator/ceph/cluster/crash/reconcile.go b/pkg/operator/ceph/cluster/crash/reconcile.go index 8e1f20bf1..c04e58a08 100644 --- a/pkg/operator/ceph/cluster/crash/reconcile.go +++ b/pkg/operator/ceph/cluster/crash/reconcile.go @@ -178,9 +178,10 @@ func getImageVersion(image string) (*version.CephVersion, bool) { for i := 0; i < getVersionMaxRetries; i++ { cephVersion, ok := version.GetImageVersion(image) if ok { + logger.Debugf("ceph version found %+v", cephVersion) return cephVersion, true } - <-time.After(getVersionRetryInterval) + <-time.After(time.Second * getVersionRetryInterval) } return nil, false } From b26c614ecf42840dcf24b6c4f19db7b626ca534f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 10 Dec 2019 15:01:21 +0100 Subject: [PATCH 3/5] ceph: crash do not use selector = label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We must not default the selectors to the labels since labels can be changed and selector cannot until the deployment is updated. This way the deployment was failing with: 2019-12-10 10:48:13.414438 E | ceph-crashcollector-controller: node reconcile failed on op "unchanged": Deployment.apps "rook-ceph-crashcollector-minikube" is invalid: spec.template.metadata.labels: Invalid value: map[string]string{"app":"rook-ceph-crashcollector", "ceph-version":"15.0.0", "ceph_daemon_id":"crash", "crashcollector":"crash", "kubernetes.io/hostname":"minikube", "node_name":"minikube", "rook-version":"v1.2.0-3.g8d3b85306.dirty"}: `selector` does not match template `labels` Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/crash/crash.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/operator/ceph/cluster/crash/crash.go b/pkg/operator/ceph/cluster/crash/crash.go index 7d545d86f..56db52389 100644 --- a/pkg/operator/ceph/cluster/crash/crash.go +++ b/pkg/operator/ceph/cluster/crash/crash.go @@ -76,13 +76,19 @@ func (r *ReconcileNode) createOrUpdateCephCrash(node corev1.Node, tolerations [] deploymentLabels[string(config.CrashType)] = "crash" deploymentLabels["ceph_daemon_id"] = "crash" + selectorLabels := map[string]string{ + corev1.LabelHostname: nodeHostnameLabel, + k8sutil.AppAttr: AppName, + NodeNameLabel: node.GetName(), + } + nodeSelector := map[string]string{corev1.LabelHostname: nodeHostnameLabel} // Deployment selector is immutable so we set this value only if // a new object is going to be created if deploy.ObjectMeta.CreationTimestamp.IsZero() { deploy.Spec.Selector = &metav1.LabelSelector{ - MatchLabels: deploymentLabels, + MatchLabels: selectorLabels, } } From 6051a3ca262ebe4caffdae14d5163e2019d3e388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 10 Dec 2019 15:03:37 +0100 Subject: [PATCH 4/5] ceph: controller accept multiple changes at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If editing the cluster CR we must update the spec to reflect its new content. Now the spec is updated so we can update with an unsupported image as well as setting allowUnsupported: true. It was not possible before we had to restart the operator, now we can do it as an update. Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/cluster.go | 6 +++--- pkg/operator/ceph/cluster/controller.go | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 41dab32ea..eb073e361 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -122,21 +122,21 @@ func (c *cluster) detectCephVersion(rookImage, cephImage string, timeout time.Du if err != nil { return nil, errors.Wrapf(err, "failed to extract ceph version") } - logger.Infof("Detected ceph image version: %s", version) + logger.Infof("Detected ceph image version: %q", version) return version, nil } func (c *cluster) validateCephVersion(version *cephver.CephVersion) error { if !c.Spec.External.Enable { if !version.IsAtLeast(cephver.Minimum) { - return errors.Errorf("the version does not meet the minimum version: %s", cephver.Minimum.String()) + return errors.Errorf("the version does not meet the minimum version: %q", cephver.Minimum.String()) } if !version.Supported() { - logger.Warningf("unsupported ceph version detected: %s.", version) if !c.Spec.CephVersion.AllowUnsupported { return errors.Errorf("allowUnsupported must be set to true to run with this version: %+v", version) } + logger.Warningf("unsupported ceph version detected: %q, pursuing", version) } } diff --git a/pkg/operator/ceph/cluster/controller.go b/pkg/operator/ceph/cluster/controller.go index 1be9e5533..41a7271b7 100644 --- a/pkg/operator/ceph/cluster/controller.go +++ b/pkg/operator/ceph/cluster/controller.go @@ -594,6 +594,11 @@ func (c *ClusterController) onUpdate(oldObj, newObj interface{}) { c.osdChecker.Update(newClust.Spec.RemoveOSDsIfOutAndSafeToRemove) } + logger.Debugf("old cluster: %+v", oldClust.Spec) + logger.Debugf("new cluster: %+v", newClust.Spec) + + cluster.Spec = &newClust.Spec + // if the image changed, we need to detect the new image version versionChanged := false if oldClust.Spec.CephVersion.Image != newClust.Spec.CephVersion.Image { @@ -607,11 +612,6 @@ func (c *ClusterController) onUpdate(oldObj, newObj interface{}) { cluster.Info.CephVersion = *version } - logger.Debugf("old cluster: %+v", oldClust.Spec) - logger.Debugf("new cluster: %+v", newClust.Spec) - - cluster.Spec = &newClust.Spec - // Get cluster running versions versions, err := client.GetAllCephDaemonVersions(c.context, cluster.Namespace) if err != nil { From f56d61856828cae2bfc8258eecb36f5107205370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Wed, 11 Dec 2019 10:34:33 +0100 Subject: [PATCH 5/5] ceph: add missing ownerref to csi secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSI secrets were missing OwnerReference so when csi went away secrets were remaining. Now this is fixed. Signed-off-by: Sébastien Han --- pkg/operator/ceph/csi/secrets.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/operator/ceph/csi/secrets.go b/pkg/operator/ceph/csi/secrets.go index e33f34850..a1bc30eeb 100644 --- a/pkg/operator/ceph/csi/secrets.go +++ b/pkg/operator/ceph/csi/secrets.go @@ -107,7 +107,7 @@ func cephCSIKeyringCephFSProvisionerCaps() []string { } } -func createOrUpdateCSISecret(namespace, csiRBDProvisionerSecretKey, csiRBDNodeSecretKey, csiCephFSProvisionerSecretKey, csiCephFSNodeSecretKey string, k *keyring.SecretStore) error { +func createOrUpdateCSISecret(namespace, csiRBDProvisionerSecretKey, csiRBDNodeSecretKey, csiCephFSProvisionerSecretKey, csiCephFSNodeSecretKey string, k *keyring.SecretStore, ownerRef *metav1.OwnerReference) error { csiRBDProvisionerSecrets := map[string][]byte{ // userID is expected for the rbd provisioner driver "userID": []byte("csi-rbd-provisioner"), @@ -147,6 +147,7 @@ func createOrUpdateCSISecret(namespace, csiRBDProvisionerSecretKey, csiRBDNodeSe Data: secret, Type: k8sutil.RookType, } + k8sutil.SetOwnerRef(&s.ObjectMeta, ownerRef) // Create Kubernetes Secret err := k.CreateSecret(s) @@ -189,7 +190,7 @@ func CreateCSISecrets(context *clusterd.Context, clusterName string, ownerRef *m } // Create or update Kubernetes CSI secret - if err := createOrUpdateCSISecret(clusterName, csiRBDProvisionerSecretKey, csiRBDNodeSecretKey, csiCephFSProvisionerSecretKey, csiCephFSNodeSecretKey, k); err != nil { + if err := createOrUpdateCSISecret(clusterName, csiRBDProvisionerSecretKey, csiRBDNodeSecretKey, csiCephFSProvisionerSecretKey, csiCephFSNodeSecretKey, k, ownerRef); err != nil { return errors.Wrapf(err, "failed to create kubernetes csi secret") }