From ecef80eda5f0cd053561eaac24393bbb50a7478a Mon Sep 17 00:00:00 2001 From: Rohan CJ Date: Fri, 27 Mar 2020 20:24:05 +0530 Subject: [PATCH 1/2] Ceph: Use both "apps" and "extensions" for deployment RBAC. Some versions of k8s can optionally enable extensions after deprecation. This could lead to rbac denials when trying to perform operations on deployments Signed-off-by: Rohan CJ --- .../charts/rook-ceph/templates/clusterrole.yaml | 15 ++++----------- cluster/charts/rook-ceph/templates/role.yaml | 14 +++----------- cluster/examples/kubernetes/ceph/common.yaml | 16 +++++----------- tests/framework/installer/ceph_manifests.go | 16 +++++----------- 4 files changed, 17 insertions(+), 44 deletions(-) diff --git a/cluster/charts/rook-ceph/templates/clusterrole.yaml b/cluster/charts/rook-ceph/templates/clusterrole.yaml index f1120b862..f5ba5bb9c 100644 --- a/cluster/charts/rook-ceph/templates/clusterrole.yaml +++ b/cluster/charts/rook-ceph/templates/clusterrole.yaml @@ -24,29 +24,21 @@ metadata: rules: - apiGroups: - "" + - apps + - extensions resources: - secrets - pods - pods/log - services - configmaps - verbs: - - get - - list - - watch - - patch - - create - - update - - delete -- apiGroups: - - apps - resources: - deployments - daemonsets verbs: - get - list - watch + - patch - create - update - delete @@ -137,6 +129,7 @@ rules: - apiGroups: - policy - apps + - extensions resources: # This is for the clusterdisruption controller - poddisruptionbudgets diff --git a/cluster/charts/rook-ceph/templates/role.yaml b/cluster/charts/rook-ceph/templates/role.yaml index 84743d729..66de05a8b 100644 --- a/cluster/charts/rook-ceph/templates/role.yaml +++ b/cluster/charts/rook-ceph/templates/role.yaml @@ -10,21 +10,12 @@ metadata: rules: - apiGroups: - "" + - apps + - extensions resources: - pods - configmaps - services - verbs: - - get - - list - - watch - - patch - - create - - update - - delete -- apiGroups: - - apps - resources: - daemonsets - statefulsets - deployment @@ -32,6 +23,7 @@ rules: - get - list - watch + - patch - create - update - delete diff --git a/cluster/examples/kubernetes/ceph/common.yaml b/cluster/examples/kubernetes/ceph/common.yaml index 3c831be8f..03d169121 100644 --- a/cluster/examples/kubernetes/ceph/common.yaml +++ b/cluster/examples/kubernetes/ceph/common.yaml @@ -645,29 +645,21 @@ metadata: rules: - apiGroups: - "" + - apps + - extensions resources: - secrets - pods - pods/log - services - configmaps - verbs: - - get - - list - - watch - - patch - - create - - update - - delete -- apiGroups: - - apps - resources: - deployments - daemonsets verbs: - get - list - watch + - patch - create - update - delete @@ -698,6 +690,7 @@ rules: - delete - apiGroups: - apps + - extensions resources: - daemonsets - statefulsets @@ -796,6 +789,7 @@ rules: - apiGroups: - policy - apps + - extensions resources: # This is for the clusterdisruption controller - poddisruptionbudgets diff --git a/tests/framework/installer/ceph_manifests.go b/tests/framework/installer/ceph_manifests.go index c5f77d938..0f4b4462b 100644 --- a/tests/framework/installer/ceph_manifests.go +++ b/tests/framework/installer/ceph_manifests.go @@ -658,21 +658,12 @@ metadata: rules: - apiGroups: - "" + - apps + - extensions resources: - pods - configmaps - services - verbs: - - get - - list - - watch - - patch - - create - - update - - delete -- apiGroups: - - apps - resources: - daemonsets - statefulsets - deployments @@ -680,6 +671,7 @@ rules: - get - list - watch + - patch - create - update - delete @@ -724,6 +716,7 @@ rules: - delete - apiGroups: - apps + - extensions resources: - deployments - daemonsets @@ -817,6 +810,7 @@ rules: - apiGroups: - policy - apps + - extensions resources: # This is for the clusterdisruption controller - poddisruptionbudgets From 03e92da102226072dbcd2136bd609e437e553f27 Mon Sep 17 00:00:00 2001 From: Rohan CJ Date: Fri, 27 Mar 2020 23:07:01 +0530 Subject: [PATCH 2/2] Ceph: handle not-found error on delete. Previously, delete was returning an error causing the drain-canary to reconcile even though it no longer existed. Signed-off-by: Rohan CJ --- pkg/operator/ceph/disruption/nodedrain/reconcile.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/operator/ceph/disruption/nodedrain/reconcile.go b/pkg/operator/ceph/disruption/nodedrain/reconcile.go index 8b0b26896..2b2492d26 100644 --- a/pkg/operator/ceph/disruption/nodedrain/reconcile.go +++ b/pkg/operator/ceph/disruption/nodedrain/reconcile.go @@ -92,7 +92,9 @@ func (r *ReconcileNode) reconcile(request reconcile.Request) (reconcile.Result, // delete any canary deployments if the node doesn't exist logger.Infof("deleting drain-canary deployment for deleted node %q", request.Name) err = r.client.Delete(context.TODO(), deploy) - if err != nil { + if kerrors.IsNotFound(err) { + return reconcile.Result{}, nil + } else if err != nil { return reconcile.Result{}, errors.Wrapf(err, "could not delete drain-canary deployment for deleted node %q", request.Name) } return reconcile.Result{}, nil