-
Notifications
You must be signed in to change notification settings - Fork 277
Implement EPP Plugins by datalayer objects #1901
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
Changes from 8 commits
3af0260
ec27b21
7f39815
4cb4e25
c05299a
b0b61c1
b986abc
9b72e7e
2f704b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /* | ||
| Copyright 2025 The Kubernetes Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package datalayer | ||
|
|
||
| // Config defines the configuration of EPP data layer, as the set of DataSources and | ||
| // Extractors defined on them. | ||
| type Config struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this struct a prep for next PR?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct! |
||
| Sources []DataSourceConfig // the data sources configured in the data layer | ||
| } | ||
|
|
||
| // DataSourceConfig defines the configuration of a specific DataSource | ||
| type DataSourceConfig struct { | ||
| Plugin DataSource // the data source plugin instance | ||
| Extractors []Extractor // extractors defined for the data source | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,20 +22,26 @@ import ( | |
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins" | ||
| ) | ||
|
|
||
| const ( | ||
| testType = "test-ds-type" | ||
| ) | ||
|
|
||
| type mockDataSource struct { | ||
| name string | ||
| tn plugins.TypedName | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typedName? |
||
| } | ||
|
|
||
| func (m *mockDataSource) Name() string { return m.name } | ||
| func (m *mockDataSource) TypedName() plugins.TypedName { return m.tn } | ||
| func (m *mockDataSource) Extractors() []string { return []string{} } | ||
| func (m *mockDataSource) AddExtractor(_ Extractor) error { return nil } | ||
| func (m *mockDataSource) Collect(_ context.Context, _ Endpoint) error { return nil } | ||
|
|
||
| func TestRegisterAndGetSource(t *testing.T) { | ||
| reg := DataSourceRegistry{} | ||
| ds := &mockDataSource{name: "test"} | ||
| ds := &mockDataSource{tn: plugins.TypedName{Type: testType, Name: testType}} | ||
|
|
||
| err := reg.Register(ds) | ||
| assert.NoError(t, err, "expected no error on first registration") | ||
|
|
@@ -47,34 +53,34 @@ func TestRegisterAndGetSource(t *testing.T) { | |
| err = reg.Register(nil) | ||
| assert.Error(t, err, "expected error on nil") | ||
|
|
||
| // Get by name | ||
| got, found := reg.GetNamedSource("test") | ||
| // Get by type | ||
| got, found := reg.GetSourceByType(testType) | ||
| assert.True(t, found, "expected to find registered data source") | ||
| assert.Equal(t, "test", got.Name()) | ||
| assert.Equal(t, testType, got.TypedName().Type) | ||
|
|
||
| // Get all sources | ||
| all := reg.GetSources() | ||
| assert.Len(t, all, 1) | ||
| assert.Equal(t, "test", all[0].Name()) | ||
| assert.Equal(t, testType, all[0].TypedName().Type) | ||
|
|
||
| // Default registry | ||
| err = RegisterSource(ds) | ||
| assert.NoError(t, err, "expected no error on registration") | ||
|
|
||
| // Get by name | ||
| got, found = GetNamedSource[*mockDataSource]("test") | ||
| // Get by type | ||
| got, found = GetSourceByType[*mockDataSource](testType) | ||
| assert.True(t, found, "expected to find registered data source") | ||
| assert.Equal(t, "test", got.Name()) | ||
| assert.Equal(t, testType, got.TypedName().Type) | ||
|
|
||
| // Get all sources | ||
| all = GetSources() | ||
| assert.Len(t, all, 1) | ||
| assert.Equal(t, "test", all[0].Name()) | ||
| assert.Equal(t, testType, all[0].TypedName().Type) | ||
| } | ||
|
|
||
| func TestGetNamedSourceWhenNotFound(t *testing.T) { | ||
| reg := DataSourceRegistry{} | ||
| _, found := reg.GetNamedSource("missing") | ||
| _, found := reg.GetSourceByType("missing") | ||
| assert.False(t, found, "expected source to be missing") | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,15 +25,17 @@ import ( | |||||
| "sync" | ||||||
|
|
||||||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/datalayer" | ||||||
| "sigs.k8s.io/gateway-api-inference-extension/pkg/epp/plugins" | ||||||
| ) | ||||||
|
|
||||||
| const ( | ||||||
| DataSourceName = "metrics-data-source" | ||||||
| DataSourceType = "metrics-data-source" | ||||||
| ) | ||||||
|
|
||||||
| // DataSource is a Model Server Protocol (MSP) compliant metrics data source, | ||||||
| // returning Prometheus formatted metrics for an endpoint. | ||||||
| type DataSource struct { | ||||||
| tn plugins.TypedName | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: consistency with other plugins?
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. current plugins use both |
||||||
| metricsScheme string // scheme to use in metrics URL | ||||||
| metricsPath string // path to use in metrics URL | ||||||
|
|
||||||
|
|
@@ -42,10 +44,9 @@ type DataSource struct { | |||||
| } | ||||||
|
|
||||||
| // NewDataSource returns a new MSP compliant metrics data source, configured with | ||||||
| // the provided client factory. If ClientFactory is nil, a default factory is used. | ||||||
| // The Scheme, port and path are command line options. It should be noted that | ||||||
| // a port value of zero is set if the command line is unspecified. | ||||||
| func NewDataSource(metricsScheme string, metricsPath string, skipCertVerification bool, cl Client) *DataSource { | ||||||
| // the provided client configuration. | ||||||
| // The Scheme, path and certificate validation setting are command line options. | ||||||
| func NewDataSource(metricsScheme string, metricsPath string, skipCertVerification bool) *DataSource { | ||||||
| if metricsScheme == "https" { | ||||||
| httpsTransport := baseTransport.Clone() | ||||||
| httpsTransport.TLSClientConfig = &tls.Config{ | ||||||
|
|
@@ -54,33 +55,33 @@ func NewDataSource(metricsScheme string, metricsPath string, skipCertVerificatio | |||||
| defaultClient.Transport = httpsTransport | ||||||
| } | ||||||
|
|
||||||
| if cl == nil { | ||||||
| cl = defaultClient | ||||||
| } | ||||||
|
|
||||||
| dataSrc := &DataSource{ | ||||||
| tn: plugins.TypedName{ | ||||||
| Type: DataSourceType, | ||||||
| Name: DataSourceType, | ||||||
| }, | ||||||
| metricsScheme: metricsScheme, | ||||||
| metricsPath: metricsPath, | ||||||
| client: cl, | ||||||
| client: defaultClient, | ||||||
| } | ||||||
| return dataSrc | ||||||
| } | ||||||
|
|
||||||
| // Name returns the metrics data source name. | ||||||
| func (dataSrc *DataSource) Name() string { | ||||||
| return DataSourceName | ||||||
| // TypedName returns the metrics data source type and name. | ||||||
| func (dataSrc *DataSource) TypedName() plugins.TypedName { | ||||||
| return dataSrc.tn | ||||||
| } | ||||||
|
|
||||||
| // Extractors returns a list of registered Extractor names. | ||||||
| func (dataSrc *DataSource) Extractors() []string { | ||||||
| names := []string{} | ||||||
| extractors := []string{} | ||||||
| dataSrc.extractors.Range(func(_, val any) bool { | ||||||
| if ex, ok := val.(datalayer.Extractor); ok { | ||||||
| names = append(names, ex.Name()) | ||||||
| extractors = append(extractors, ex.TypedName().String()) | ||||||
| } | ||||||
| return true // continue iteration | ||||||
| }) | ||||||
| return names | ||||||
| return extractors | ||||||
| } | ||||||
|
|
||||||
| // AddExtractor adds an extractor to the data source, validating it can process | ||||||
|
|
@@ -89,8 +90,8 @@ func (dataSrc *DataSource) AddExtractor(extractor datalayer.Extractor) error { | |||||
| if err := datalayer.ValidateExtractorType(PrometheusMetricType, extractor.ExpectedInputType()); err != nil { | ||||||
| return err | ||||||
| } | ||||||
| if _, loaded := dataSrc.extractors.LoadOrStore(extractor.Name(), extractor); loaded { | ||||||
| return fmt.Errorf("attempt to add extractor with duplicate name %s to %s", extractor.Name(), dataSrc.Name()) | ||||||
| if _, loaded := dataSrc.extractors.LoadOrStore(extractor.TypedName().Type, extractor); loaded { | ||||||
elevran marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| return fmt.Errorf("attempt to add duplicate extractor %s to %s", extractor.TypedName(), dataSrc.TypedName()) | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.