Skip to content

Commit b9495d6

Browse files
copejoncursoragent
andcommitted
USHIFT-6401: Add per-attempt timeout to RBAC bootstrap hook
Thread a 5s context.WithTimeout into each poll attempt of the RBAC bootstrap PostStartHook, preventing indefinite hangs when etcd is unresponsive during initial readiness checks. Replace context.TODO() with the deadline-scoped context in List calls and prime functions. Add a klog.Warningf on timeout to distinguish deadline exceeded from other transient errors. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c1bf0af commit b9495d6

4 files changed

Lines changed: 174 additions & 29 deletions

File tree

deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac.go

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,11 @@ func (p *PolicyData) EnsureRBACPolicy() genericapiserver.PostStartHookFunc {
169169
utilruntime.HandleError(fmt.Errorf("unable to initialize client set: %v", err))
170170
return false, nil
171171
}
172-
return ensureRBACPolicy(p, client)
172+
// hookContext only cancels on server shutdown — add a per-attempt
173+
// deadline so individual API calls cannot block indefinitely.
174+
ctx, cancel := context.WithTimeout(hookContext, 5*time.Second)
175+
defer cancel()
176+
return ensureRBACPolicy(ctx, p, client)
173177
})
174178
// if we're never able to make it through initialization, kill the API server
175179
if err != nil {
@@ -180,26 +184,34 @@ func (p *PolicyData) EnsureRBACPolicy() genericapiserver.PostStartHookFunc {
180184
}
181185
}
182186

183-
func ensureRBACPolicy(p *PolicyData, client clientset.Interface) (done bool, err error) {
187+
func ensureRBACPolicy(ctx context.Context, p *PolicyData, client clientset.Interface) (done bool, err error) {
184188
failedReconciliation := false
185189
// Make sure etcd is responding before we start reconciling
186-
if _, err := client.RbacV1().ClusterRoles().List(context.TODO(), metav1.ListOptions{}); err != nil {
190+
if _, err := client.RbacV1().ClusterRoles().List(ctx, metav1.ListOptions{}); err != nil {
191+
if ctx.Err() != nil {
192+
klog.Warningf("RBAC bootstrap attempt timed out waiting for etcd readiness, retrying")
193+
return false, nil
194+
}
187195
utilruntime.HandleError(fmt.Errorf("unable to initialize clusterroles: %v", err))
188196
return false, nil
189197
}
190-
if _, err := client.RbacV1().ClusterRoleBindings().List(context.TODO(), metav1.ListOptions{}); err != nil {
198+
if _, err := client.RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}); err != nil {
199+
if ctx.Err() != nil {
200+
klog.Warningf("RBAC bootstrap attempt timed out waiting for etcd readiness, retrying")
201+
return false, nil
202+
}
191203
utilruntime.HandleError(fmt.Errorf("unable to initialize clusterrolebindings: %v", err))
192204
return false, nil
193205
}
194206

195207
// if the new cluster roles to aggregate do not yet exist, then we need to copy the old roles if they don't exist
196208
// in new locations
197-
if err := primeAggregatedClusterRoles(p.ClusterRolesToAggregate, client.RbacV1()); err != nil {
209+
if err := primeAggregatedClusterRoles(ctx, p.ClusterRolesToAggregate, client.RbacV1()); err != nil {
198210
utilruntime.HandleError(fmt.Errorf("unable to prime aggregated clusterroles: %v", err))
199211
return false, nil
200212
}
201213

202-
if err := primeSplitClusterRoleBindings(p.ClusterRoleBindingsToSplit, client.RbacV1()); err != nil {
214+
if err := primeSplitClusterRoleBindings(ctx, p.ClusterRoleBindingsToSplit, client.RbacV1()); err != nil {
203215
utilruntime.HandleError(fmt.Errorf("unable to prime split ClusterRoleBindings: %v", err))
204216
return false, nil
205217
}
@@ -345,17 +357,17 @@ func (p RESTStorageProvider) GroupName() string {
345357

346358
// primeAggregatedClusterRoles copies roles that have transitioned to aggregated roles and may need to pick up changes
347359
// that were done to the legacy roles.
348-
func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clusterRoleClient rbacv1client.ClusterRolesGetter) error {
360+
func primeAggregatedClusterRoles(ctx context.Context, clusterRolesToAggregate map[string]string, clusterRoleClient rbacv1client.ClusterRolesGetter) error {
349361
for oldName, newName := range clusterRolesToAggregate {
350-
_, err := clusterRoleClient.ClusterRoles().Get(context.TODO(), newName, metav1.GetOptions{})
362+
_, err := clusterRoleClient.ClusterRoles().Get(ctx, newName, metav1.GetOptions{})
351363
if err == nil {
352364
continue
353365
}
354366
if !apierrors.IsNotFound(err) {
355367
return err
356368
}
357369

358-
existingRole, err := clusterRoleClient.ClusterRoles().Get(context.TODO(), oldName, metav1.GetOptions{})
370+
existingRole, err := clusterRoleClient.ClusterRoles().Get(ctx, oldName, metav1.GetOptions{})
359371
if apierrors.IsNotFound(err) {
360372
continue
361373
}
@@ -369,7 +381,7 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
369381
klog.V(1).Infof("migrating %v to %v", existingRole.Name, newName)
370382
existingRole.Name = newName
371383
existingRole.ResourceVersion = "" // clear this so the object can be created.
372-
if _, err := clusterRoleClient.ClusterRoles().Create(context.TODO(), existingRole, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
384+
if _, err := clusterRoleClient.ClusterRoles().Create(ctx, existingRole, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
373385
return err
374386
}
375387
}
@@ -380,10 +392,10 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
380392
// primeSplitClusterRoleBindings ensures the existence of target ClusterRoleBindings
381393
// by copying Subjects, Annotations, and Labels from the specified source
382394
// ClusterRoleBinding, if present.
383-
func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv1.ClusterRoleBinding, clusterRoleBindingClient rbacv1client.ClusterRoleBindingsGetter) error {
395+
func primeSplitClusterRoleBindings(ctx context.Context, clusterRoleBindingToSplit map[string]rbacapiv1.ClusterRoleBinding, clusterRoleBindingClient rbacv1client.ClusterRoleBindingsGetter) error {
384396
for existingBindingName, clusterRoleBindingToCreate := range clusterRoleBindingToSplit {
385397
// If source ClusterRoleBinding does not exist, do nothing.
386-
existingRoleBinding, err := clusterRoleBindingClient.ClusterRoleBindings().Get(context.TODO(), existingBindingName, metav1.GetOptions{})
398+
existingRoleBinding, err := clusterRoleBindingClient.ClusterRoleBindings().Get(ctx, existingBindingName, metav1.GetOptions{})
387399
if apierrors.IsNotFound(err) {
388400
continue
389401
}
@@ -392,7 +404,7 @@ func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv
392404
}
393405

394406
// If the target ClusterRoleBinding already exists, do nothing.
395-
_, err = clusterRoleBindingClient.ClusterRoleBindings().Get(context.TODO(), clusterRoleBindingToCreate.Name, metav1.GetOptions{})
407+
_, err = clusterRoleBindingClient.ClusterRoleBindings().Get(ctx, clusterRoleBindingToCreate.Name, metav1.GetOptions{})
396408
if err == nil {
397409
continue
398410
}
@@ -407,7 +419,7 @@ func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv
407419
newCRB.Subjects = existingRoleBinding.Subjects
408420
newCRB.Labels = existingRoleBinding.Labels
409421
newCRB.Annotations = existingRoleBinding.Annotations
410-
if _, err := clusterRoleBindingClient.ClusterRoleBindings().Create(context.TODO(), newCRB, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
422+
if _, err := clusterRoleBindingClient.ClusterRoleBindings().Create(ctx, newCRB, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
411423
return err
412424
}
413425
}

deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package rest
1818

1919
import (
20+
"context"
2021
"testing"
2122

2223
"k8s.io/client-go/kubernetes/fake"
@@ -34,6 +35,8 @@ func BenchmarkEnsureRBACPolicy(b *testing.B) {
3435
ClusterRoleBindingsToSplit: bootstrappolicy.ClusterRoleBindingsToSplit(),
3536
}
3637
coreClientSet := fake.NewSimpleClientset()
37-
_, _ = ensureRBACPolicy(policy, coreClientSet)
38+
if _, err := ensureRBACPolicy(context.Background(), policy, coreClientSet); err != nil {
39+
b.Fatalf("ensureRBACPolicy failed: %v", err)
40+
}
3841
}
3942
}
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
diff --git a/deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac.go b/deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac.go
2+
index 16502c476..e0cb709fc 100644
3+
--- a/deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac.go
4+
+++ b/deps/github.com/openshift/kubernetes/pkg/registry/rbac/rest/storage_rbac.go
5+
@@ -169,7 +169,11 @@ func (p *PolicyData) EnsureRBACPolicy() genericapiserver.PostStartHookFunc {
6+
utilruntime.HandleError(fmt.Errorf("unable to initialize client set: %v", err))
7+
return false, nil
8+
}
9+
- return ensureRBACPolicy(p, client)
10+
+ // hookContext only cancels on server shutdown — add a per-attempt
11+
+ // deadline so individual API calls cannot block indefinitely.
12+
+ ctx, cancel := context.WithTimeout(hookContext, 5*time.Second)
13+
+ defer cancel()
14+
+ return ensureRBACPolicy(ctx, p, client)
15+
})
16+
// if we're never able to make it through initialization, kill the API server
17+
if err != nil {
18+
@@ -180,26 +184,34 @@ func (p *PolicyData) EnsureRBACPolicy() genericapiserver.PostStartHookFunc {
19+
}
20+
}
21+
22+
-func ensureRBACPolicy(p *PolicyData, client clientset.Interface) (done bool, err error) {
23+
+func ensureRBACPolicy(ctx context.Context, p *PolicyData, client clientset.Interface) (done bool, err error) {
24+
failedReconciliation := false
25+
// Make sure etcd is responding before we start reconciling
26+
- if _, err := client.RbacV1().ClusterRoles().List(context.TODO(), metav1.ListOptions{}); err != nil {
27+
+ if _, err := client.RbacV1().ClusterRoles().List(ctx, metav1.ListOptions{}); err != nil {
28+
+ if ctx.Err() != nil {
29+
+ klog.Warningf("RBAC bootstrap attempt timed out waiting for etcd readiness, retrying")
30+
+ return false, nil
31+
+ }
32+
utilruntime.HandleError(fmt.Errorf("unable to initialize clusterroles: %v", err))
33+
return false, nil
34+
}
35+
- if _, err := client.RbacV1().ClusterRoleBindings().List(context.TODO(), metav1.ListOptions{}); err != nil {
36+
+ if _, err := client.RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}); err != nil {
37+
+ if ctx.Err() != nil {
38+
+ klog.Warningf("RBAC bootstrap attempt timed out waiting for etcd readiness, retrying")
39+
+ return false, nil
40+
+ }
41+
utilruntime.HandleError(fmt.Errorf("unable to initialize clusterrolebindings: %v", err))
42+
return false, nil
43+
}
44+
45+
// if the new cluster roles to aggregate do not yet exist, then we need to copy the old roles if they don't exist
46+
// in new locations
47+
- if err := primeAggregatedClusterRoles(p.ClusterRolesToAggregate, client.RbacV1()); err != nil {
48+
+ if err := primeAggregatedClusterRoles(ctx, p.ClusterRolesToAggregate, client.RbacV1()); err != nil {
49+
utilruntime.HandleError(fmt.Errorf("unable to prime aggregated clusterroles: %v", err))
50+
return false, nil
51+
}
52+
53+
- if err := primeSplitClusterRoleBindings(p.ClusterRoleBindingsToSplit, client.RbacV1()); err != nil {
54+
+ if err := primeSplitClusterRoleBindings(ctx, p.ClusterRoleBindingsToSplit, client.RbacV1()); err != nil {
55+
utilruntime.HandleError(fmt.Errorf("unable to prime split ClusterRoleBindings: %v", err))
56+
return false, nil
57+
}
58+
@@ -345,9 +357,9 @@ func (p RESTStorageProvider) GroupName() string {
59+
60+
// primeAggregatedClusterRoles copies roles that have transitioned to aggregated roles and may need to pick up changes
61+
// that were done to the legacy roles.
62+
-func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clusterRoleClient rbacv1client.ClusterRolesGetter) error {
63+
+func primeAggregatedClusterRoles(ctx context.Context, clusterRolesToAggregate map[string]string, clusterRoleClient rbacv1client.ClusterRolesGetter) error {
64+
for oldName, newName := range clusterRolesToAggregate {
65+
- _, err := clusterRoleClient.ClusterRoles().Get(context.TODO(), newName, metav1.GetOptions{})
66+
+ _, err := clusterRoleClient.ClusterRoles().Get(ctx, newName, metav1.GetOptions{})
67+
if err == nil {
68+
continue
69+
}
70+
@@ -355,7 +367,7 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
71+
return err
72+
}
73+
74+
- existingRole, err := clusterRoleClient.ClusterRoles().Get(context.TODO(), oldName, metav1.GetOptions{})
75+
+ existingRole, err := clusterRoleClient.ClusterRoles().Get(ctx, oldName, metav1.GetOptions{})
76+
if apierrors.IsNotFound(err) {
77+
continue
78+
}
79+
@@ -369,7 +381,7 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
80+
klog.V(1).Infof("migrating %v to %v", existingRole.Name, newName)
81+
existingRole.Name = newName
82+
existingRole.ResourceVersion = "" // clear this so the object can be created.
83+
- if _, err := clusterRoleClient.ClusterRoles().Create(context.TODO(), existingRole, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
84+
+ if _, err := clusterRoleClient.ClusterRoles().Create(ctx, existingRole, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
85+
return err
86+
}
87+
}
88+
@@ -380,10 +392,10 @@ func primeAggregatedClusterRoles(clusterRolesToAggregate map[string]string, clus
89+
// primeSplitClusterRoleBindings ensures the existence of target ClusterRoleBindings
90+
// by copying Subjects, Annotations, and Labels from the specified source
91+
// ClusterRoleBinding, if present.
92+
-func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv1.ClusterRoleBinding, clusterRoleBindingClient rbacv1client.ClusterRoleBindingsGetter) error {
93+
+func primeSplitClusterRoleBindings(ctx context.Context, clusterRoleBindingToSplit map[string]rbacapiv1.ClusterRoleBinding, clusterRoleBindingClient rbacv1client.ClusterRoleBindingsGetter) error {
94+
for existingBindingName, clusterRoleBindingToCreate := range clusterRoleBindingToSplit {
95+
// If source ClusterRoleBinding does not exist, do nothing.
96+
- existingRoleBinding, err := clusterRoleBindingClient.ClusterRoleBindings().Get(context.TODO(), existingBindingName, metav1.GetOptions{})
97+
+ existingRoleBinding, err := clusterRoleBindingClient.ClusterRoleBindings().Get(ctx, existingBindingName, metav1.GetOptions{})
98+
if apierrors.IsNotFound(err) {
99+
continue
100+
}
101+
@@ -392,7 +404,7 @@ func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv
102+
}
103+
104+
// If the target ClusterRoleBinding already exists, do nothing.
105+
- _, err = clusterRoleBindingClient.ClusterRoleBindings().Get(context.TODO(), clusterRoleBindingToCreate.Name, metav1.GetOptions{})
106+
+ _, err = clusterRoleBindingClient.ClusterRoleBindings().Get(ctx, clusterRoleBindingToCreate.Name, metav1.GetOptions{})
107+
if err == nil {
108+
continue
109+
}
110+
@@ -407,7 +419,7 @@ func primeSplitClusterRoleBindings(clusterRoleBindingToSplit map[string]rbacapiv
111+
newCRB.Subjects = existingRoleBinding.Subjects
112+
newCRB.Labels = existingRoleBinding.Labels
113+
newCRB.Annotations = existingRoleBinding.Annotations
114+
- if _, err := clusterRoleBindingClient.ClusterRoleBindings().Create(context.TODO(), newCRB, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
115+
+ if _, err := clusterRoleBindingClient.ClusterRoleBindings().Create(ctx, newCRB, metav1.CreateOptions{}); err != nil && !apierrors.IsAlreadyExists(err) {
116+
return err
117+
}
118+
}

0 commit comments

Comments
 (0)