Skip to content

Commit 94249dc

Browse files
authored
Improve probe parsing (#3250)
* begin interface change * move over processor config * use it * fix tests * Generics * rename * builder * Change to use the builder pattern instead of the option pattern * remove unused methods * fix unit tests * Delete dead code * Delete more * Add parsing for extensions * Add parser for liveness and readiness probes * Set defaults * chlog * fix tests * fix merge
1 parent c38fb49 commit 94249dc

19 files changed

+853
-431
lines changed

.chloggen/improve-probe-parsing.yaml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: enhancement
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: collector
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Improves healthcheck parsing capabilities, allowing for future extensions to configure a healthcheck other than the v1 healthcheck extension.
9+
10+
# One or more tracking issues related to the change
11+
issues: [3184]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext:

apis/v1beta1/config.go

+45-3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131

3232
"github.com/open-telemetry/opentelemetry-operator/internal/components"
3333
"github.com/open-telemetry/opentelemetry-operator/internal/components/exporters"
34+
"github.com/open-telemetry/opentelemetry-operator/internal/components/extensions"
3435
"github.com/open-telemetry/opentelemetry-operator/internal/components/processors"
3536
"github.com/open-telemetry/opentelemetry-operator/internal/components/receivers"
3637
)
@@ -41,10 +42,11 @@ const (
4142
KindReceiver ComponentKind = iota
4243
KindExporter
4344
KindProcessor
45+
KindExtension
4446
)
4547

4648
func (c ComponentKind) String() string {
47-
return [...]string{"receiver", "exporter", "processor"}[c]
49+
return [...]string{"receiver", "exporter", "processor", "extension"}[c]
4850
}
4951

5052
// AnyConfig represent parts of the config.
@@ -108,6 +110,7 @@ func (c *Config) GetEnabledComponents() map[ComponentKind]map[string]interface{}
108110
KindReceiver: {},
109111
KindProcessor: {},
110112
KindExporter: {},
113+
KindExtension: {},
111114
}
112115
for _, pipeline := range c.Service.Pipelines {
113116
if pipeline == nil {
@@ -123,6 +126,9 @@ func (c *Config) GetEnabledComponents() map[ComponentKind]map[string]interface{}
123126
toReturn[KindProcessor][componentId] = struct{}{}
124127
}
125128
}
129+
for _, componentId := range c.Service.Extensions {
130+
toReturn[KindExtension][componentId] = struct{}{}
131+
}
126132
return toReturn
127133
}
128134

@@ -162,6 +168,8 @@ func (c *Config) getRbacRulesForComponentKinds(logger logr.Logger, componentKind
162168
} else {
163169
cfg = *c.Processors
164170
}
171+
case KindExtension:
172+
continue
165173
}
166174
for componentName := range enabledComponents[componentKind] {
167175
// TODO: Clean up the naming here and make it simpler to use a retriever.
@@ -191,7 +199,9 @@ func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds ..
191199
retriever = exporters.ParserFor
192200
cfg = c.Exporters
193201
case KindProcessor:
194-
break
202+
continue
203+
case KindExtension:
204+
continue
195205
}
196206
for componentName := range enabledComponents[componentKind] {
197207
// TODO: Clean up the naming here and make it simpler to use a retriever.
@@ -227,6 +237,38 @@ func (c *Config) GetAllRbacRules(logger logr.Logger) ([]rbacv1.PolicyRule, error
227237
return c.getRbacRulesForComponentKinds(logger, KindReceiver, KindExporter, KindProcessor)
228238
}
229239

240+
// GetLivenessProbe gets the first enabled liveness probe. There should only ever be one extension enabled
241+
// that provides the hinting for the liveness probe.
242+
func (c *Config) GetLivenessProbe(logger logr.Logger) (*corev1.Probe, error) {
243+
enabledComponents := c.GetEnabledComponents()
244+
for componentName := range enabledComponents[KindExtension] {
245+
// TODO: Clean up the naming here and make it simpler to use a retriever.
246+
parser := extensions.ParserFor(componentName)
247+
if probe, err := parser.GetLivenessProbe(logger, c.Extensions.Object[componentName]); err != nil {
248+
return nil, err
249+
} else if probe != nil {
250+
return probe, nil
251+
}
252+
}
253+
return nil, nil
254+
}
255+
256+
// GetReadinessProbe gets the first enabled readiness probe. There should only ever be one extension enabled
257+
// that provides the hinting for the readiness probe.
258+
func (c *Config) GetReadinessProbe(logger logr.Logger) (*corev1.Probe, error) {
259+
enabledComponents := c.GetEnabledComponents()
260+
for componentName := range enabledComponents[KindExtension] {
261+
// TODO: Clean up the naming here and make it simpler to use a retriever.
262+
parser := extensions.ParserFor(componentName)
263+
if probe, err := parser.GetReadinessProbe(logger, c.Extensions.Object[componentName]); err != nil {
264+
return nil, err
265+
} else if probe != nil {
266+
return probe, nil
267+
}
268+
}
269+
return nil, nil
270+
}
271+
230272
// Yaml encodes the current object and returns it as a string.
231273
func (c *Config) Yaml() (string, error) {
232274
var buf bytes.Buffer
@@ -268,7 +310,7 @@ func (c *Config) nullObjects() []string {
268310
}
269311

270312
type Service struct {
271-
Extensions *[]string `json:"extensions,omitempty" yaml:"extensions,omitempty"`
313+
Extensions []string `json:"extensions,omitempty" yaml:"extensions,omitempty"`
272314
// +kubebuilder:pruning:PreserveUnknownFields
273315
Telemetry *AnyConfig `json:"telemetry,omitempty" yaml:"telemetry,omitempty"`
274316
// +kubebuilder:pruning:PreserveUnknownFields

apis/v1beta1/config_test.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func TestConfigYaml(t *testing.T) {
143143
},
144144
},
145145
Service: Service{
146-
Extensions: &[]string{"addon"},
146+
Extensions: []string{"addon"},
147147
Telemetry: &AnyConfig{
148148
Object: map[string]interface{}{
149149
"insights": "yeah!",
@@ -304,6 +304,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
304304
"bar": struct{}{},
305305
"count": struct{}{},
306306
},
307+
KindExtension: {},
307308
},
308309
},
309310
{
@@ -321,6 +322,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
321322
KindExporter: {
322323
"prometheus": struct{}{},
323324
},
325+
KindExtension: {},
324326
},
325327
},
326328
{
@@ -339,6 +341,11 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
339341
"otlp": struct{}{},
340342
"prometheus": struct{}{},
341343
},
344+
KindExtension: {
345+
"health_check": struct{}{},
346+
"pprof": struct{}{},
347+
"zpages": struct{}{},
348+
},
342349
},
343350
},
344351
{
@@ -352,6 +359,9 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
352359
KindExporter: {
353360
"otlp/auth": struct{}{},
354361
},
362+
KindExtension: {
363+
"oauth2client": struct{}{},
364+
},
355365
},
356366
},
357367
{
@@ -365,6 +375,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
365375
KindExporter: {
366376
"debug": struct{}{},
367377
},
378+
KindExtension: {},
368379
},
369380
},
370381
{
@@ -374,6 +385,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) {
374385
KindReceiver: {},
375386
KindProcessor: {},
376387
KindExporter: {},
388+
KindExtension: {},
377389
},
378390
},
379391
}

apis/v1beta1/zz_generated.deepcopy.go

+2-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/components/builder.go

+30-9
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ import (
2626
type ParserOption[ComponentConfigType any] func(*Settings[ComponentConfigType])
2727

2828
type Settings[ComponentConfigType any] struct {
29-
protocol corev1.Protocol
30-
appProtocol *string
31-
targetPort intstr.IntOrString
32-
nodePort int32
33-
name string
34-
port int32
35-
portParser PortParser[ComponentConfigType]
36-
rbacGen RBACRuleGenerator[ComponentConfigType]
29+
protocol corev1.Protocol
30+
appProtocol *string
31+
targetPort intstr.IntOrString
32+
nodePort int32
33+
name string
34+
port int32
35+
portParser PortParser[ComponentConfigType]
36+
rbacGen RBACRuleGenerator[ComponentConfigType]
37+
livenessGen ProbeGenerator[ComponentConfigType]
38+
readinessGen ProbeGenerator[ComponentConfigType]
3739
}
3840

3941
func NewEmptySettings[ComponentConfigType any]() *Settings[ComponentConfigType] {
@@ -104,13 +106,32 @@ func (b Builder[ComponentConfigType]) WithRbacGen(rbacGen RBACRuleGenerator[Comp
104106
})
105107
}
106108

109+
func (b Builder[ComponentConfigType]) WithLivenessGen(livenessGen ProbeGenerator[ComponentConfigType]) Builder[ComponentConfigType] {
110+
return append(b, func(o *Settings[ComponentConfigType]) {
111+
o.livenessGen = livenessGen
112+
})
113+
}
114+
115+
func (b Builder[ComponentConfigType]) WithReadinessGen(readinessGen ProbeGenerator[ComponentConfigType]) Builder[ComponentConfigType] {
116+
return append(b, func(o *Settings[ComponentConfigType]) {
117+
o.readinessGen = readinessGen
118+
})
119+
}
120+
107121
func (b Builder[ComponentConfigType]) Build() (*GenericParser[ComponentConfigType], error) {
108122
o := NewEmptySettings[ComponentConfigType]()
109123
o.Apply(b...)
110124
if len(o.name) == 0 {
111125
return nil, fmt.Errorf("invalid settings struct, no name specified")
112126
}
113-
return &GenericParser[ComponentConfigType]{name: o.name, portParser: o.portParser, rbacGen: o.rbacGen, settings: o}, nil
127+
return &GenericParser[ComponentConfigType]{
128+
name: o.name,
129+
portParser: o.portParser,
130+
rbacGen: o.rbacGen,
131+
livenessGen: o.livenessGen,
132+
readinessGen: o.readinessGen,
133+
settings: o,
134+
}, nil
114135
}
115136

116137
func (b Builder[ComponentConfigType]) MustBuild() *GenericParser[ComponentConfigType] {

0 commit comments

Comments
 (0)