Add patch namespace and split test utils #5506
Conversation
| return fmt.Errorf("error applying namespace: %w", err) | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
This functionality was copied from the handlers package. It didn't have any tests.
| return c.WithWatch.Create(ctx, obj) | ||
| } else { | ||
| return c.WithWatch.Update(ctx, obj) | ||
| } |
There was a problem hiding this comment.
@youngbupark - this is similar to the workaround we have for client-go + Server-Side Apply. Fake runtime client does not support SSA, and they are not planning to fix it, so they recommend this workaround.
It's really lame that none of the clients support SSA for testing, because SSA is recommended in new code.
Test Results2 460 tests +1 2 453 ✔️ +1 1m 58s ⏱️ +2s Results for commit e4f1666. ± Comparison against base commit 22877cd. This pull request removes 2 and adds 3 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
Please fix the linters and tests. |
| ) | ||
|
|
||
| // PatchNamespace creates a namespace if it does not exist. | ||
| func PatchNamespace(ctx context.Context, client runtime_client.Client, namespace string) error { |
There was a problem hiding this comment.
Are we able to replace the following with PatchNamespace ?
https://github.com/project-radius/radius/blob/42a281c73cfc532360512ae0c5a022502f7960a2/pkg/cli/kubernetes/kubernetes.go#L97-L107
There was a problem hiding this comment.
There changes end up being big because of the two clients. All of the intermediate layers of code have to change when you swap to use a different client. This is why I started a discussion with you.
Let me see what I can easily consolidate.
|
Looks like there's a unit test failure to fix. I'll also see if I can dedupe further. |
|
I found like 3 more copies of this code 😆 |
This change adds the server-side "patch namespace" functionality to a shared utility package and adds tests for it. Adding tests triggered a need to refactor our `testutil` package. The problem is that `testutil` references many different abstractions across Radius (like store and secrets) which causes a package reference cycle. Our shared Kubernetes utilities are used by the store, and secret manager. The fix is to split the Kubernetes testutils functionality into its own package. 'utils' packages need to be really granular in Go or else you end up with cycles like this frequently.
rynowak
force-pushed
the
rynowak/runtime-client
branch
from
a5deb41 to
e4f1666
Compare
|
For the detailed report, please go to
|
Description
This change adds the server-side "patch namespace" functionality to a shared utility package and adds tests for it.
Adding tests triggered a need to refactor our
testutilpackage. The problem is thattestutilreferences many different abstractions across Radius (like store and secrets) which causes a package reference cycle. Our shared Kubernetes utilities are used by the store, and secret manager.The fix is to split the Kubernetes testutils functionality into its own package. 'utils' packages need to be really granular in Go or else you end up with cycles like this frequently.
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: