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.

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

cluster: refactor client initialization #5618

Merged
merged 1 commit into from
Mar 23, 2022
Merged

Conversation

milas
Copy link
Contributor

@milas milas commented Mar 22, 2022

The motivation here is in preparation for cluster status polling
to make it possible to write more robust unit tests. Currently,
the reconciler behaved very differently when provided with fake
clients for test. (In fact, there was actually a bug as a result
of this! The ConnectedAt timestamp handling was different for
tests.)

Now, there's factories for both K8s/Docker clients that get
injected, and tests can override these. Helper methods are
offered to return a static client for all created connections
or an error.

This latter part is critical to simulating errors with fake clients
in a controlled fashion to verify the (forthcoming) retry behavior.

The motivation here is in preparation for cluster status polling
to make it possible to write more robust unit tests. Currently,
the reconciler behaved very differently when provided with fake
clients for test. (In fact, there was actually a bug as a result
of this! The `ConnectedAt` timestamp handling was different for
tests.)

Now, there's factories for both K8s/Docker clients that get
injected, and tests can override these. Helper methods are
offered to return a static client for all created connections
or an error.

This latter part is critical to simulating errors with fake clients
in a controlled fashion to verify the (forthcoming) retry behavior.
@milas milas requested review from nicksieger, nicks and landism March 22, 2022 16:36
@milas milas self-assigned this Mar 22, 2022
@milas milas merged commit e969c95 into master Mar 23, 2022
@milas milas deleted the milas/cluster-client-factory branch March 23, 2022 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants