Skip to content

Commit

Permalink
Create service for extensions (#3403)
Browse files Browse the repository at this point in the history
* feat: create service for extensions

Signed-off-by: Ankit152 <[email protected]>

* chore: added extension service in manifest factories

Signed-off-by: Ankit152 <[email protected]>

* chore: added unit test for extension service function

Signed-off-by: Ankit152 <[email protected]>

* chore: added e2e tests for extensions

Signed-off-by: Ankit152 <[email protected]>

---------

Signed-off-by: Ankit152 <[email protected]>
  • Loading branch information
Ankit152 authored Nov 27, 2024
1 parent 54caa0e commit 6ae647a
Show file tree
Hide file tree
Showing 10 changed files with 461 additions and 4 deletions.
16 changes: 16 additions & 0 deletions .chloggen/service-extension.yaml
Original file line number Diff line number Diff line change
@@ -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: support for creating a service for extensions when ports are specified.

# One or more tracking issues related to the change
issues: [3460]

# (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:
17 changes: 15 additions & 2 deletions apis/v1beta1/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@ func (c *Config) getPortsForComponentKinds(logger logr.Logger, componentKinds ..
case KindProcessor:
continue
case KindExtension:
continue
retriever = extensions.ParserFor
if c.Extensions == nil {
cfg = AnyConfig{}
} else {
cfg = *c.Extensions
}
}
for componentName := range enabledComponents[componentKind] {
// TODO: Clean up the naming here and make it simpler to use a retriever.
Expand Down Expand Up @@ -318,10 +323,18 @@ func (c *Config) GetExporterPorts(logger logr.Logger) ([]corev1.ServicePort, err
return c.getPortsForComponentKinds(logger, KindExporter)
}

func (c *Config) GetAllPorts(logger logr.Logger) ([]corev1.ServicePort, error) {
func (c *Config) GetExtensionPorts(logger logr.Logger) ([]corev1.ServicePort, error) {
return c.getPortsForComponentKinds(logger, KindExtension)
}

func (c *Config) GetReceiverAndExporterPorts(logger logr.Logger) ([]corev1.ServicePort, error) {
return c.getPortsForComponentKinds(logger, KindReceiver, KindExporter)
}

func (c *Config) GetAllPorts(logger logr.Logger) ([]corev1.ServicePort, error) {
return c.getPortsForComponentKinds(logger, KindReceiver, KindExporter, KindExtension)
}

func (c *Config) GetEnvironmentVariables(logger logr.Logger) ([]corev1.EnvVar, error) {
return c.getEnvironmentVariablesForComponentKinds(logger, KindReceiver)
}
Expand Down
3 changes: 3 additions & 0 deletions internal/components/extensions/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ var (
return components.ParseSingleEndpointSilent(logger, name, defaultPort, &config.SingleEndpointConfig)
}).
MustBuild(),
components.NewSinglePortParserBuilder("jaeger_query", 16686).
WithTargetPort(16686).
MustBuild(),
}
)

Expand Down
1 change: 1 addition & 0 deletions internal/manifests/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func Build(params manifests.Params) ([]client.Object, error) {
manifests.Factory(Service),
manifests.Factory(HeadlessService),
manifests.Factory(MonitoringService),
manifests.Factory(ExtensionService),
manifests.Factory(Ingress),
}...)

Expand Down
38 changes: 36 additions & 2 deletions internal/manifests/collector/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ const (
BaseServiceType ServiceType = iota
HeadlessServiceType
MonitoringServiceType
ExtensionServiceType
)

func (s ServiceType) String() string {
return [...]string{"base", "headless", "monitoring"}[s]
return [...]string{"base", "headless", "monitoring", "extension"}[s]
}

func HeadlessService(params manifests.Params) (*corev1.Service, error) {
Expand Down Expand Up @@ -108,6 +109,39 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
}, nil
}

func ExtensionService(params manifests.Params) (*corev1.Service, error) {
name := naming.ExtensionService(params.OtelCol.Name)
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
labels[serviceTypeLabel] = ExtensionServiceType.String()

annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter())
if err != nil {
return nil, err
}

ports, err := params.OtelCol.Spec.Config.GetExtensionPorts(params.Log)
if err != nil {
return nil, err
}

if len(ports) == 0 {
return nil, nil
}

return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: params.OtelCol.Namespace,
Labels: labels,
Annotations: annotations,
},
Spec: corev1.ServiceSpec{
Ports: ports,
Selector: manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector),
},
}, nil
}

func Service(params manifests.Params) (*corev1.Service, error) {
name := naming.Service(params.OtelCol.Name)
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
Expand All @@ -118,7 +152,7 @@ func Service(params manifests.Params) (*corev1.Service, error) {
return nil, err
}

ports, err := params.OtelCol.Spec.Config.GetAllPorts(params.Log)
ports, err := params.OtelCol.Spec.Config.GetReceiverAndExporterPorts(params.Log)
if err != nil {
return nil, err
}
Expand Down
201 changes: 201 additions & 0 deletions internal/manifests/collector/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

func TestExtractPortNumbersAndNames(t *testing.T) {
Expand Down Expand Up @@ -321,6 +322,206 @@ func TestMonitoringService(t *testing.T) {
})
}

func TestExtensionService(t *testing.T) {
testCases := []struct {
name string
params manifests.Params
expectedPorts []v1.ServicePort
}{
{
name: "when the extension has http endpoint",
params: manifests.Params{
Config: config.Config{},
Log: logger,
OtelCol: v1beta1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
Config: v1beta1.Config{
Service: v1beta1.Service{
Extensions: []string{"jaeger_query"},
},
Extensions: &v1beta1.AnyConfig{
Object: map[string]interface{}{
"jaeger_query": map[string]interface{}{
"http": map[string]interface{}{
"endpoint": "0.0.0.0:16686",
},
},
},
},
},
},
},
},
expectedPorts: []v1.ServicePort{
{
Name: "jaeger-query",
Port: 16686,
TargetPort: intstr.IntOrString{
IntVal: 16686,
},
},
},
},
{
name: "when the extension has grpc endpoint",
params: manifests.Params{
Config: config.Config{},
Log: logger,
OtelCol: v1beta1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
Config: v1beta1.Config{
Service: v1beta1.Service{
Extensions: []string{"jaeger_query"},
},
Extensions: &v1beta1.AnyConfig{
Object: map[string]interface{}{
"jaeger_query": map[string]interface{}{
"http": map[string]interface{}{
"endpoint": "0.0.0.0:16686",
},
},
},
},
},
},
},
},
expectedPorts: []v1.ServicePort{
{
Name: "jaeger-query",
Port: 16686,
TargetPort: intstr.IntOrString{
IntVal: 16686,
},
},
},
},
{
name: "when the extension has both http and grpc endpoint",
params: manifests.Params{
Config: config.Config{},
Log: logger,
OtelCol: v1beta1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
Config: v1beta1.Config{
Service: v1beta1.Service{
Extensions: []string{"jaeger_query"},
},
Extensions: &v1beta1.AnyConfig{
Object: map[string]interface{}{
"jaeger_query": map[string]interface{}{
"http": map[string]interface{}{
"endpoint": "0.0.0.0:16686",
},
"grpc": map[string]interface{}{
"endpoint": "0.0.0.0:16686",
},
},
},
},
},
},
},
},
expectedPorts: []v1.ServicePort{
{
Name: "jaeger-query",
Port: 16686,
TargetPort: intstr.IntOrString{
IntVal: 16686,
},
},
},
},
{
name: "when the extension has no extensions defined",
params: manifests.Params{
Config: config.Config{},
Log: logger,
OtelCol: v1beta1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
Config: v1beta1.Config{
Service: v1beta1.Service{
Extensions: []string{"jaeger_query"},
},
Extensions: &v1beta1.AnyConfig{
Object: map[string]interface{}{},
},
},
},
},
},
expectedPorts: []v1.ServicePort{},
},
{
name: "when the extension has no endpoint defined",
params: manifests.Params{
Config: config.Config{},
Log: logger,
OtelCol: v1beta1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1beta1.OpenTelemetryCollectorSpec{
Config: v1beta1.Config{
Service: v1beta1.Service{
Extensions: []string{"jaeger_query"},
},
Extensions: &v1beta1.AnyConfig{
Object: map[string]interface{}{
"jaeger_query": map[string]interface{}{},
},
},
},
},
},
},
expectedPorts: []v1.ServicePort{
{
Name: "jaeger-query",
Port: 16686,
TargetPort: intstr.IntOrString{
IntVal: 16686,
},
},
},
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
actual, err := ExtensionService(tc.params)
assert.NoError(t, err)

if len(tc.expectedPorts) > 0 {
assert.NotNil(t, actual)
assert.Equal(t, actual.Name, naming.ExtensionService(tc.params.OtelCol.Name))
// ports assertion
assert.Equal(t, len(tc.expectedPorts), len(actual.Spec.Ports))
assert.Equal(t, tc.expectedPorts[0].Name, actual.Spec.Ports[0].Name)
assert.Equal(t, tc.expectedPorts[0].Port, actual.Spec.Ports[0].Port)
assert.Equal(t, tc.expectedPorts[0].TargetPort.IntVal, actual.Spec.Ports[0].TargetPort.IntVal)
} else {
// no ports, no service
assert.Nil(t, actual)
}
})
}
}

func service(name string, ports []v1beta1.PortsSpec) v1.Service {
return serviceWithInternalTrafficPolicy(name, ports, v1.ServiceInternalTrafficPolicyCluster)
}
Expand Down
5 changes: 5 additions & 0 deletions internal/naming/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ func MonitoringService(otelcol string) string {
return DNSName(Truncate("%s-monitoring", 63, Service(otelcol)))
}

// ExtensionService builds the name for the extension service based on the instance.
func ExtensionService(otelcol string) string {
return DNSName(Truncate("%s-extension", 63, Service(otelcol)))
}

// Service builds the service name based on the instance.
func Service(otelcol string) string {
return DNSName(Truncate("%s-collector", 63, otelcol))
Expand Down
Loading

0 comments on commit 6ae647a

Please sign in to comment.