Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid non static labels in workload objects selector #849

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/collector/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem
},
Spec: appsv1.DaemonSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: labels,
MatchLabels: SelectorLabels(otelcol),
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Expand Down
29 changes: 25 additions & 4 deletions pkg/collector/daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func TestDaemonSetNewDefault(t *testing.T) {
// prepare
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Name: "my-instance",
Namespace: "my-namespace",
},
Spec: v1alpha1.OpenTelemetryCollectorSpec{
Tolerations: testTolerationValues,
Expand All @@ -57,8 +58,28 @@ func TestDaemonSetNewDefault(t *testing.T) {
}
assert.Equal(t, expectedAnnotations, d.Spec.Template.Annotations)

// the pod selector should match the pod spec's labels
assert.Equal(t, d.Spec.Selector.MatchLabels, d.Spec.Template.Labels)
expectedLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/name": "my-instance-collector",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/version": "latest",
}
assert.Equal(t, expectedLabels, d.Spec.Template.Labels)

expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
}
assert.Equal(t, expectedSelectorLabels, d.Spec.Selector.MatchLabels)

// the pod selector must be contained within pod spec's labels
for k, v := range d.Spec.Selector.MatchLabels {
assert.Equal(t, v, d.Spec.Template.Labels[k])
}
}

func TestDaemonsetHostNetwork(t *testing.T) {
Expand Down Expand Up @@ -95,7 +116,7 @@ func TestDaemonsetPodAnnotations(t *testing.T) {
// test
ds := DaemonSet(cfg, logger, otelcol)

//Add sha256 podAnnotation
// Add sha256 podAnnotation
testPodAnnotationValues["opentelemetry-operator-config/sha256"] = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

// verify
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele
Spec: appsv1.DeploymentSpec{
Replicas: otelcol.Spec.Replicas,
Selector: &metav1.LabelSelector{
MatchLabels: labels,
MatchLabels: SelectorLabels(otelcol),
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Expand Down
29 changes: 25 additions & 4 deletions pkg/collector/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ func TestDeploymentNewDefault(t *testing.T) {
// prepare
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Name: "my-instance",
Namespace: "my-namespace",
},
Spec: v1alpha1.OpenTelemetryCollectorSpec{
Tolerations: testTolerationValues,
Expand All @@ -65,8 +66,28 @@ func TestDeploymentNewDefault(t *testing.T) {
}
assert.Equal(t, expectedAnnotations, d.Spec.Template.Annotations)

// the pod selector should match the pod spec's labels
assert.Equal(t, d.Spec.Template.Labels, d.Spec.Selector.MatchLabels)
expectedLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/name": "my-instance-collector",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/version": "latest",
}
assert.Equal(t, expectedLabels, d.Spec.Template.Labels)

expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
}
assert.Equal(t, expectedSelectorLabels, d.Spec.Selector.MatchLabels)

// the pod selector must be contained within pod spec's labels
for k, v := range d.Spec.Selector.MatchLabels {
assert.Equal(t, v, d.Spec.Template.Labels[k])
}
}

func TestDeploymentPodAnnotations(t *testing.T) {
Expand All @@ -85,7 +106,7 @@ func TestDeploymentPodAnnotations(t *testing.T) {
// test
d := Deployment(cfg, logger, otelcol)

//Add sha256 podAnnotation
// Add sha256 podAnnotation
testPodAnnotationValues["opentelemetry-operator-config/sha256"] = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

// verify
Expand Down
20 changes: 16 additions & 4 deletions pkg/collector/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ func Labels(instance v1alpha1.OpenTelemetryCollector, filterLabels []string) map
}
}

base["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
base["app.kubernetes.io/instance"] = naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name)
base["app.kubernetes.io/part-of"] = "opentelemetry"
base["app.kubernetes.io/component"] = "opentelemetry-collector"
for k, v := range SelectorLabels(instance) {
base[k] = v
}

version := strings.Split(instance.Spec.Image, ":")
if len(version) > 1 {
base["app.kubernetes.io/version"] = version[len(version)-1]
Expand All @@ -56,3 +56,15 @@ func Labels(instance v1alpha1.OpenTelemetryCollector, filterLabels []string) map

return base
}

// SelectorLabels return the common labels to all objects that are part of a managed OpenTelemetryCollector to use as selector.
// Selector labels are immutable for Deployment, StatefulSet and DaemonSet, therefore, no labels in selector should be
// expected to be modified for the lifetime of the object.
func SelectorLabels(instance v1alpha1.OpenTelemetryCollector) map[string]string {
return map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name),
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/component": "opentelemetry-collector",
}
}
19 changes: 19 additions & 0 deletions pkg/collector/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,22 @@ func TestLabelsFilter(t *testing.T) {
assert.NotContains(t, labels, "test.bar.io")
assert.Equal(t, "bar", labels["test.foo.io"])
}

func TestSelectorLabels(t *testing.T) {
// prepare
expected := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-opentelemetry-collector",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
}
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{Name: "my-opentelemetry-collector", Namespace: "my-namespace"},
}

// test
result := SelectorLabels(otelcol)

// verify
assert.Equal(t, expected, result)
}
3 changes: 3 additions & 0 deletions pkg/collector/reconcile/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ func expectedDaemonSets(ctx context.Context, params Params, expected []appsv1.Da
updated.ObjectMeta.Labels[k] = v
}

// Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error.
updated.Spec.Selector = existing.Spec.Selector.DeepCopy()

patch := client.MergeFrom(existing)
if err := params.Client.Patch(ctx, updated, patch); err != nil {
return fmt.Errorf("failed to apply changes: %w", err)
Expand Down
3 changes: 3 additions & 0 deletions pkg/collector/reconcile/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D
updated.Spec.Replicas = &currentReplicas
}

// Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error.
updated.Spec.Selector = existing.Spec.Selector.DeepCopy()

patch := client.MergeFrom(existing)

if err := params.Client.Patch(ctx, updated, patch); err != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/collector/reconcile/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ func expectedStatefulSets(ctx context.Context, params Params, expected []appsv1.
updated.ObjectMeta.Labels[k] = v
}

// Selector is an immutable field, if set, we cannot modify it otherwise we will face reconciliation error.
updated.Spec.Selector = existing.Spec.Selector.DeepCopy()

patch := client.MergeFrom(existing)
if err := params.Client.Patch(ctx, updated, patch); err != nil {
return fmt.Errorf("failed to apply changes: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/serviceaccount.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string {
return instance.Spec.ServiceAccount
}

//ServiceAccount returns the service account for the given instance.
// ServiceAccount returns the service account for the given instance.
func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount {
labels := Labels(otelcol, []string{})
labels["app.kubernetes.io/name"] = naming.ServiceAccount(otelcol)
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTel
Spec: appsv1.StatefulSetSpec{
ServiceName: naming.Service(otelcol),
Selector: &metav1.LabelSelector{
MatchLabels: labels,
MatchLabels: SelectorLabels(otelcol),
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Expand Down
29 changes: 25 additions & 4 deletions pkg/collector/statefulset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func TestStatefulSetNewDefault(t *testing.T) {
// prepare
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Name: "my-instance",
Namespace: "my-namespace",
},
Spec: v1alpha1.OpenTelemetryCollectorSpec{
Mode: "statefulset",
Expand Down Expand Up @@ -61,8 +62,28 @@ func TestStatefulSetNewDefault(t *testing.T) {
}
assert.Equal(t, expectedAnnotations, ss.Spec.Template.Annotations)

// the pod selector should match the pod spec's labels
assert.Equal(t, ss.Spec.Selector.MatchLabels, ss.Spec.Template.Labels)
expectedLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/name": "my-instance-collector",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/version": "latest",
}
assert.Equal(t, expectedLabels, ss.Spec.Template.Labels)

expectedSelectorLabels := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-instance",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
}
assert.Equal(t, expectedSelectorLabels, ss.Spec.Selector.MatchLabels)

// the pod selector must be contained within pod spec's labels
for k, v := range ss.Spec.Selector.MatchLabels {
assert.Equal(t, v, ss.Spec.Template.Labels[k])
}

// assert correct service name
assert.Equal(t, "my-instance-collector", ss.Spec.ServiceName)
Expand Down Expand Up @@ -144,7 +165,7 @@ func TestStatefulSetPodAnnotations(t *testing.T) {
// test
ss := StatefulSet(cfg, logger, otelcol)

//Add sha256 podAnnotation
// Add sha256 podAnnotation
testPodAnnotationValues["opentelemetry-operator-config/sha256"] = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

// verify
Expand Down