diff --git a/.chloggen/improve-probe-parsing.yaml b/.chloggen/improve-probe-parsing.yaml new file mode 100755 index 0000000000..ec9b3fe8c2 --- /dev/null +++ b/.chloggen/improve-probe-parsing.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Improves healthcheck parsing capabilities, allowing for future extensions to configure a healthcheck other than the v1 healthcheck extension. + +# One or more tracking issues related to the change +issues: [3184] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/apis/v1beta1/config.go b/apis/v1beta1/config.go index fce6a610f4..b34601bf05 100644 --- a/apis/v1beta1/config.go +++ b/apis/v1beta1/config.go @@ -31,6 +31,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/internal/components" "github.com/open-telemetry/opentelemetry-operator/internal/components/exporters" + "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" "github.com/open-telemetry/opentelemetry-operator/internal/components/processors" "github.com/open-telemetry/opentelemetry-operator/internal/components/receivers" ) @@ -41,10 +42,11 @@ const ( KindReceiver ComponentKind = iota KindExporter KindProcessor + KindExtension ) func (c ComponentKind) String() string { - return [...]string{"receiver", "exporter", "processor"}[c] + return [...]string{"receiver", "exporter", "processor", "extension"}[c] } // AnyConfig represent parts of the config. @@ -108,6 +110,7 @@ func (c *Config) GetEnabledComponents() map[ComponentKind]map[string]interface{} KindReceiver: {}, KindProcessor: {}, KindExporter: {}, + KindExtension: {}, } for _, pipeline := range c.Service.Pipelines { if pipeline == nil { @@ -123,6 +126,9 @@ func (c *Config) GetEnabledComponents() map[ComponentKind]map[string]interface{} toReturn[KindProcessor][componentId] = struct{}{} } } + for _, componentId := range c.Service.Extensions { + toReturn[KindExtension][componentId] = struct{}{} + } return toReturn } @@ -162,6 +168,8 @@ func (c *Config) getRbacRulesForComponentKinds(logger logr.Logger, componentKind } else { cfg = *c.Processors } + case KindExtension: + continue } for componentName := range enabledComponents[componentKind] { // 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 .. retriever = exporters.ParserFor cfg = c.Exporters case KindProcessor: - break + continue + case KindExtension: + continue } for componentName := range enabledComponents[componentKind] { // 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 return c.getRbacRulesForComponentKinds(logger, KindReceiver, KindExporter, KindProcessor) } +// GetLivenessProbe gets the first enabled liveness probe. There should only ever be one extension enabled +// that provides the hinting for the liveness probe. +func (c *Config) GetLivenessProbe(logger logr.Logger) (*corev1.Probe, error) { + enabledComponents := c.GetEnabledComponents() + for componentName := range enabledComponents[KindExtension] { + // TODO: Clean up the naming here and make it simpler to use a retriever. + parser := extensions.ParserFor(componentName) + if probe, err := parser.GetLivenessProbe(logger, c.Extensions.Object[componentName]); err != nil { + return nil, err + } else if probe != nil { + return probe, nil + } + } + return nil, nil +} + +// GetReadinessProbe gets the first enabled readiness probe. There should only ever be one extension enabled +// that provides the hinting for the readiness probe. +func (c *Config) GetReadinessProbe(logger logr.Logger) (*corev1.Probe, error) { + enabledComponents := c.GetEnabledComponents() + for componentName := range enabledComponents[KindExtension] { + // TODO: Clean up the naming here and make it simpler to use a retriever. + parser := extensions.ParserFor(componentName) + if probe, err := parser.GetReadinessProbe(logger, c.Extensions.Object[componentName]); err != nil { + return nil, err + } else if probe != nil { + return probe, nil + } + } + return nil, nil +} + // Yaml encodes the current object and returns it as a string. func (c *Config) Yaml() (string, error) { var buf bytes.Buffer @@ -268,7 +310,7 @@ func (c *Config) nullObjects() []string { } type Service struct { - Extensions *[]string `json:"extensions,omitempty" yaml:"extensions,omitempty"` + Extensions []string `json:"extensions,omitempty" yaml:"extensions,omitempty"` // +kubebuilder:pruning:PreserveUnknownFields Telemetry *AnyConfig `json:"telemetry,omitempty" yaml:"telemetry,omitempty"` // +kubebuilder:pruning:PreserveUnknownFields diff --git a/apis/v1beta1/config_test.go b/apis/v1beta1/config_test.go index 6f36d18a93..31895b3252 100644 --- a/apis/v1beta1/config_test.go +++ b/apis/v1beta1/config_test.go @@ -143,7 +143,7 @@ func TestConfigYaml(t *testing.T) { }, }, Service: Service{ - Extensions: &[]string{"addon"}, + Extensions: []string{"addon"}, Telemetry: &AnyConfig{ Object: map[string]interface{}{ "insights": "yeah!", @@ -304,6 +304,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) { "bar": struct{}{}, "count": struct{}{}, }, + KindExtension: {}, }, }, { @@ -321,6 +322,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) { KindExporter: { "prometheus": struct{}{}, }, + KindExtension: {}, }, }, { @@ -339,6 +341,11 @@ func TestConfig_GetEnabledComponents(t *testing.T) { "otlp": struct{}{}, "prometheus": struct{}{}, }, + KindExtension: { + "health_check": struct{}{}, + "pprof": struct{}{}, + "zpages": struct{}{}, + }, }, }, { @@ -352,6 +359,9 @@ func TestConfig_GetEnabledComponents(t *testing.T) { KindExporter: { "otlp/auth": struct{}{}, }, + KindExtension: { + "oauth2client": struct{}{}, + }, }, }, { @@ -365,6 +375,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) { KindExporter: { "debug": struct{}{}, }, + KindExtension: {}, }, }, { @@ -374,6 +385,7 @@ func TestConfig_GetEnabledComponents(t *testing.T) { KindReceiver: {}, KindProcessor: {}, KindExporter: {}, + KindExtension: {}, }, }, } diff --git a/apis/v1beta1/zz_generated.deepcopy.go b/apis/v1beta1/zz_generated.deepcopy.go index d0334a8051..eaf24ed0ba 100644 --- a/apis/v1beta1/zz_generated.deepcopy.go +++ b/apis/v1beta1/zz_generated.deepcopy.go @@ -635,12 +635,8 @@ func (in *Service) DeepCopyInto(out *Service) { *out = *in if in.Extensions != nil { in, out := &in.Extensions, &out.Extensions - *out = new([]string) - if **in != nil { - in, out := *in, *out - *out = make([]string, len(*in)) - copy(*out, *in) - } + *out = make([]string, len(*in)) + copy(*out, *in) } if in.Telemetry != nil { in, out := &in.Telemetry, &out.Telemetry diff --git a/internal/components/builder.go b/internal/components/builder.go index 7dc3ba186e..be459cc513 100644 --- a/internal/components/builder.go +++ b/internal/components/builder.go @@ -26,14 +26,16 @@ import ( type ParserOption[ComponentConfigType any] func(*Settings[ComponentConfigType]) type Settings[ComponentConfigType any] struct { - protocol corev1.Protocol - appProtocol *string - targetPort intstr.IntOrString - nodePort int32 - name string - port int32 - portParser PortParser[ComponentConfigType] - rbacGen RBACRuleGenerator[ComponentConfigType] + protocol corev1.Protocol + appProtocol *string + targetPort intstr.IntOrString + nodePort int32 + name string + port int32 + portParser PortParser[ComponentConfigType] + rbacGen RBACRuleGenerator[ComponentConfigType] + livenessGen ProbeGenerator[ComponentConfigType] + readinessGen ProbeGenerator[ComponentConfigType] } func NewEmptySettings[ComponentConfigType any]() *Settings[ComponentConfigType] { @@ -104,13 +106,32 @@ func (b Builder[ComponentConfigType]) WithRbacGen(rbacGen RBACRuleGenerator[Comp }) } +func (b Builder[ComponentConfigType]) WithLivenessGen(livenessGen ProbeGenerator[ComponentConfigType]) Builder[ComponentConfigType] { + return append(b, func(o *Settings[ComponentConfigType]) { + o.livenessGen = livenessGen + }) +} + +func (b Builder[ComponentConfigType]) WithReadinessGen(readinessGen ProbeGenerator[ComponentConfigType]) Builder[ComponentConfigType] { + return append(b, func(o *Settings[ComponentConfigType]) { + o.readinessGen = readinessGen + }) +} + func (b Builder[ComponentConfigType]) Build() (*GenericParser[ComponentConfigType], error) { o := NewEmptySettings[ComponentConfigType]() o.Apply(b...) if len(o.name) == 0 { return nil, fmt.Errorf("invalid settings struct, no name specified") } - return &GenericParser[ComponentConfigType]{name: o.name, portParser: o.portParser, rbacGen: o.rbacGen, settings: o}, nil + return &GenericParser[ComponentConfigType]{ + name: o.name, + portParser: o.portParser, + rbacGen: o.rbacGen, + livenessGen: o.livenessGen, + readinessGen: o.readinessGen, + settings: o, + }, nil } func (b Builder[ComponentConfigType]) MustBuild() *GenericParser[ComponentConfigType] { diff --git a/internal/components/builder_test.go b/internal/components/builder_test.go index d600db4a70..0e82f6bde3 100644 --- a/internal/components/builder_test.go +++ b/internal/components/builder_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/intstr" "github.com/open-telemetry/opentelemetry-operator/internal/components" ) @@ -33,9 +34,11 @@ func TestBuilder_Build(t *testing.T) { m map[string]interface{} } type want struct { - name string - ports []corev1.ServicePort - rules []rbacv1.PolicyRule + name string + ports []corev1.ServicePort + rules []rbacv1.PolicyRule + livenessProbe *corev1.Probe + readinessProbe *corev1.Probe } type fields[T any] struct { b components.Builder[T] @@ -44,12 +47,13 @@ func TestBuilder_Build(t *testing.T) { conf interface{} } type testCase[T any] struct { - name string - fields fields[T] - params params - want want - wantErr assert.ErrorAssertionFunc - wantRbacErr assert.ErrorAssertionFunc + name string + fields fields[T] + params params + want want + wantErr assert.ErrorAssertionFunc + wantRbacErr assert.ErrorAssertionFunc + wantLivenessErr assert.ErrorAssertionFunc } examplePortParser := func(logger logr.Logger, name string, defaultPort *corev1.ServicePort, config sampleConfig) ([]corev1.ServicePort, error) { if defaultPort != nil { @@ -57,6 +61,16 @@ func TestBuilder_Build(t *testing.T) { } return nil, nil } + exampleProbeGen := func(logger logr.Logger, config sampleConfig) (*corev1.Probe, error) { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, nil + } tests := []testCase[sampleConfig]{ { name: "basic valid configuration", @@ -83,8 +97,9 @@ func TestBuilder_Build(t *testing.T) { }, rules: nil, }, - wantErr: assert.NoError, - wantRbacErr: assert.NoError, + wantErr: assert.NoError, + wantRbacErr: assert.NoError, + wantLivenessErr: assert.NoError, }, { name: "missing name", @@ -96,9 +111,10 @@ func TestBuilder_Build(t *testing.T) { params: params{ conf: sampleConfig{}, }, - want: want{}, - wantErr: assert.Error, - wantRbacErr: assert.NoError, + want: want{}, + wantErr: assert.Error, + wantRbacErr: assert.NoError, + wantLivenessErr: assert.NoError, }, { name: "complete configuration with RBAC rules", @@ -147,8 +163,9 @@ func TestBuilder_Build(t *testing.T) { }, }, }, - wantErr: assert.NoError, - wantRbacErr: assert.NoError, + wantErr: assert.NoError, + wantRbacErr: assert.NoError, + wantLivenessErr: assert.NoError, }, { name: "complete configuration with RBAC rules errors", @@ -186,8 +203,81 @@ func TestBuilder_Build(t *testing.T) { ports: nil, rules: nil, }, - wantErr: assert.NoError, - wantRbacErr: assert.Error, + wantErr: assert.NoError, + wantRbacErr: assert.Error, + wantLivenessErr: assert.NoError, + }, + { + name: "complete configuration with probe gen", + fields: fields[sampleConfig]{ + b: components.NewBuilder[sampleConfig](). + WithName("secure-service"). + WithPort(443). + WithProtocol(corev1.ProtocolTCP). + WithLivenessGen(exampleProbeGen). + WithReadinessGen(exampleProbeGen), + }, + params: params{ + conf: sampleConfig{ + example: "test", + number: 100, + m: map[string]interface{}{ + "key": "value", + }, + }, + }, + want: want{ + name: "__secure-service", + ports: nil, + livenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + readinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + }, + wantErr: assert.NoError, + wantRbacErr: assert.NoError, + wantLivenessErr: assert.NoError, + }, + { + name: "complete configuration with probe gen errors", + fields: fields[sampleConfig]{ + b: components.NewBuilder[sampleConfig](). + WithName("secure-service"). + WithPort(443). + WithProtocol(corev1.ProtocolTCP). + WithLivenessGen(func(logger logr.Logger, config sampleConfig) (*corev1.Probe, error) { + return nil, fmt.Errorf("no probe") + }), + }, + params: params{ + conf: sampleConfig{ + example: "test", + number: 100, + m: map[string]interface{}{ + "key": "value", + }, + }, + }, + want: want{ + name: "__secure-service", + ports: nil, + rules: nil, + }, + wantErr: assert.NoError, + wantRbacErr: assert.NoError, + wantLivenessErr: assert.Error, }, } for _, tt := range tests { @@ -205,6 +295,14 @@ func TestBuilder_Build(t *testing.T) { return } assert.Equalf(t, tt.want.rules, rules, "GetRBACRules()") + livenessProbe, livenessErr := got.GetLivenessProbe(logr.Discard(), tt.params.conf) + if tt.wantLivenessErr(t, livenessErr, "wantLivenessErr()") && livenessErr != nil { + return + } + assert.Equalf(t, tt.want.livenessProbe, livenessProbe, "GetLivenessProbe()") + readinessProbe, readinessErr := got.GetReadinessProbe(logr.Discard(), tt.params.conf) + assert.NoError(t, readinessErr) + assert.Equalf(t, tt.want.readinessProbe, readinessProbe, "GetReadinessProbe()") }) } } diff --git a/internal/components/component.go b/internal/components/component.go index 36d3744839..f97daba497 100644 --- a/internal/components/component.go +++ b/internal/components/component.go @@ -45,6 +45,10 @@ type PortParser[ComponentConfigType any] func(logger logr.Logger, name string, d // It's expected that type Config is the configuration used by a parser. type RBACRuleGenerator[ComponentConfigType any] func(logger logr.Logger, config ComponentConfigType) ([]rbacv1.PolicyRule, error) +// ProbeGenerator is a function that generates a valid probe for a container given Config +// It's expected that type Config is the configuration used by a parser. +type ProbeGenerator[ComponentConfigType any] func(logger logr.Logger, config ComponentConfigType) (*corev1.Probe, error) + // ComponentType returns the type for a given component name. // components have a name like: // - mycomponent/custom @@ -90,6 +94,12 @@ type Parser interface { // GetRBACRules returns the rbac rules for this component GetRBACRules(logger logr.Logger, config interface{}) ([]rbacv1.PolicyRule, error) + // GetLivenessProbe returns a liveness probe set for the collector + GetLivenessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) + + // GetReadinessProbe returns a readiness probe set for the collector + GetReadinessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) + // ParserType returns the type of this parser ParserType() string diff --git a/internal/components/extensions/healthcheckv1.go b/internal/components/extensions/healthcheckv1.go new file mode 100644 index 0000000000..49769f2f8b --- /dev/null +++ b/internal/components/extensions/healthcheckv1.go @@ -0,0 +1,50 @@ +// Copyright The OpenTelemetry 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 extensions + +import ( + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/open-telemetry/opentelemetry-operator/internal/components" +) + +const ( + DefaultHealthcheckV1Path = "/" + DefaultHealthcheckV1Port = 13133 +) + +type healthcheckV1Config struct { + components.SingleEndpointConfig `mapstructure:",squash"` + Path string `mapstructure:"path"` +} + +// HealthCheckV1Probe returns the probe configuration for the healthcheck v1 extension. +// Right now no TLS config is parsed. +func HealthCheckV1Probe(logger logr.Logger, config healthcheckV1Config) (*corev1.Probe, error) { + path := config.Path + if len(path) == 0 { + path = DefaultHealthcheckV1Path + } + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: path, + Port: intstr.FromInt32(config.GetPortNumOrDefault(logger, DefaultHealthcheckV1Port)), + }, + }, + }, nil +} diff --git a/internal/components/extensions/healthcheckv1_test.go b/internal/components/extensions/healthcheckv1_test.go new file mode 100644 index 0000000000..52f857f864 --- /dev/null +++ b/internal/components/extensions/healthcheckv1_test.go @@ -0,0 +1,186 @@ +// Copyright The OpenTelemetry 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 extensions_test + +import ( + "fmt" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" +) + +func TestHealthCheckV1Probe(t *testing.T) { + type args struct { + config interface{} + } + tests := []struct { + name string + args args + want *corev1.Probe + wantErr assert.ErrorAssertionFunc + }{ + { + name: "Valid path and custom port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1:8080", + "path": "/healthz", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(8080), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Valid path and default port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1", + "path": "/healthz", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Empty path and custom port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1:9090", + "path": "", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(9090), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Empty path and default port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1", + "path": "", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Nil path and custom port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1:7070", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(7070), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Nil path and default port", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/", + Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "Invalid endpoint", + args: args{ + config: map[string]interface{}{ + "endpoint": 123, + "path": "/healthz", + }, + }, + want: nil, + wantErr: assert.Error, + }, + { + name: "Zero custom port, default port fallback", + args: args{ + config: map[string]interface{}{ + "endpoint": "127.0.0.1:0", + "path": "/healthz", + }, + }, + want: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(extensions.DefaultHealthcheckV1Port), + }, + }, + }, + wantErr: assert.NoError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := extensions.ParserFor("health_check") + got, err := parser.GetLivenessProbe(logr.Discard(), tt.args.config) + if !tt.wantErr(t, err, fmt.Sprintf("GetLivenessProbe(%v)", tt.args.config)) { + return + } + assert.Equalf(t, tt.want, got, "GetLivenessProbe(%v)", tt.args.config) + }) + } +} diff --git a/internal/components/extensions/helpers.go b/internal/components/extensions/helpers.go new file mode 100644 index 0000000000..d05a04f3d9 --- /dev/null +++ b/internal/components/extensions/helpers.go @@ -0,0 +1,65 @@ +// Copyright The OpenTelemetry 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 extensions + +import ( + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + + "github.com/open-telemetry/opentelemetry-operator/internal/components" +) + +// registry holds a record of all known receiver parsers. +var registry = make(map[string]components.Parser) + +// Register adds a new parser builder to the list of known builders. +func Register(name string, p components.Parser) { + registry[name] = p +} + +// IsRegistered checks whether a parser is registered with the given name. +func IsRegistered(name string) bool { + _, ok := registry[name] + return ok +} + +// ParserFor returns a parser builder for the given exporter name. +func ParserFor(name string) components.Parser { + if parser, ok := registry[components.ComponentType(name)]; ok { + return parser + } + // We want the default for exporters to fail silently. + return components.NewBuilder[any]().WithName(name).MustBuild() +} + +var ( + componentParsers = []components.Parser{ + components.NewBuilder[healthcheckV1Config](). + WithName("health_check"). + WithPort(13133). + WithReadinessGen(HealthCheckV1Probe). + WithLivenessGen(HealthCheckV1Probe). + WithPortParser(func(logger logr.Logger, name string, defaultPort *corev1.ServicePort, config healthcheckV1Config) ([]corev1.ServicePort, error) { + return components.ParseSingleEndpointSilent(logger, name, defaultPort, &config.SingleEndpointConfig) + }). + MustBuild(), + } +) + +func init() { + for _, parser := range componentParsers { + Register(parser.ParserType(), parser) + } +} diff --git a/internal/components/extensions/helpers_test.go b/internal/components/extensions/helpers_test.go new file mode 100644 index 0000000000..826072aef9 --- /dev/null +++ b/internal/components/extensions/helpers_test.go @@ -0,0 +1,111 @@ +// Copyright The OpenTelemetry 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 extensions_test + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + + "github.com/open-telemetry/opentelemetry-operator/internal/components" + "github.com/open-telemetry/opentelemetry-operator/internal/components/extensions" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func TestParserForReturns(t *testing.T) { + const testComponentName = "test" + parser := extensions.ParserFor(testComponentName) + assert.Equal(t, "test", parser.ParserType()) + assert.Equal(t, "__test", parser.ParserName()) + ports, err := parser.Ports(logr.Discard(), testComponentName, map[string]interface{}{ + "endpoint": "localhost:9000", + }) + assert.NoError(t, err) + assert.Len(t, ports, 0) // Should use the nop parser +} + +func TestCanRegister(t *testing.T) { + const testComponentName = "test" + extensions.Register(testComponentName, components.NewSinglePortParserBuilder(testComponentName, 9000).MustBuild()) + assert.True(t, extensions.IsRegistered(testComponentName)) + parser := extensions.ParserFor(testComponentName) + assert.Equal(t, "test", parser.ParserType()) + assert.Equal(t, "__test", parser.ParserName()) + ports, err := parser.Ports(logr.Discard(), testComponentName, map[string]interface{}{}) + assert.NoError(t, err) + assert.Len(t, ports, 1) + assert.Equal(t, ports[0].Port, int32(9000)) +} + +func TestExtensionsComponentParsers(t *testing.T) { + for _, tt := range []struct { + exporterName string + parserName string + defaultPort int + }{ + {"health_check", "__health_check", 13133}, + } { + t.Run(tt.exporterName, func(t *testing.T) { + t.Run("is registered", func(t *testing.T) { + assert.True(t, extensions.IsRegistered(tt.exporterName)) + }) + t.Run("bad config errors", func(t *testing.T) { + // prepare + parser := extensions.ParserFor(tt.exporterName) + + // test throwing in pure junk + _, err := parser.Ports(logr.Discard(), tt.exporterName, func() {}) + + // verify + assert.ErrorContains(t, err, "expected a map, got ") + }) + + t.Run("assigns the expected port", func(t *testing.T) { + // prepare + parser := extensions.ParserFor(tt.exporterName) + + // test + ports, err := parser.Ports(logr.Discard(), tt.exporterName, map[string]interface{}{}) + + if tt.defaultPort == 0 { + assert.Len(t, ports, 0) + return + } + // verify + assert.NoError(t, err) + assert.Len(t, ports, 1) + assert.EqualValues(t, tt.defaultPort, ports[0].Port) + assert.Equal(t, naming.PortName(tt.exporterName, int32(tt.defaultPort)), ports[0].Name) + }) + + t.Run("allows port to be overridden", func(t *testing.T) { + // prepare + parser := extensions.ParserFor(tt.exporterName) + + // test + ports, err := parser.Ports(logr.Discard(), tt.exporterName, map[string]interface{}{ + "endpoint": "0.0.0.0:65535", + }) + + // verify + assert.NoError(t, err) + assert.Len(t, ports, 1) + assert.EqualValues(t, 65535, ports[0].Port) + assert.Equal(t, naming.PortName(tt.exporterName, int32(tt.defaultPort)), ports[0].Name) + }) + }) + } +} diff --git a/internal/components/generic_parser.go b/internal/components/generic_parser.go index 4e3c3325ed..6bead9442c 100644 --- a/internal/components/generic_parser.go +++ b/internal/components/generic_parser.go @@ -30,10 +30,34 @@ var ( // GenericParser serves as scaffolding for custom parsing logic by isolating // functionality to idempotent functions. type GenericParser[T any] struct { - name string - settings *Settings[T] - portParser PortParser[T] - rbacGen RBACRuleGenerator[T] + name string + settings *Settings[T] + portParser PortParser[T] + rbacGen RBACRuleGenerator[T] + livenessGen ProbeGenerator[T] + readinessGen ProbeGenerator[T] +} + +func (g *GenericParser[T]) GetLivenessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) { + if g.livenessGen == nil { + return nil, nil + } + var parsed T + if err := mapstructure.Decode(config, &parsed); err != nil { + return nil, err + } + return g.livenessGen(logger, parsed) +} + +func (g *GenericParser[T]) GetReadinessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) { + if g.readinessGen == nil { + return nil, nil + } + var parsed T + if err := mapstructure.Decode(config, &parsed); err != nil { + return nil, err + } + return g.readinessGen(logger, parsed) } func (g *GenericParser[T]) GetRBACRules(logger logr.Logger, config interface{}) ([]rbacv1.PolicyRule, error) { diff --git a/internal/components/generic_parser_test.go b/internal/components/generic_parser_test.go index a271bcd2dd..6059313454 100644 --- a/internal/components/generic_parser_test.go +++ b/internal/components/generic_parser_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/util/intstr" "github.com/open-telemetry/opentelemetry-operator/internal/components" ) @@ -220,3 +221,154 @@ func TestGenericParser_GetRBACRules(t *testing.T) { }) } } + +func TestGenericParser_GetProbe(t *testing.T) { + type args struct { + logger logr.Logger + config interface{} + } + type testCase[T any] struct { + name string + g *components.GenericParser[T] + args args + livenessProbe *corev1.Probe + readinessProbe *corev1.Probe + wantLivenessErr assert.ErrorAssertionFunc + wantReadinessErr assert.ErrorAssertionFunc + } + probeFunc := func(logger logr.Logger, config *components.SingleEndpointConfig) (*corev1.Probe, error) { + if config.Endpoint == "" && config.ListenAddress == "" { + return nil, fmt.Errorf("either endpoint or listen_address must be specified") + } + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, nil + } + + tests := []testCase[*components.SingleEndpointConfig]{ + { + name: "valid config with endpoint", + g: components.NewSinglePortParserBuilder("test", 0).WithReadinessGen(probeFunc).WithLivenessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: map[string]interface{}{ + "endpoint": "http://localhost:8080", + }, + }, + livenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + readinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + wantLivenessErr: assert.NoError, + wantReadinessErr: assert.NoError, + }, + { + name: "valid config with listen_address", + g: components.NewSinglePortParserBuilder("test", 0).WithReadinessGen(probeFunc).WithLivenessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: map[string]interface{}{ + "listen_address": "0.0.0.0:9090", + }, + }, + livenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + readinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/hello", + Port: intstr.FromInt32(8080), + }, + }, + }, + wantLivenessErr: assert.NoError, + wantReadinessErr: assert.NoError, + }, + { + name: "readiness invalid config with no endpoint or listen_address", + g: components.NewSinglePortParserBuilder("test", 0).WithReadinessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: map[string]interface{}{}, + }, + readinessProbe: nil, + livenessProbe: nil, + wantReadinessErr: assert.Error, + wantLivenessErr: assert.NoError, + }, + { + name: "liveness invalid config with no endpoint or listen_address", + g: components.NewSinglePortParserBuilder("test", 0).WithLivenessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: map[string]interface{}{}, + }, + readinessProbe: nil, + livenessProbe: nil, + wantReadinessErr: assert.NoError, + wantLivenessErr: assert.Error, + }, + { + name: "liveness failed to parse config", + g: components.NewSinglePortParserBuilder("test", 0).WithLivenessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: func() {}, + }, + livenessProbe: nil, + readinessProbe: nil, + wantLivenessErr: assert.Error, + wantReadinessErr: assert.NoError, + }, + { + name: "readiness failed to parse config", + g: components.NewSinglePortParserBuilder("test", 0).WithReadinessGen(probeFunc).MustBuild(), + args: args{ + logger: logr.Discard(), + config: func() {}, + }, + livenessProbe: nil, + readinessProbe: nil, + wantLivenessErr: assert.NoError, + wantReadinessErr: assert.Error, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + livenessProbe, err := tt.g.GetLivenessProbe(tt.args.logger, tt.args.config) + if !tt.wantLivenessErr(t, err, fmt.Sprintf("GetLivenessProbe(%v, %v)", tt.args.logger, tt.args.config)) { + return + } + assert.Equalf(t, tt.livenessProbe, livenessProbe, "GetLivenessProbe(%v, %v)", tt.args.logger, tt.args.config) + readinessProbe, err := tt.g.GetReadinessProbe(tt.args.logger, tt.args.config) + if !tt.wantReadinessErr(t, err, fmt.Sprintf("GetReadinessProbe(%v, %v)", tt.args.logger, tt.args.config)) { + return + } + assert.Equalf(t, tt.readinessProbe, readinessProbe, "GetReadinessProbe(%v, %v)", tt.args.logger, tt.args.config) + }) + } +} diff --git a/internal/components/multi_endpoint.go b/internal/components/multi_endpoint.go index 7a10b90fd6..261700bb17 100644 --- a/internal/components/multi_endpoint.go +++ b/internal/components/multi_endpoint.go @@ -72,6 +72,14 @@ func (m *MultiPortReceiver) ParserName() string { return fmt.Sprintf("__%s", m.name) } +func (m *MultiPortReceiver) GetLivenessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) { + return nil, nil +} + +func (m *MultiPortReceiver) GetReadinessProbe(logger logr.Logger, config interface{}) (*corev1.Probe, error) { + return nil, nil +} + func (m *MultiPortReceiver) GetRBACRules(logr.Logger, interface{}) ([]rbacv1.PolicyRule, error) { return nil, nil } diff --git a/internal/components/multi_endpoint_test.go b/internal/components/multi_endpoint_test.go index deabf377a4..83baf24d5c 100644 --- a/internal/components/multi_endpoint_test.go +++ b/internal/components/multi_endpoint_test.go @@ -358,6 +358,12 @@ func TestMultiPortReceiver_Ports(t *testing.T) { rbacGen, err := s.GetRBACRules(logr.Discard(), tt.args.config) assert.NoError(t, err) assert.Nil(t, rbacGen) + livenessProbe, livenessErr := s.GetLivenessProbe(logr.Discard(), tt.args.config) + assert.NoError(t, livenessErr) + assert.Nil(t, livenessProbe) + readinessProbe, readinessErr := s.GetReadinessProbe(logr.Discard(), tt.args.config) + assert.NoError(t, readinessErr) + assert.Nil(t, readinessProbe) }) } } diff --git a/internal/manifests/collector/adapters/config_to_probe.go b/internal/manifests/collector/adapters/config_to_probe.go deleted file mode 100644 index 897b7db068..0000000000 --- a/internal/manifests/collector/adapters/config_to_probe.go +++ /dev/null @@ -1,158 +0,0 @@ -// Copyright The OpenTelemetry 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 adapters - -import ( - "errors" - "strings" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/intstr" -) - -var ( - errNoService = errors.New("no service available as part of the configuration") - errNoExtensions = errors.New("no extensions available as part of the configuration") - - errServiceNotAMap = errors.New("service property in the configuration doesn't contain valid services") - errExtensionsNotAMap = errors.New("extensions property in the configuration doesn't contain valid extensions") - - errNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") - - ErrNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") - - errServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") - ErrNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") -) - -type probeConfiguration struct { - path string - port intstr.IntOrString -} - -const ( - defaultHealthCheckPath = "/" - defaultHealthCheckPort = 13133 -) - -// ConfigToContainerProbe converts the incoming configuration object into a container probe or returns an error. -func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, error) { - serviceProperty, withService := config["service"] - if !withService { - return nil, errNoService - } - service, withSvcProperty := serviceProperty.(map[interface{}]interface{}) - if !withSvcProperty { - return nil, errServiceNotAMap - } - - serviceExtensionsProperty, withExtension := service["extensions"] - if !withExtension { - return nil, ErrNoServiceExtensions - } - - serviceExtensions, withExtProperty := serviceExtensionsProperty.([]interface{}) - if !withExtProperty { - return nil, errServiceExtensionsNotSlice - } - healthCheckServiceExtensions := make([]string, 0) - for _, ext := range serviceExtensions { - parsedExt, ok := ext.(string) - if ok && strings.HasPrefix(parsedExt, "health_check") { - healthCheckServiceExtensions = append(healthCheckServiceExtensions, parsedExt) - } - } - - if len(healthCheckServiceExtensions) == 0 { - return nil, ErrNoServiceExtensionHealthCheck - } - - extensionsProperty, ok := config["extensions"] - if !ok { - return nil, errNoExtensions - } - extensions, ok := extensionsProperty.(map[interface{}]interface{}) - if !ok { - return nil, errExtensionsNotAMap - } - // in the event of multiple health_check service extensions defined, we arbitrarily take the first one found - for _, healthCheckForProbe := range healthCheckServiceExtensions { - healthCheckExtension, ok := extensions[healthCheckForProbe] - if ok { - return createProbeFromExtension(healthCheckExtension) - } - } - - return nil, errNoExtensionHealthCheck -} - -func createProbeFromExtension(extension interface{}) (*corev1.Probe, error) { - probeCfg := extractProbeConfigurationFromExtension(extension) - return &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: probeCfg.path, - Port: probeCfg.port, - }, - }, - }, nil -} - -func extractProbeConfigurationFromExtension(ext interface{}) probeConfiguration { - extensionCfg, ok := ext.(map[interface{}]interface{}) - if !ok { - return defaultProbeConfiguration() - } - return probeConfiguration{ - path: extractPathFromExtensionConfig(extensionCfg), - port: extractPortFromExtensionConfig(extensionCfg), - } -} - -func defaultProbeConfiguration() probeConfiguration { - return probeConfiguration{ - path: defaultHealthCheckPath, - port: intstr.FromInt(defaultHealthCheckPort), - } -} - -func extractPathFromExtensionConfig(cfg map[interface{}]interface{}) string { - if path, ok := cfg["path"]; ok { - if parsedPath, ok := path.(string); ok { - return parsedPath - } - } - return defaultHealthCheckPath -} - -func extractPortFromExtensionConfig(cfg map[interface{}]interface{}) intstr.IntOrString { - endpoint, ok := cfg["endpoint"] - if !ok { - return defaultHealthCheckEndpoint() - } - parsedEndpoint, ok := endpoint.(string) - if !ok { - return defaultHealthCheckEndpoint() - } - endpointComponents := strings.Split(parsedEndpoint, ":") - if len(endpointComponents) != 2 { - return defaultHealthCheckEndpoint() - } - return intstr.Parse(endpointComponents[1]) -} - -func defaultHealthCheckEndpoint() intstr.IntOrString { - return intstr.FromInt(defaultHealthCheckPort) -} diff --git a/internal/manifests/collector/adapters/config_to_probe_test.go b/internal/manifests/collector/adapters/config_to_probe_test.go deleted file mode 100644 index 89e1f97349..0000000000 --- a/internal/manifests/collector/adapters/config_to_probe_test.go +++ /dev/null @@ -1,187 +0,0 @@ -// Copyright The OpenTelemetry 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 adapters - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestConfigToProbeShouldCreateProbeFor(t *testing.T) { - tests := []struct { - desc string - config string - expectedPath string - expectedPort int32 - }{ - { - desc: "SimpleHappyPath", - expectedPort: int32(13133), - expectedPath: "/", - config: `extensions: - health_check: -service: - extensions: [health_check]`, - }, { - desc: "CustomEndpointAndPath", - expectedPort: int32(1234), - expectedPath: "/checkit", - config: `extensions: - health_check: - endpoint: localhost:1234 - path: /checkit -service: - extensions: [health_check]`, - }, { - desc: "CustomEndpointAndDefaultPath", - expectedPort: int32(1234), - expectedPath: "/", - config: `extensions: - health_check: - endpoint: localhost:1234 -service: - extensions: [health_check]`, - }, { - desc: "CustomEndpointWithJustPortAndDefaultPath", - expectedPort: int32(1234), - expectedPath: "/", - config: `extensions: - health_check: - endpoint: :1234 -service: - extensions: [health_check]`, - }, { - desc: "DefaultEndpointAndCustomPath", - expectedPort: int32(13133), - expectedPath: "/checkit", - config: `extensions: - health_check: - path: /checkit -service: - extensions: [health_check]`, - }, { - desc: "DefaultEndpointForUnexpectedEndpoint", - expectedPort: int32(13133), - expectedPath: "/", - config: `extensions: - health_check: - endpoint: 0:0:0" -service: - extensions: [health_check]`, - }, { - desc: "DefaultEndpointForUnparseablendpoint", - expectedPort: int32(13133), - expectedPath: "/", - config: `extensions: - health_check: - endpoint: - this: should-not-be-a-map" -service: - extensions: [health_check]`, - }, { - desc: "WillUseSecondServiceExtension", - config: `extensions: - health_check: -service: - extensions: [health_check/1, health_check]`, - expectedPort: int32(13133), - expectedPath: "/", - }, - } - - for _, test := range tests { - // prepare - config, err := ConfigFromString(test.config) - require.NoError(t, err, test.desc) - require.NotEmpty(t, config, test.desc) - - // test - actualProbe, err := ConfigToContainerProbe(config) - assert.NoError(t, err) - assert.Equal(t, test.expectedPath, actualProbe.HTTPGet.Path, test.desc) - assert.Equal(t, test.expectedPort, actualProbe.HTTPGet.Port.IntVal, test.desc) - assert.Equal(t, "", actualProbe.HTTPGet.Host, test.desc) - } -} - -func TestConfigToProbeShouldErrorIf(t *testing.T) { - tests := []struct { - expectedErr error - desc string - config string - }{ - { - desc: "NoHealthCheckExtension", - config: `extensions: - pprof: -service: - extensions: [health_check]`, - expectedErr: errNoExtensionHealthCheck, - }, { - desc: "BadlyFormattedExtensions", - config: `extensions: [hi] -service: - extensions: [health_check]`, - expectedErr: errExtensionsNotAMap, - }, { - desc: "NoExtensions", - config: `service: - extensions: [health_check]`, - expectedErr: errNoExtensions, - }, { - desc: "NoHealthCheckInServiceExtensions", - config: `service: - extensions: [pprof]`, - expectedErr: ErrNoServiceExtensionHealthCheck, - }, { - desc: "BadlyFormattedServiceExtensions", - config: `service: - extensions: - this: should-not-be-a-map`, - expectedErr: errServiceExtensionsNotSlice, - }, { - desc: "NoServiceExtensions", - config: `service: - pipelines: - traces: - receivers: [otlp]`, - expectedErr: ErrNoServiceExtensions, - }, { - desc: "BadlyFormattedService", - config: `extensions: - health_check: -service: [hi]`, - expectedErr: errServiceNotAMap, - }, { - desc: "NoService", - config: `extensions: - health_check:`, - expectedErr: errNoService, - }, - } - - for _, test := range tests { - // prepare - config, err := ConfigFromString(test.config) - require.NoError(t, err, test.desc) - require.NotEmpty(t, config, test.desc) - - // test - _, err = ConfigToContainerProbe(config) - assert.Equal(t, test.expectedErr, err, test.desc) - } -} diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index de53314ffb..3cf0b1a2e4 100644 --- a/internal/manifests/collector/container.go +++ b/internal/manifests/collector/container.go @@ -15,7 +15,6 @@ package collector import ( - "errors" "fmt" "path" "sort" @@ -27,7 +26,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/naming" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) @@ -43,12 +41,6 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme image = cfg.CollectorImage() } - configYaml, err := otelcol.Spec.Config.Yaml() - if err != nil { - logger.Error(err, "could not convert json to yaml") - return corev1.Container{} - } - // build container ports from service ports ports, err := getConfigContainerPorts(logger, otelcol.Spec.Config) if err != nil { @@ -140,28 +132,17 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme }) } - var livenessProbe *corev1.Probe - var readinessProbe *corev1.Probe - if configFromString, err := adapters.ConfigFromString(configYaml); err == nil { - if probe, err := getProbe(configFromString, otelcol.Spec.LivenessProbe); err == nil { - livenessProbe = probe - } else if errors.Is(err, adapters.ErrNoServiceExtensions) { - logger.V(4).Info("extensions not configured, skipping liveness probe creation") - } else if errors.Is(err, adapters.ErrNoServiceExtensionHealthCheck) { - logger.V(4).Info("healthcheck extension not configured, skipping liveness probe creation") - } else { - logger.Error(err, "cannot create liveness probe.") - } - - if probe, err := getProbe(configFromString, otelcol.Spec.ReadinessProbe); err == nil { - readinessProbe = probe - } else if errors.Is(err, adapters.ErrNoServiceExtensions) { - logger.V(4).Info("extensions not configured, skipping readiness probe creation") - } else if errors.Is(err, adapters.ErrNoServiceExtensionHealthCheck) { - logger.V(4).Info("healthcheck extension not configured, skipping readiness probe creation") - } else { - logger.Error(err, "cannot create readiness probe.") - } + livenessProbe, livenessProbeErr := otelcol.Spec.Config.GetLivenessProbe(logger) + if livenessProbeErr != nil { + logger.Error(livenessProbeErr, "cannot create liveness probe.") + } else { + defaultProbeSettings(livenessProbe, otelcol.Spec.LivenessProbe) + } + readinessProbe, readinessProbeErr := otelcol.Spec.Config.GetReadinessProbe(logger) + if readinessProbeErr != nil { + logger.Error(readinessProbeErr, "cannot create readiness probe.") + } else { + defaultProbeSettings(readinessProbe, otelcol.Spec.ReadinessProbe) } if featuregate.SetGolangFlags.IsEnabled() { @@ -257,12 +238,8 @@ func portMapToList(portMap map[string]corev1.ContainerPort) []corev1.ContainerPo return ports } -func getProbe(config map[interface{}]interface{}, probeConfig *v1beta1.Probe) (*corev1.Probe, error) { - probe, err := adapters.ConfigToContainerProbe(config) - if err != nil { - return nil, err - } - if probeConfig != nil { +func defaultProbeSettings(probe *corev1.Probe, probeConfig *v1beta1.Probe) { + if probe != nil && probeConfig != nil { if probeConfig.InitialDelaySeconds != nil { probe.InitialDelaySeconds = *probeConfig.InitialDelaySeconds } @@ -280,5 +257,4 @@ func getProbe(config map[interface{}]interface{}, probeConfig *v1beta1.Probe) (* } probe.TerminationGracePeriodSeconds = probeConfig.TerminationGracePeriodSeconds } - return probe, nil } diff --git a/internal/manifests/manifestutils/annotations_test.go b/internal/manifests/manifestutils/annotations_test.go index 818adff3f7..7ae4673a8e 100644 --- a/internal/manifests/manifestutils/annotations_test.go +++ b/internal/manifests/manifestutils/annotations_test.go @@ -34,10 +34,7 @@ func TestDefaultAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{ Config: v1beta1.Config{ Service: v1beta1.Service{ - Extensions: func() *[]string { - res := []string{"test"} - return &res - }(), + Extensions: []string{"test"}, }, }, }, @@ -101,10 +98,7 @@ func TestUserAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{ Config: v1beta1.Config{ Service: v1beta1.Service{ - Extensions: func() *[]string { - res := []string{"test2"} - return &res - }(), + Extensions: []string{"test2"}, }, }, },