Skip to content

Commit e8b6292

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3145 from k8s-infra-cherrypick-robot/cherry-pick-3143-to-release-0.20
[release-0.20] 🐛 Fakeclient: Fix dataraces when writing to the scheme
2 parents 44bed88 + 39fefb9 commit e8b6292

File tree

2 files changed

+108
-4
lines changed

2 files changed

+108
-4
lines changed

‎pkg/client/fake/client.go

+20-4
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ type fakeClient struct {
7575
trackerWriteLock sync.Mutex
7676
tracker versionedTracker
7777

78-
schemeWriteLock sync.Mutex
79-
scheme *runtime.Scheme
78+
schemeLock sync.RWMutex
79+
scheme *runtime.Scheme
8080

8181
restMapper meta.RESTMapper
8282
withStatusSubresource sets.Set[schema.GroupVersionKind]
@@ -512,6 +512,8 @@ func (t versionedTracker) updateObject(gvr schema.GroupVersionResource, obj runt
512512
}
513513

514514
func (c *fakeClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
515+
c.schemeLock.RLock()
516+
defer c.schemeLock.RUnlock()
515517
gvr, err := getGVRFromObject(obj, c.scheme)
516518
if err != nil {
517519
return err
@@ -561,6 +563,8 @@ func (c *fakeClient) Watch(ctx context.Context, list client.ObjectList, opts ...
561563
}
562564

563565
func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...client.ListOption) error {
566+
c.schemeLock.RLock()
567+
defer c.schemeLock.RUnlock()
564568
gvk, err := apiutil.GVKForObject(obj, c.scheme)
565569
if err != nil {
566570
return err
@@ -573,9 +577,11 @@ func (c *fakeClient) List(ctx context.Context, obj client.ObjectList, opts ...cl
573577
if _, isUnstructuredList := obj.(runtime.Unstructured); isUnstructuredList && !c.scheme.Recognizes(gvk) {
574578
// We need to register the ListKind with UnstructuredList:
575579
// https://github.com/kubernetes/kubernetes/blob/7b2776b89fb1be28d4e9203bdeec079be903c103/staging/src/k8s.io/client-go/dynamic/fake/simple.go#L44-L51
576-
c.schemeWriteLock.Lock()
580+
c.schemeLock.RUnlock()
581+
c.schemeLock.Lock()
577582
c.scheme.AddKnownTypeWithName(gvk.GroupVersion().WithKind(gvk.Kind+"List"), &unstructured.UnstructuredList{})
578-
c.schemeWriteLock.Unlock()
583+
c.schemeLock.Unlock()
584+
c.schemeLock.RLock()
579585
}
580586

581587
listOpts := client.ListOptions{}
@@ -726,6 +732,8 @@ func (c *fakeClient) IsObjectNamespaced(obj runtime.Object) (bool, error) {
726732
}
727733

728734
func (c *fakeClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
735+
c.schemeLock.RLock()
736+
defer c.schemeLock.RUnlock()
729737
createOptions := &client.CreateOptions{}
730738
createOptions.ApplyOptions(opts)
731739

@@ -762,6 +770,8 @@ func (c *fakeClient) Create(ctx context.Context, obj client.Object, opts ...clie
762770
}
763771

764772
func (c *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
773+
c.schemeLock.RLock()
774+
defer c.schemeLock.RUnlock()
765775
gvr, err := getGVRFromObject(obj, c.scheme)
766776
if err != nil {
767777
return err
@@ -807,6 +817,8 @@ func (c *fakeClient) Delete(ctx context.Context, obj client.Object, opts ...clie
807817
}
808818

809819
func (c *fakeClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error {
820+
c.schemeLock.RLock()
821+
defer c.schemeLock.RUnlock()
810822
gvk, err := apiutil.GVKForObject(obj, c.scheme)
811823
if err != nil {
812824
return err
@@ -856,6 +868,8 @@ func (c *fakeClient) Update(ctx context.Context, obj client.Object, opts ...clie
856868
}
857869

858870
func (c *fakeClient) update(obj client.Object, isStatus bool, opts ...client.UpdateOption) error {
871+
c.schemeLock.RLock()
872+
defer c.schemeLock.RUnlock()
859873
updateOptions := &client.UpdateOptions{}
860874
updateOptions.ApplyOptions(opts)
861875

@@ -884,6 +898,8 @@ func (c *fakeClient) Patch(ctx context.Context, obj client.Object, patch client.
884898
}
885899

886900
func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
901+
c.schemeLock.RLock()
902+
defer c.schemeLock.RUnlock()
887903
patchOptions := &client.PatchOptions{}
888904
patchOptions.ApplyOptions(opts)
889905

‎pkg/client/fake/client_test.go

+88
Original file line numberDiff line numberDiff line change
@@ -2516,6 +2516,93 @@ var _ = Describe("Fake client", func() {
25162516
Expect(cl.SubResource(subResourceScale).Update(context.Background(), obj, client.WithSubResourceBody(scale)).Error()).To(Equal(expectedErr))
25172517
})
25182518

2519+
It("is threadsafe", func() {
2520+
cl := NewClientBuilder().Build()
2521+
2522+
u := func() *unstructured.Unstructured {
2523+
u := &unstructured.Unstructured{}
2524+
u.SetAPIVersion("custom/v1")
2525+
u.SetKind("Version")
2526+
u.SetName("foo")
2527+
return u
2528+
}
2529+
2530+
uList := func() *unstructured.UnstructuredList {
2531+
u := &unstructured.UnstructuredList{}
2532+
u.SetAPIVersion("custom/v1")
2533+
u.SetKind("Version")
2534+
2535+
return u
2536+
}
2537+
2538+
meta := func() *metav1.PartialObjectMetadata {
2539+
return &metav1.PartialObjectMetadata{
2540+
ObjectMeta: metav1.ObjectMeta{
2541+
Name: "foo",
2542+
Namespace: "default",
2543+
},
2544+
TypeMeta: metav1.TypeMeta{
2545+
APIVersion: "custom/v1",
2546+
Kind: "Version",
2547+
},
2548+
}
2549+
}
2550+
metaList := func() *metav1.PartialObjectMetadataList {
2551+
return &metav1.PartialObjectMetadataList{
2552+
TypeMeta: metav1.TypeMeta{
2553+
2554+
APIVersion: "custom/v1",
2555+
Kind: "Version",
2556+
},
2557+
}
2558+
}
2559+
2560+
pod := func() *corev1.Pod {
2561+
return &corev1.Pod{ObjectMeta: metav1.ObjectMeta{
2562+
Name: "foo",
2563+
Namespace: "default",
2564+
}}
2565+
}
2566+
2567+
ctx := context.Background()
2568+
ops := []func(){
2569+
func() { _ = cl.Create(ctx, u()) },
2570+
func() { _ = cl.Get(ctx, client.ObjectKeyFromObject(u()), u()) },
2571+
func() { _ = cl.Update(ctx, u()) },
2572+
func() { _ = cl.Patch(ctx, u(), client.RawPatch(types.StrategicMergePatchType, []byte("foo"))) },
2573+
func() { _ = cl.Delete(ctx, u()) },
2574+
func() { _ = cl.DeleteAllOf(ctx, u(), client.HasLabels{"foo"}) },
2575+
func() { _ = cl.List(ctx, uList()) },
2576+
2577+
func() { _ = cl.Create(ctx, meta()) },
2578+
func() { _ = cl.Get(ctx, client.ObjectKeyFromObject(meta()), meta()) },
2579+
func() { _ = cl.Update(ctx, meta()) },
2580+
func() { _ = cl.Patch(ctx, meta(), client.RawPatch(types.StrategicMergePatchType, []byte("foo"))) },
2581+
func() { _ = cl.Delete(ctx, meta()) },
2582+
func() { _ = cl.DeleteAllOf(ctx, meta(), client.HasLabels{"foo"}) },
2583+
func() { _ = cl.List(ctx, metaList()) },
2584+
2585+
func() { _ = cl.Create(ctx, pod()) },
2586+
func() { _ = cl.Get(ctx, client.ObjectKeyFromObject(pod()), pod()) },
2587+
func() { _ = cl.Update(ctx, pod()) },
2588+
func() { _ = cl.Patch(ctx, pod(), client.RawPatch(types.StrategicMergePatchType, []byte("foo"))) },
2589+
func() { _ = cl.Delete(ctx, pod()) },
2590+
func() { _ = cl.DeleteAllOf(ctx, pod(), client.HasLabels{"foo"}) },
2591+
func() { _ = cl.List(ctx, &corev1.PodList{}) },
2592+
}
2593+
2594+
wg := sync.WaitGroup{}
2595+
wg.Add(len(ops))
2596+
for _, op := range ops {
2597+
go func() {
2598+
defer wg.Done()
2599+
op()
2600+
}()
2601+
}
2602+
2603+
wg.Wait()
2604+
})
2605+
25192606
scalableObjs := []client.Object{
25202607
&appsv1.Deployment{
25212608
ObjectMeta: metav1.ObjectMeta{
@@ -2604,6 +2691,7 @@ var _ = Describe("Fake client", func() {
26042691
scaleExpected.ResourceVersion = scaleActual.ResourceVersion
26052692
Expect(cmp.Diff(scaleExpected, scaleActual)).To(BeEmpty())
26062693
})
2694+
26072695
}
26082696
})
26092697

0 commit comments

Comments
 (0)