-
Notifications
You must be signed in to change notification settings - Fork 96
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
Add recipe engine #8180
base: main
Are you sure you want to change the base?
Add recipe engine #8180
Conversation
limitations under the License. | ||
*/ | ||
|
||
package kubernetesclientprovider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to previous changes, moving the functionality to access a Kubernetes client into a "provider". By deferring and standardizing the initialization that makes it easier to override in tests.
01e54b5
to
a425853
Compare
@@ -35,6 +35,8 @@ data: | |||
port: 6062 | |||
secretProvider: | |||
provider: kubernetes | |||
kubernetes: | |||
kind: default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only needed for dynamic-rp. UCP doesn't talk directly to Kubernetes, and applications-rp doesn't have the same expectations for testing.
Signed-off-by: Ryan Nowak <[email protected]>
a425853
to
c8296ae
Compare
@@ -92,6 +97,9 @@ func Start(t *testing.T, opts ...TestHostOption) (*TestHost, *ucptesthost.TestHo | |||
options, err := dynamicrp.NewOptions(context.Background(), config) | |||
require.NoError(t, err) | |||
|
|||
// Prevent the default recipe drivers from being registered. | |||
options.Recipes.Drivers = map[string]func(options *dynamicrp.Options) (driver.Driver, error){} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we can load/use/mock the recipe engine in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be used in the next PR, where we're actually going to test a resource that uses recipes.
} | ||
|
||
// WithLegacy implements discovery.DiscoveryInterface. | ||
func (d *DiscoveryClient) WithLegacy() discovery.DiscoveryInterface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions were missing from our mock. The discovery package defines a ton of different interfaces. I changed from a really scenario-specific one to a more general one.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8180 +/- ##
==========================================
- Coverage 60.09% 59.89% -0.20%
==========================================
Files 584 585 +1
Lines 38675 38850 +175
==========================================
+ Hits 23240 23269 +29
- Misses 13729 13863 +134
- Partials 1706 1718 +12 ☔ View full report in Codecov by Sentry. |
|
||
// FromConfig creates a new Kubernetes client provider from the given config. | ||
func FromConfig(config *rest.Config) *KubernetesClientProvider { | ||
return &KubernetesClientProvider{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to return error here if config is nil? It will also distinguish it from FromEmpty()
Description
This change adds the recipe engine to the dynamic-rp. There's no usage of the recipe engine yet in this PR, it's a prerequisite for what's coming next.
The main problem being solved is improving the initialization code so that it can be used in our integration tests.
Type of change
Part of: #6688
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: