Skip to content

Commit

Permalink
Compatibility with OpenShift
Browse files Browse the repository at this point in the history
OpenShift requires special permissions for setting owner references. We
were unable to identify what specific permission is needed to allow
setting "BlockOwnerDeletion", and we do not require blocking the
deletion of User kind on the deletion of the generated crdedentials
Secret. We decided to simply set to false "BlockOwnerDeletion" to
workaround the problem.

Signed-off-by: Aitor Perez Cedres <acedres@vmware.com>
  • Loading branch information
Zerpet committed Jul 28, 2021
1 parent b2de176 commit e09d8df
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
7 changes: 7 additions & 0 deletions controllers/user_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"encoding/json"
"errors"
"fmt"
"k8s.io/utils/pointer"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -174,6 +175,12 @@ func (r *UserReconciler) declareCredentials(ctx context.Context, user *topology.
if err := controllerutil.SetControllerReference(user, &credentialSecret, r.Scheme); err != nil {
return fmt.Errorf("failed setting controller reference: %v", err)
}
// required for OpenShift compatibility. See:
// https://github.com/rabbitmq/cluster-operator/blob/057b61eb50102a66f504b31464e5956526cbdc90/internal/resource/statefulset.go#L220-L226
// https://github.com/rabbitmq/messaging-topology-operator/issues/194
for i := range credentialSecret.ObjectMeta.OwnerReferences {
credentialSecret.ObjectMeta.OwnerReferences[i].BlockOwnerDeletion = pointer.BoolPtr(false)
}
return nil
})
return apiError
Expand Down
33 changes: 33 additions & 0 deletions controllers/user_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,39 @@ var _ = Describe("UserController", func() {
"Status": Equal(corev1.ConditionTrue),
})))
})

It("sets an owner reference and does not block owner deletion", func() {
user.Name = "test-owner-reference"
Expect(client.Create(ctx, &user)).To(Succeed())
EventuallyWithOffset(1, func() []topology.Condition {
_ = client.Get(
ctx,
types.NamespacedName{Name: user.Name, Namespace: user.Namespace},
&user,
)

return user.Status.Conditions
}, 10*time.Second, 1*time.Second).Should(ContainElement(MatchFields(IgnoreExtras, Fields{
"Type": Equal(topology.ConditionType("Ready")),
"Reason": Equal("SuccessfulCreateOrUpdate"),
"Status": Equal(corev1.ConditionTrue),
})), "User should have been created and have a True Ready condition")

generatedSecret := &corev1.Secret{}
EventuallyWithOffset(1, func() error {
return client.Get(ctx,
types.NamespacedName{
Namespace: user.Namespace,
Name: user.Name + "-user-credentials",
}, generatedSecret)
}, 10*time.Second).ShouldNot(HaveOccurred())

for _, ref := range generatedSecret.ObjectMeta.OwnerReferences {
Expect(ref).To(MatchFields(IgnoreExtras, Fields{
"BlockOwnerDeletion": PointTo(BeFalse()),
}))
}
})
})
})

Expand Down

0 comments on commit e09d8df

Please sign in to comment.