Skip to content

Commit

Permalink
feat(teamrbac): show failing namespaces (#669)
Browse files Browse the repository at this point in the history 
reconciliation failures for rolebindings did not mention
the namespace. Now events and PropagationStatus will contain
the namespace where reconciliation failed
  • Loading branch information
@IvoGoman
IvoGoman authored Oct 29, 2024
1 parent 31f4d46 commit 6c50e67
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
26 changes: 12 additions & 14 deletions pkg/controllers/teamrbac/teamrolebinding_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ func (r *TeamRoleBindingReconciler) doReconcile(ctx context.Context, teamRole *g
remoteRestClient, err := clientutil.NewK8sClientFromCluster(ctx, r.Client, &cluster)
if err != nil {
r.recorder.Eventf(trb, corev1.EventTypeWarning, greenhousev1alpha1.FailedEvent, "Error getting client for cluster %s to replicate %s", cluster.GetName(), trb.GetName())
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.ClusterConnectionFailed)
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.ClusterConnectionFailed, err.Error())
failedClusters = append(failedClusters, cluster.GetName())
continue
}

if err := reconcileClusterRole(ctx, remoteRestClient, &cluster, cr); err != nil {
r.recorder.Eventf(trb, corev1.EventTypeWarning, greenhousev1alpha1.FailedEvent, "Failed to reconcile ClusterRole %s in cluster %s", cr.GetName(), cluster.GetName())
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.ClusterRoleFailed)
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.ClusterRoleFailed, err.Error())
failedClusters = append(failedClusters, cluster.GetName())
continue
}
Expand All @@ -176,30 +176,28 @@ func (r *TeamRoleBindingReconciler) doReconcile(ctx context.Context, teamRole *g
crb := rbacClusterRoleBinding(trb, cr, team)
if err := reconcileClusterRoleBinding(ctx, remoteRestClient, &cluster, crb); err != nil {
r.recorder.Eventf(trb, corev1.EventTypeWarning, greenhousev1alpha1.FailedEvent, "Failed to reconcile ClusterRoleBinding %s in cluster %s", crb.GetName(), cluster.GetName())
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.RoleBindingFailed)
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.RoleBindingFailed, err.Error())
failedClusters = append(failedClusters, cluster.GetName())
continue
}
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionTrue, greenhousev1alpha1.RBACReconciled)
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionTrue, greenhousev1alpha1.RBACReconciled, "")
default:
hasFailed := false
errorMesages := []string{}
for _, namespace := range trb.Spec.Namespaces {
rbacRoleBinding := rbacRoleBinding(trb, cr, team, namespace)

if err := reconcileRoleBinding(ctx, remoteRestClient, &cluster, rbacRoleBinding); err != nil {
r.recorder.Eventf(trb, corev1.EventTypeWarning, greenhousev1alpha1.FailedEvent, "Failed to reconcile RoleBinding %s in cluster %s", rbacRoleBinding.GetName(), cluster.GetName())
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.RoleBindingFailed)
if !hasFailed {
hasFailed = true
}
r.recorder.Eventf(trb, corev1.EventTypeWarning, greenhousev1alpha1.FailedEvent, "Failed to reconcile RoleBinding %s in cluster/namespace %s/%s: ", rbacRoleBinding.GetName(), cluster.GetName(), namespace)
failedClusters = append(failedClusters, cluster.GetName())
errorMesages = append(errorMesages, err.Error())
}
}
if hasFailed {
if len(errorMesages) > 0 {
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionFalse, greenhousev1alpha1.RoleBindingFailed, "Failed to reconcile RoleBindings: "+strings.Join(errorMesages, ", "))
continue
}
}
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionTrue, greenhousev1alpha1.RBACReconciled)
setPropagationStatus(&trbStatus, cluster.GetName(), metav1.ConditionTrue, greenhousev1alpha1.RBACReconciled, "")
}

if len(failedClusters) > 0 {
Expand Down Expand Up @@ -709,9 +707,9 @@ func computeReadyCondition(status greenhousev1alpha1.TeamRoleBindingStatus) gree
}

// setPropagationStatus updates the TeamRoleBinding's PropagationStatus for the Cluster
func setPropagationStatus(status *greenhousev1alpha1.TeamRoleBindingStatus, cluster string, rbacReady metav1.ConditionStatus, reason greenhousev1alpha1.ConditionReason) {
func setPropagationStatus(status *greenhousev1alpha1.TeamRoleBindingStatus, cluster string, rbacReady metav1.ConditionStatus, reason greenhousev1alpha1.ConditionReason, message string) {
exists := false
condition := greenhousev1alpha1.NewCondition(greenhousev1alpha1.RBACReady, rbacReady, reason, "")
condition := greenhousev1alpha1.NewCondition(greenhousev1alpha1.RBACReady, rbacReady, reason, message)
for i, ps := range status.PropagationStatus {
if ps.ClusterName == cluster {
if ps.Condition.Status == rbacReady {
Expand Down
52 changes: 52 additions & 0 deletions pkg/controllers/teamrbac/teamrolebinding_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,58 @@ var _ = Describe("Validate ClusterRole & RoleBinding on Remote Cluster", Ordered
})
})

Context("When creating a Greenhouse TeamRoleBinding with non-existing namespaces on the central cluster", func() {
It("Should fail to create ClusterRole and RoleBinding on the remote cluster", func() {
By("creating a TeamRoleBinding on the central cluster")
trb := setup.CreateTeamRoleBinding(test.Ctx, "test-rolebinding",
test.WithTeamRoleRef(teamRoleUT.Name),
test.WithTeamRef(teamUT.Name),
test.WithClusterName(clusterA.Name),
test.WithNamespaces("non-existing-namespace", setup.Namespace()))

By("validating the RoleBinding created on the remote cluster")
remoteRoleBinding := &rbacv1.RoleBinding{}
remoteRoleBindingName := types.NamespacedName{
Name: trb.GetRBACName(),
Namespace: trb.Namespace,
}
Eventually(func(g Gomega) bool {
g.Expect(clusterAKubeClient.Get(context.TODO(), remoteRoleBindingName, remoteRoleBinding)).To(Succeed(), "there should be no error getting the RoleBinding from the Remote Cluster")
return !remoteRoleBinding.CreationTimestamp.IsZero()
}).Should(BeTrue(), "there should be no error getting the RoleBinding")
Expect(remoteRoleBinding.RoleRef.Name).To(HavePrefix(greenhouseapis.RBACPrefix))
Expect(remoteRoleBinding.RoleRef.Name).To(ContainSubstring(teamRoleUT.Name))

By("validating the ClusterRole created on the remote cluster")
remoteClusterRole := &rbacv1.ClusterRole{}
remoteClusterRoleName := types.NamespacedName{
Name: teamRoleUT.GetRBACName(),
}
Eventually(func(g Gomega) bool {
g.Expect(clusterAKubeClient.Get(test.Ctx, remoteClusterRoleName, remoteClusterRole)).To(Succeed(), "there should be no error getting the ClusterRole from the Remote Cluster")
return !remoteClusterRole.CreationTimestamp.IsZero()
}).Should(BeTrue(), "there should be no error getting the ClusterRole")
Expect(remoteClusterRole.Name).To(HavePrefix(greenhouseapis.RBACPrefix))
Expect(remoteClusterRole.Name).To(ContainSubstring(teamRoleUT.Name))
Expect(remoteClusterRole.Rules).To(Equal(teamRoleUT.Spec.Rules))

By("validating the TeamRoleBinding PropagationStatus for the remote cluster is false")
actTRB := &greenhousev1alpha1.TeamRoleBinding{}
actTRBKey := types.NamespacedName{Name: trb.Name, Namespace: trb.Namespace}
Eventually(func(g Gomega) {
g.Expect(test.K8sClient.Get(test.Ctx, actTRBKey, actTRB)).To(Succeed(), "there should be no error getting the TeamRoleBinding from the Central Cluster")
g.Expect(actTRB.Status.PropagationStatus).To(HaveLen(1), "the TeamRoleBinding should be propagated to one cluster")
g.Expect(actTRB.Status.PropagationStatus[0].Message).To(ContainSubstring("Failed to reconcile RoleBindings"))
}).Should(Succeed(), "there should be no error validating the PropagationStatus")
Expect(remoteClusterRole.Name).To(HavePrefix(greenhouseapis.RBACPrefix))
Expect(remoteClusterRole.Name).To(ContainSubstring(teamRoleUT.Name))
Expect(remoteClusterRole.Rules).To(Equal(teamRoleUT.Spec.Rules))

By("cleaning up the test")
test.EventuallyDeleted(test.Ctx, test.K8sClient, trb)
})
})

Context("When creating a Greenhouse TeamRoleBinding without namespaces on the central cluster", func() {
It("Should create a ClusterRole and ClusterRoleBinding on the remote cluster", func() {
By("creating a TeamRoleBinding without Namespaces on the central cluster")
Expand Down

0 comments on commit 6c50e67

Please sign in to comment.