Skip to content

Commit 843012f

Browse files
committed
chore: add review comments and clean up whitespace
Document the upgrade gap for pre-existing Roles without the ClusterLabelName label, the single-owner assumption in SetControllerReference, and the race window between the Pre hook and the ObjectStore controller. Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
1 parent 6ecbd9c commit 843012f

4 files changed

Lines changed: 16 additions & 1 deletion

File tree

internal/cnpgi/operator/rbac/ensure.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ import (
4242
// This function is called from the Pre hook (gRPC). It creates the
4343
// Role if it does not exist, then patches rules and labels to match
4444
// the desired state.
45+
//
46+
// Note: the ObjectStore controller (EnsureRoleRules) can patch the
47+
// same Role concurrently. Both paths use RetryOnConflict but compute
48+
// desired rules from their own view of ObjectStores. If the Pre hook
49+
// reads stale ObjectStore data from the informer cache, it may
50+
// briefly revert a fresher update. This is self-healing: the next
51+
// ObjectStore reconcile restores the correct state.
4552
func EnsureRole(
4653
ctx context.Context,
4754
c client.Client,

internal/cnpgi/operator/specs/ownership.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ import (
3333
// the operator does not know the CNPG API group at compile time
3434
// (it may be customized), while the Cluster object decoded from
3535
// the gRPC request carries the correct GVK in its TypeMeta.
36+
//
37+
// This function replaces all existing owner references rather than
38+
// merging, so it assumes the controlled object has a single owner.
39+
// This holds for plugin-managed Roles and RoleBindings, which are
40+
// exclusively owned by one Cluster.
3641
func SetControllerReference(owner, controlled metav1.Object) error {
3742
ro, ok := owner.(runtime.Object)
3843
if !ok {

internal/cnpgi/restore/manager.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/common"
3333
)
3434

35-
3635
// Start starts the sidecar informers and CNPG-i server
3736
func Start(ctx context.Context) error {
3837
setupLog := log.FromContext(ctx)

internal/controller/objectstore_controller.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ func (r *ObjectStoreReconciler) Reconcile(ctx context.Context, req ctrl.Request)
6868

6969
contextLogger.Info("ObjectStore reconciliation start")
7070

71+
// NOTE: Roles created before the introduction of ClusterLabelName
72+
// are not discovered here. The Pre hook patches the label on every
73+
// Cluster reconciliation, so unlabeled Roles are picked up after
74+
// the next Cluster reconcile cycle.
7175
var roleList rbacv1.RoleList
7276
if err := r.List(ctx, &roleList,
7377
client.InNamespace(req.Namespace),

0 commit comments

Comments
 (0)