Skip to content

Commit

Permalink
squash
Browse files Browse the repository at this point in the history
  • Loading branch information
jaronoff97 committed Aug 2, 2023
1 parent 89915ad commit 3d16dc1
Show file tree
Hide file tree
Showing 35 changed files with 237 additions and 208 deletions.
16 changes: 16 additions & 0 deletions .chloggen/1954.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: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix port name matching between ingress/route and service. All ports are truncated to 15 characters. If the port name is longer it is changed to port-%d pattern.

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

# (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:
8 changes: 4 additions & 4 deletions internal/manifests/collector/adapters/config_to_ports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ func TestExtractPortsFromConfig(t *testing.T) {

expectedPorts := []corev1.ServicePort{
{Name: "examplereceiver", Port: 12345},
{Name: "examplereceiver-settings", Port: 12346},
{Name: "jaeger-custom-thrift-http", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 15268, TargetPort: targetPortZero},
{Name: "port-12346", Port: 12346},
{Name: "port-15268", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 15268, TargetPort: targetPortZero},
{Name: "jaeger-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 14250},
{Name: "jaeger-thrift-binary", Protocol: "UDP", Port: 6833},
{Name: "jaeger-thrift-compact", Protocol: "UDP", Port: 6831},
{Name: "port-6833", Protocol: "UDP", Port: 6833},
{Name: "port-6831", Protocol: "UDP", Port: 6831},
{Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 55555},
{Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: 4317, TargetPort: targetPort4317},
{Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318},
Expand Down
11 changes: 5 additions & 6 deletions internal/manifests/collector/config_replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@ package collector
import (
"time"

"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"

promconfig "github.com/prometheus/prometheus/config"
_ "github.com/prometheus/prometheus/discovery/install" // Package install has the side-effect of registering all builtin.
"gopkg.in/yaml.v2"

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
)

Expand Down Expand Up @@ -67,7 +66,7 @@ func ReplaceConfig(instance v1alpha1.OpenTelemetryCollector) (string, error) {
if featuregate.EnableTargetAllocatorRewrite.IsEnabled() {
// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance))
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
if getCfgPromErr != nil {
return "", getCfgPromErr
}
Expand All @@ -85,7 +84,7 @@ func ReplaceConfig(instance v1alpha1.OpenTelemetryCollector) (string, error) {

// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
updPromCfgMap, err := ta.AddHTTPSDConfigToPromConfig(promCfgMap, naming.TAService(instance))
updPromCfgMap, err := ta.AddHTTPSDConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
if err != nil {
return "", err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/collector/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
)

func ConfigMap(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.ConfigMap {
name := naming.ConfigMap(otelcol)
name := naming.ConfigMap(otelcol.Name)
labels := Labels(otelcol, name, []string{})

replacedConf, err := ReplaceConfig(otelcol)
Expand Down
10 changes: 5 additions & 5 deletions internal/manifests/collector/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
naming2 "github.com/open-telemetry/opentelemetry-operator/internal/naming"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

// maxPortLen allows us to truncate a port name according to what is considered valid port syntax:
Expand Down Expand Up @@ -74,7 +74,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem
args = append(args, fmt.Sprintf("--config=/conf/%s", cfg.CollectorConfigMapEntry()))
volumeMounts = append(volumeMounts,
corev1.VolumeMount{
Name: naming2.ConfigMapVolume(),
Name: naming.ConfigMapVolume(),
MountPath: "/conf",
})
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem
}

return corev1.Container{
Name: naming2.Container(),
Name: naming.Container(),
Image: image,
ImagePullPolicy: otelcol.Spec.ImagePullPolicy,
Ports: portMapToList(ports),
Expand All @@ -156,7 +156,7 @@ func getConfigContainerPorts(logger logr.Logger, cfg string) map[string]corev1.C
logger.Error(err, "couldn't build container ports from configuration")
} else {
for _, p := range ps {
truncName := naming2.Truncate(p.Name, maxPortLen)
truncName := naming.Truncate(p.Name, maxPortLen)
if p.Name != truncName {
logger.Info("truncating container port name",
"port.name.prev", p.Name, "port.name.new", truncName)
Expand Down Expand Up @@ -232,7 +232,7 @@ func getPrometheusExporterPorts(c map[interface{}]interface{}) ([]corev1.Contain
}
ports = append(ports,
corev1.ContainerPort{
Name: naming2.PortName(exporterName),
Name: naming.PortName(exporterName, containerPort),
ContainerPort: containerPort,
Protocol: corev1.ProtocolTCP,
},
Expand Down
4 changes: 2 additions & 2 deletions internal/manifests/collector/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ import (

// DaemonSet builds the deployment for the given instance.
func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.DaemonSet {
name := naming.Collector(otelcol)
name := naming.Collector(otelcol.Name)
labels := Labels(otelcol, name, cfg.LabelsFilter())

annotations := Annotations(otelcol)
podAnnotations := PodAnnotations(otelcol)
return &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: naming.Collector(otelcol),
Name: naming.Collector(otelcol.Name),
Namespace: otelcol.Namespace,
Labels: labels,
Annotations: annotations,
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/collector/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

// Deployment builds the deployment for the given instance.
func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.Deployment {
name := naming.Collector(otelcol)
name := naming.Collector(otelcol.Name)
labels := Labels(otelcol, name, cfg.LabelsFilter())

annotations := Annotations(otelcol)
Expand Down
8 changes: 4 additions & 4 deletions internal/manifests/collector/horizontalpodautoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ import (
)

func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) client.Object {
name := naming.Collector(otelcol)
name := naming.Collector(otelcol.Name)
labels := Labels(otelcol, name, cfg.LabelsFilter())
annotations := Annotations(otelcol)
var result client.Object

objectMeta := metav1.ObjectMeta{
Name: naming.HorizontalPodAutoscaler(otelcol),
Name: naming.HorizontalPodAutoscaler(otelcol.Name),
Namespace: otelcol.Namespace,
Labels: labels,
Annotations: annotations,
Expand Down Expand Up @@ -95,7 +95,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al
ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "OpenTelemetryCollector",
Name: naming.OpenTelemetryCollector(otelcol),
Name: naming.OpenTelemetryCollector(otelcol.Name),
},
MinReplicas: otelcol.Spec.Autoscaler.MinReplicas,
MaxReplicas: *otelcol.Spec.Autoscaler.MaxReplicas,
Expand Down Expand Up @@ -152,7 +152,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al
ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{
APIVersion: v1alpha1.GroupVersion.String(),
Kind: "OpenTelemetryCollector",
Name: naming.OpenTelemetryCollector(otelcol),
Name: naming.OpenTelemetryCollector(otelcol.Name),
},
MinReplicas: otelcol.Spec.Autoscaler.MinReplicas,
MaxReplicas: *otelcol.Spec.Autoscaler.MaxReplicas,
Expand Down
13 changes: 6 additions & 7 deletions internal/manifests/collector/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ package collector
import (
"fmt"

"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
naming2 "github.com/open-telemetry/opentelemetry-operator/internal/naming"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"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"
)

func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *networkingv1.Ingress {
Expand Down Expand Up @@ -54,10 +53,10 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet
PathType: &pathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: naming2.Service(otelcol),
Name: naming.Service(otelcol.Name),
Port: networkingv1.ServiceBackendPort{
// Valid names must be non-empty and no more than 15 characters long.
Name: naming2.Truncate(p.Name, 15),
Name: naming.Truncate(p.Name, 15),
},
},
},
Expand All @@ -66,11 +65,11 @@ func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemet

return &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: naming2.Ingress(otelcol),
Name: naming.Ingress(otelcol.Name),
Namespace: otelcol.Namespace,
Annotations: otelcol.Spec.Ingress.Annotations,
Labels: map[string]string{
"app.kubernetes.io/name": naming2.Ingress(otelcol),
"app.kubernetes.io/name": naming.Ingress(otelcol.Name),
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", otelcol.Namespace, otelcol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
},
Expand Down
4 changes: 2 additions & 2 deletions internal/manifests/collector/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,11 @@ func TestDesiredIngresses(t *testing.T) {

assert.NotEqual(t, &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: naming.Ingress(params.Instance),
Name: naming.Ingress(params.Instance.Name),
Namespace: ns,
Annotations: params.Instance.Spec.Ingress.Annotations,
Labels: map[string]string{
"app.kubernetes.io/name": naming.Ingress(params.Instance),
"app.kubernetes.io/name": naming.Ingress(params.Instance.Name),
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
},
Expand Down
23 changes: 2 additions & 21 deletions internal/manifests/collector/parser/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@ import (
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
)

var (
// DNS_LABEL constraints: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names
dnsLabelValidation = regexp.MustCompile("^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$")
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

// ReceiverParser is an interface that should be implemented by all receiver parsers.
Expand Down Expand Up @@ -159,7 +156,7 @@ func singlePortFromConfigEndpoint(logger logr.Logger, name string, config map[in
}

return &corev1.ServicePort{
Name: portName(name, port),
Name: naming.PortName(name, port),
Port: port,
}
default:
Expand All @@ -178,22 +175,6 @@ func getAddressFromConfig(logger logr.Logger, name, key string, config map[inter
return endpoint
}

func portName(receiverName string, port int32) string {
if len(receiverName) > 63 {
return fmt.Sprintf("port-%d", port)
}

candidate := strings.ReplaceAll(receiverName, "/", "-")
candidate = strings.ReplaceAll(candidate, "_", "-")

if !dnsLabelValidation.MatchString(candidate) {
return fmt.Sprintf("port-%d", port)
}

// matches the pattern and has less than 63 chars -- the candidate name is good to go!
return candidate
}

func portFromEndpoint(endpoint string) (int32, error) {
var err error
var port int64
Expand Down
4 changes: 3 additions & 1 deletion internal/manifests/collector/parser/receiver_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package parser
import (
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"

"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

const parserNameGeneric = "__generic"
Expand Down Expand Up @@ -59,7 +61,7 @@ func (g *GenericReceiver) Ports() ([]corev1.ServicePort, error) {
if g.defaultPort > 0 {
return []corev1.ServicePort{{
Port: g.defaultPort,
Name: portName(g.name, g.defaultPort),
Name: naming.PortName(g.name, g.defaultPort),
Protocol: g.defaultProtocol,
AppProtocol: g.defaultAppProtocol,
}}, nil
Expand Down
4 changes: 3 additions & 1 deletion internal/manifests/collector/parser/receiver_jaeger.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"

"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

var _ ReceiverParser = &JaegerReceiverParser{}
Expand Down Expand Up @@ -104,7 +106,7 @@ func (j *JaegerReceiverParser) Ports() ([]corev1.ServicePort, error) {
// if not, we use the default port
if protocolPort == nil {
protocolPort = &corev1.ServicePort{
Name: portName(nameWithProtocol, protocol.defaultPort),
Name: naming.PortName(nameWithProtocol, protocol.defaultPort),
Port: protocol.defaultPort,
}
}
Expand Down
8 changes: 4 additions & 4 deletions internal/manifests/collector/parser/receiver_jaeger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ func TestJaegerExposeDefaultPorts(t *testing.T) {
portNumber int32
seen bool
}{
"jaeger-grpc": {portNumber: 14250, transportProtocol: corev1.ProtocolTCP},
"jaeger-thrift-http": {portNumber: 14268, transportProtocol: corev1.ProtocolTCP},
"jaeger-thrift-compact": {portNumber: 6831, transportProtocol: corev1.ProtocolUDP},
"jaeger-thrift-binary": {portNumber: 6832, transportProtocol: corev1.ProtocolUDP},
"jaeger-grpc": {portNumber: 14250, transportProtocol: corev1.ProtocolTCP},
"port-14268": {portNumber: 14268, transportProtocol: corev1.ProtocolTCP},
"port-6831": {portNumber: 6831, transportProtocol: corev1.ProtocolUDP},
"port-6832": {portNumber: 6832, transportProtocol: corev1.ProtocolUDP},
}

// test
Expand Down
6 changes: 4 additions & 2 deletions internal/manifests/collector/parser/receiver_otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ 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/naming"
)

var _ ReceiverParser = &OTLPReceiverParser{}
Expand Down Expand Up @@ -71,7 +73,7 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
name: grpc,
defaultPorts: []corev1.ServicePort{
{
Name: portName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
Name: naming.PortName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
Port: defaultOTLPGRPCPort,
TargetPort: intstr.FromInt(int(defaultOTLPGRPCPort)),
AppProtocol: &grpc,
Expand All @@ -82,7 +84,7 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
name: http,
defaultPorts: []corev1.ServicePort{
{
Name: portName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
Name: naming.PortName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
Port: defaultOTLPHTTPPort,
TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)),
AppProtocol: &http,
Expand Down
6 changes: 4 additions & 2 deletions internal/manifests/collector/parser/receiver_skywalking.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ 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/naming"
)

var _ ReceiverParser = &SkywalkingReceiverParser{}
Expand Down Expand Up @@ -66,7 +68,7 @@ func (o *SkywalkingReceiverParser) Ports() ([]corev1.ServicePort, error) {
name: grpc,
defaultPorts: []corev1.ServicePort{
{
Name: portName(fmt.Sprintf("%s-grpc", o.name), defaultSkywalkingGRPCPort),
Name: naming.PortName(fmt.Sprintf("%s-grpc", o.name), defaultSkywalkingGRPCPort),
Port: defaultSkywalkingGRPCPort,
TargetPort: intstr.FromInt(int(defaultSkywalkingGRPCPort)),
AppProtocol: &grpc,
Expand All @@ -77,7 +79,7 @@ func (o *SkywalkingReceiverParser) Ports() ([]corev1.ServicePort, error) {
name: http,
defaultPorts: []corev1.ServicePort{
{
Name: portName(fmt.Sprintf("%s-http", o.name), defaultSkywalkingHTTPPort),
Name: naming.PortName(fmt.Sprintf("%s-http", o.name), defaultSkywalkingHTTPPort),
Port: defaultSkywalkingHTTPPort,
TargetPort: intstr.FromInt(int(defaultSkywalkingHTTPPort)),
AppProtocol: &http,
Expand Down
Loading

0 comments on commit 3d16dc1

Please sign in to comment.