Skip to content

Commit

Permalink
add, fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Kristina Pathak committed Sep 27, 2022
1 parent c900dfd commit ed81ddc
Show file tree
Hide file tree
Showing 3 changed files with 259 additions and 46 deletions.
8 changes: 5 additions & 3 deletions apis/v1alpha1/opentelemetrycollector_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {

// validate autoscale with horizontal pod autoscaler
if r.Spec.MaxReplicas != nil {
if *r.Spec.MaxReplicas < int32(1) {
if *r.Spec.MaxReplicas <= int32(1) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one")
}

Expand All @@ -150,11 +150,13 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
}

if r.Spec.Autoscaler != nil && r.Spec.Autoscaler.Behavior != nil {
if r.Spec.Autoscaler.Behavior.ScaleDown != nil && *r.Spec.Autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(1) {
if r.Spec.Autoscaler.Behavior.ScaleDown != nil && r.Spec.Autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds != nil &&
*r.Spec.Autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(1) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleDown should be one or more")
}

if r.Spec.Autoscaler.Behavior.ScaleUp != nil && *r.Spec.Autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(1) {
if r.Spec.Autoscaler.Behavior.ScaleUp != nil && r.Spec.Autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds != nil &&
*r.Spec.Autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(1) {
return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleUp should be one or more")
}
}
Expand Down
254 changes: 254 additions & 0 deletions apis/v1alpha1/opentelemetrycollector_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"testing"

"github.com/stretchr/testify/assert"
autoscalingv2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -76,3 +78,255 @@ func TestOTELColDefaultingWebhook(t *testing.T) {
})
}
}

func TestOTELColValidatingWebhook(t *testing.T) {
zero := int32(0)
one := int32(1)
three := int32(3)
five := int32(5)

tests := []struct {
name string
otelcol OpenTelemetryCollector
expectedErr string
}{
{
name: "valid empty spec",
otelcol: OpenTelemetryCollector{},
},
{
name: "valid full spec",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeStatefulSet,
MinReplicas: &one,
Replicas: &three,
MaxReplicas: &five,
UpgradeStrategy: "adhoc",
TargetAllocator: OpenTelemetryTargetAllocator{
Enabled: true,
},
Config: `receivers:
examplereceiver:
endpoint: "0.0.0.0:12345"
examplereceiver/settings:
endpoint: "0.0.0.0:12346"
prometheus:
config:
scrape_config:
job_name: otel-collector
scrape_interval: 10s
jaeger/custom:
protocols:
thrift_http:
endpoint: 0.0.0.0:15268
`,
Ports: []v1.ServicePort{
{
Name: "port1",
Port: 5555,
},
{
Name: "port2",
Port: 5554,
Protocol: v1.ProtocolUDP,
},
},
Autoscaler: &AutoscalerSpec{
Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
ScaleDown: &autoscalingv2.HPAScalingRules{
StabilizationWindowSeconds: &three,
},
ScaleUp: &autoscalingv2.HPAScalingRules{
StabilizationWindowSeconds: &five,
},
},
TargetCPUUtilization: &five,
},
},
},
},
{
name: "invalid mode with volume claim templates",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeSidecar,
VolumeClaimTemplates: []v1.PersistentVolumeClaim{{}, {}},
},
},
expectedErr: "does not support the attribute 'volumeClaimTemplates'",
},
{
name: "invalid mode with tolerations",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeSidecar,
Tolerations: []v1.Toleration{{}, {}},
},
},
expectedErr: "does not support the attribute 'tolerations'",
},
{
name: "invalid mode with target allocator",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeDeployment,
TargetAllocator: OpenTelemetryTargetAllocator{
Enabled: true,
},
},
},
expectedErr: "does not support the target allocation deployment",
},
{
name: "invalid target allocator config",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Mode: ModeStatefulSet,
TargetAllocator: OpenTelemetryTargetAllocator{
Enabled: true,
},
},
},
expectedErr: "the OpenTelemetry Spec Prometheus configuration is incorrect",
},
{
name: "invalid port name",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Ports: []v1.ServicePort{
{
// this port name contains a non alphanumeric character, which is invalid.
Name: "-test🦄port",
Port: 12345,
Protocol: v1.ProtocolTCP,
},
},
},
},
expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
},
{
name: "invalid port name, too long",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Ports: []v1.ServicePort{
{
Name: "aaaabbbbccccdddd", // len: 16, too long
Port: 5555,
},
},
},
},
expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
},
{
name: "invalid port num",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
Ports: []v1.ServicePort{
{
Name: "aaaabbbbccccddd", // len: 15
// no port set means it's 0, which is invalid
},
},
},
},
expectedErr: "the OpenTelemetry Spec Ports configuration is incorrect",
},
{
name: "invalid max replicas",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &one,
},
},
expectedErr: "maxReplicas should be defined and more than one",
},
{
name: "invalid replicas, greater than max",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
Replicas: &five,
},
},
expectedErr: "replicas must not be greater than maxReplicas",
},
{
name: "invalid min replicas, greater than max",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
MinReplicas: &five,
},
},
expectedErr: "minReplicas must not be greater than maxReplicas",
},
{
name: "invalid min replicas, lesser than 1",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
MinReplicas: &zero,
},
},
expectedErr: "minReplicas should be one or more",
},
{
name: "invalid autoscaler scale down",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
Autoscaler: &AutoscalerSpec{
Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
ScaleDown: &autoscalingv2.HPAScalingRules{
StabilizationWindowSeconds: &zero,
},
},
},
},
},
expectedErr: "scaleDown should be one or more",
},
{
name: "invalid autoscaler scale up",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
Autoscaler: &AutoscalerSpec{
Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{
ScaleUp: &autoscalingv2.HPAScalingRules{
StabilizationWindowSeconds: &zero,
},
},
},
},
},
expectedErr: "scaleUp should be one or more",
},
{
name: "invalid autoscaler target cpu utilization",
otelcol: OpenTelemetryCollector{
Spec: OpenTelemetryCollectorSpec{
MaxReplicas: &three,
Autoscaler: &AutoscalerSpec{
TargetCPUUtilization: &zero,
},
},
},
expectedErr: "targetCPUUtilization should be greater than 0 and less than 100",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := test.otelcol.validateCRDSpec()
if test.expectedErr == "" {
assert.NoError(t, err)
return
}
assert.ErrorContains(t, err, test.expectedErr)
})
}
}
43 changes: 0 additions & 43 deletions pkg/collector/container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,49 +152,6 @@ service:
},
},
},
{
description: "invalid port name",
specConfig: goodConfig,
specPorts: []corev1.ServicePort{
{
// this port name contains a non alphanumeric character, which is invalid.
Name: "-test🦄port",
Port: 12345,
Protocol: corev1.ProtocolTCP,
},
},
expectedPorts: []corev1.ContainerPort{
{
Name: "examplereceiver",
ContainerPort: 12345,
},
metricContainerPort,
},
},
{
description: "long port name",
specConfig: goodConfig,
specPorts: []corev1.ServicePort{
{
// this port name is longer than 15 characters, which is invalid.
Name: "testportaaaabbbb",
Port: 5,
Protocol: corev1.ProtocolTCP,
},
},
expectedPorts: []corev1.ContainerPort{
{
Name: "examplereceiver",
ContainerPort: 12345,
},
metricContainerPort,
{
Name: "testportaaaabbb",
ContainerPort: 5,
Protocol: corev1.ProtocolTCP,
},
},
},
{
description: "duplicate port name",
specConfig: goodConfig,
Expand Down

0 comments on commit ed81ddc

Please sign in to comment.