Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow test cases to interact with client builder #328

Merged
merged 1 commit into from Jan 4, 2023
Merged

Allow test cases to interact with client builder #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 28 additions & 15 deletions testing/client.go
View file
Open in desktop
Expand Up @@ -35,18 +35,18 @@ type clientWrapper struct {

var _ client.Client = &clientWrapper{}

// Deprecated NewFakeClient use NewFakeClientWrapper
func NewFakeClient(scheme *runtime.Scheme, objs ...client.Object) *clientWrapper {
o := make([]runtime.Object, len(objs))
for i := range objs {
obj := objs[i].DeepCopyObject().(client.Object)
// default to a non-zero creation timestamp
if obj.GetCreationTimestamp().Time.IsZero() {
obj.SetCreationTimestamp(metav1.NewTime(time.UnixMilli(1000)))
}
o[i] = obj
}
client := &clientWrapper{
client: fakeclient.NewFakeClientWithScheme(scheme, o...),
builder := fakeclient.NewClientBuilder()
builder = builder.WithScheme(scheme)
builder = builder.WithObjects(prepareObjects(objs)...)

return NewFakeClientWrapper(builder.Build())
}

func NewFakeClientWrapper(client client.Client) *clientWrapper {
c := &clientWrapper{
client: client,
CreateActions: []objectAction{},
UpdateActions: []objectAction{},
PatchActions: []PatchAction{},
Expand All @@ -58,22 +58,35 @@ func NewFakeClient(scheme *runtime.Scheme, objs ...client.Object) *clientWrapper
reactionChain: []Reactor{},
}
// generate names on create
client.AddReactor("create", "*", func(action Action) (bool, runtime.Object, error) {
c.AddReactor("create", "*", func(action Action) (bool, runtime.Object, error) {
if createAction, ok := action.(CreateAction); ok && action.GetSubresource() == "" {
obj := createAction.GetObject()
if accessor, ok := obj.(metav1.ObjectMetaAccessor); ok {
objmeta := accessor.GetObjectMeta()
if objmeta.GetName() == "" && objmeta.GetGenerateName() != "" {
client.genCount++
c.genCount++
// mutate the existing obj
objmeta.SetName(fmt.Sprintf("%s%03d", objmeta.GetGenerateName(), client.genCount))
objmeta.SetName(fmt.Sprintf("%s%03d", objmeta.GetGenerateName(), c.genCount))
}
}
}
// never handle the action
return false, nil, nil
})
return client
return c
}

func prepareObjects(objs []client.Object) []client.Object {
o := make([]client.Object, len(objs))
for i := range objs {
obj := objs[i].DeepCopyObject().(client.Object)
// default to a non-zero creation timestamp
if obj.GetCreationTimestamp().Time.IsZero() {
obj.SetCreationTimestamp(metav1.NewTime(time.UnixMilli(1000)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[thought]: We default a resource's version to 999. In my understanding, that's an intentionally odd value so that it stands out. I am wondering if defaulting the creation timestamp to time.UnixMilli(999) would serve a similar purpose. It would also feel consistent; once you see 999 it's like "Bart reconciler-runtime was here".

reconciler-runtime/testing/subreconciler.go

Line 211 in d526d74

resource.SetResourceVersion("999")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't new, just moved. While I agree with the spirit, I'd rather not risk a subtile regression

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally makes sense to me!

}
o[i] = obj
}
return o
}

func (w *clientWrapper) AddReactor(verb, kind string, reaction ReactionFunc) {
Expand Down
19 changes: 17 additions & 2 deletions testing/config.go
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

// ExpectConfig encompasses the creation of a config object using given state, captures observed
Expand All @@ -41,6 +42,8 @@ type ExpectConfig struct {
GivenObjects []client.Object
// APIGivenObjects contains objects that are only available via an API reader instead of the normal cache
APIGivenObjects []client.Object
// WithClientBuilder allows a test to modify the fake client initialization.
WithClientBuilder func(*fake.ClientBuilder) *fake.ClientBuilder
// WithReactors installs each ReactionFunc into each fake clientset. ReactionFuncs intercept
// each call to the clientset providing the ability to mutate the resource or inject an error.
WithReactors []ReactionFunc
Expand Down Expand Up @@ -88,13 +91,13 @@ func (c *ExpectConfig) init() {
apiGivenObjects[i] = c.APIGivenObjects[i].DeepCopyObject().(client.Object)
}

c.client = NewFakeClient(c.Scheme, givenObjects...)
c.client = c.createClient(givenObjects)
for i := range c.WithReactors {
// in reverse order since we prepend
reactor := c.WithReactors[len(c.WithReactors)-1-i]
c.client.PrependReactor("*", "*", reactor)
}
c.apiReader = NewFakeClient(c.Scheme, apiGivenObjects...)
c.apiReader = c.createClient(apiGivenObjects)
c.recorder = &eventRecorder{
events: []Event{},
scheme: c.Scheme,
Expand All @@ -104,6 +107,18 @@ func (c *ExpectConfig) init() {
})
}

func (c *ExpectConfig) createClient(objs []client.Object) *clientWrapper {
builder := fake.NewClientBuilder()

builder.WithScheme(c.Scheme)
builder.WithObjects(prepareObjects(objs)...)
if c.WithClientBuilder != nil {
builder = c.WithClientBuilder(builder)
}

return NewFakeClientWrapper(builder.Build())
}

// Config returns the Config object. This method should only be called once. Subsequent calls are
// ignored returning the Config from the first call.
func (c *ExpectConfig) Config() reconcilers.Config {
Expand Down
4 changes: 4 additions & 0 deletions testing/reconciler.go
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ import (
"k8s.io/apimachinery/pkg/types"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

Expand All @@ -41,6 +42,8 @@ type ReconcilerTestCase struct {
// WithReactors installs each ReactionFunc into each fake clientset. ReactionFuncs intercept
// each call to the clientset providing the ability to mutate the resource or inject an error.
WithReactors []ReactionFunc
// WithClientBuilder allows a test to modify the fake client initialization.
WithClientBuilder func(*fake.ClientBuilder) *fake.ClientBuilder
// GivenObjects build the kubernetes objects which are present at the onset of reconciliation
GivenObjects []client.Object
// APIGivenObjects contains objects that are only available via an API reader instead of the normal cache
Expand Down Expand Up @@ -157,6 +160,7 @@ func (tc *ReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, factory
Scheme: scheme,
GivenObjects: tc.GivenObjects,
APIGivenObjects: tc.APIGivenObjects,
WithClientBuilder: tc.WithClientBuilder,
WithReactors: tc.WithReactors,
GivenTracks: tc.GivenTracks,
ExpectTracks: tc.ExpectTracks,
Expand Down
4 changes: 4 additions & 0 deletions testing/subreconciler.go
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ import (
"k8s.io/apimachinery/pkg/types"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

Expand All @@ -41,6 +42,8 @@ type SubReconcilerTestCase struct {
Resource client.Object
// GivenStashedValues adds these items to the stash passed into the reconciler. Factories are resolved to their object.
GivenStashedValues map[reconcilers.StashKey]interface{}
// WithClientBuilder allows a test to modify the fake client initialization.
WithClientBuilder func(*fake.ClientBuilder) *fake.ClientBuilder
// WithReactors installs each ReactionFunc into each fake clientset. ReactionFuncs intercept
// each call to the clientset providing the ability to mutate the resource or inject an error.
WithReactors []ReactionFunc
Expand Down Expand Up @@ -181,6 +184,7 @@ func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, facto
Scheme: scheme,
GivenObjects: append(tc.GivenObjects, tc.Resource),
APIGivenObjects: append(tc.APIGivenObjects, tc.Resource),
WithClientBuilder: tc.WithClientBuilder,
WithReactors: tc.WithReactors,
GivenTracks: tc.GivenTracks,
ExpectTracks: tc.ExpectTracks,
Expand Down
4 changes: 4 additions & 0 deletions testing/webhook.go
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/vmware-labs/reconciler-runtime/reconcilers"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

Expand All @@ -38,6 +39,8 @@ type AdmissionWebhookTestCase struct {
Request *admission.Request
// HTTPRequest is the http request used to create the admission request object. If not defined, a minimal request is provided.
HTTPRequest *http.Request
// WithClientBuilder allows a test to modify the fake client initialization.
WithClientBuilder func(*fake.ClientBuilder) *fake.ClientBuilder
// WithReactors installs each ReactionFunc into each fake clientset. ReactionFuncs intercept
// each call to the clientset providing the ability to mutate the resource or inject an error.
WithReactors []ReactionFunc
Expand Down Expand Up @@ -146,6 +149,7 @@ func (tc *AdmissionWebhookTestCase) Run(t *testing.T, scheme *runtime.Scheme, fa
Scheme: scheme,
GivenObjects: tc.GivenObjects,
APIGivenObjects: tc.APIGivenObjects,
WithClientBuilder: tc.WithClientBuilder,
WithReactors: tc.WithReactors,
GivenTracks: tc.GivenTracks,
ExpectTracks: tc.ExpectTracks,
Expand Down