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

[Part 1]Add unit tests for reconcile methods #250

Merged
merged 10 commits into from
May 4, 2021
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ bin
# Release artifacts
dist
bundle.tar.gz
vendor

# editor and IDE paraphernalia
.idea
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/reconcile/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func expectedConfigMaps(ctx context.Context, params Params, expected []corev1.Co
updated.ObjectMeta.Labels[k] = v
}

patch := client.MergeFrom(&params.Instance)
patch := client.MergeFrom(existing)
jpkrohling marked this conversation as resolved.
Show resolved Hide resolved

if err := params.Client.Patch(ctx, updated, patch); err != nil {
return fmt.Errorf("failed to apply changes: %w", err)
Expand Down
95 changes: 95 additions & 0 deletions pkg/collector/reconcile/configmap_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// 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 reconcile

import (
"context"
"fmt"
"testing"

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

func TestDesiredConfigMap(t *testing.T) {
t.Run("should return expected config map", func(t *testing.T) {
expected := configMap("test-collector")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would skip the local configMap, and make explicit assertions about certain aspects of the returned config map, like checking the returned labels and the name, and possibly the config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

actual := desiredConfigMap(context.Background(), params())
assert.Equal(t, expected, actual)
})

}

func TestExpectedConfigMap(t *testing.T) {

cm := configMap("test-collector")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can call the desiredConfigMap here, as you are testing its correctness in another test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

deletecm := configMap("test")

t.Run("should create config map", func(t *testing.T) {
err := expectedConfigMaps(context.Background(), params(), []v1.ConfigMap{cm}, true)
assert.NoError(t, err)

exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)
})

t.Run("should update config map", func(t *testing.T) {

createObjectIfNotExists(t, "test-collector", &cm)

err := expectedConfigMaps(context.Background(), params(), []v1.ConfigMap{desiredConfigMap(context.Background(), params())}, true)
assert.NoError(t, err)

actual := v1.ConfigMap{}
exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)
assert.Equal(t, instanceUID, actual.OwnerReferences[0].UID)
})

t.Run("should delete config map", func(t *testing.T) {
createObjectIfNotExists(t, "test", &deletecm)
err := deleteConfigMaps(context.Background(), params(), []v1.ConfigMap{configMap("test-collector")})
assert.NoError(t, err)

exists, _ := populateObjectIfExists(t, &v1.ConfigMap{}, types.NamespacedName{Namespace: "default", Name: "test"})

assert.False(t, exists)
})
}

func configMap(name string) v1.ConfigMap {
return v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "default",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params().Instance.Namespace, params().Instance.Name),
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/name": name,
},
},
Data: map[string]string{
"collector.yaml": params().Instance.Spec.Config,
},
}
}
2 changes: 1 addition & 1 deletion pkg/collector/reconcile/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func expectedDaemonSets(ctx context.Context, params Params, expected []appsv1.Da
updated.ObjectMeta.Labels[k] = v
}

patch := client.MergeFrom(&params.Instance)
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
104 changes: 104 additions & 0 deletions pkg/collector/reconcile/daemonset_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// 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 reconcile

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
v1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/open-telemetry/opentelemetry-operator/pkg/collector"
)

func TestExpectedDaemonsets(t *testing.T) {
param := params()
expectedDs := collector.DaemonSet(param.Config, logger, param.Instance)

t.Run("should create Daemonset", func(t *testing.T) {
err := expectedDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs})
assert.NoError(t, err)

exists, err := populateObjectIfExists(t, &v1.DaemonSet{}, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)

})
t.Run("should update Daemonset", func(t *testing.T) {
ds := daemonset("test-collector")
createObjectIfNotExists(t, "test-collector", &ds)
err := expectedDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs})
assert.NoError(t, err)

actual := v1.DaemonSet{}
exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)
assert.Equal(t, instanceUID, actual.OwnerReferences[0].UID)
})

t.Run("should cleanup daemonsets", func(t *testing.T) {

ds := daemonset("dummy")
createObjectIfNotExists(t, "dummy", &ds)

err := deleteDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs})
assert.NoError(t, err)

actual := v1.DaemonSet{}
exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "dummy"})

assert.False(t, exists)

})
}

func daemonset(name string) v1.DaemonSet {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the configmap: if you have a test for the desiredDaemonSet, this one here could probably be removed in favor of the official one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below

labels := collector.Labels(params().Instance)
labels["app.kubernetes.io/name"] = name
return v1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "default",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params().Instance.Namespace, params().Instance.Name),
},
},
Spec: v1.DaemonSetSpec{
Selector: &metav1.LabelSelector{
MatchLabels: labels,
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "dummy",
Image: "busybox",
}},
},
},
},
}
}
2 changes: 1 addition & 1 deletion pkg/collector/reconcile/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D
updated.ObjectMeta.Labels[k] = v
}

patch := client.MergeFrom(&params.Instance)
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
105 changes: 105 additions & 0 deletions pkg/collector/reconcile/deployment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// 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 reconcile

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
v1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/open-telemetry/opentelemetry-operator/pkg/collector"
)

func TestExpectedDeployments(t *testing.T) {
param := params()
expectedDeploy := collector.Deployment(param.Config, logger, param.Instance)

t.Run("should create deployment", func(t *testing.T) {
err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy})
assert.NoError(t, err)

exists, err := populateObjectIfExists(t, &v1.Deployment{}, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)

})
t.Run("should update deployment", func(t *testing.T) {
deploy := deployment("test-collector")
createObjectIfNotExists(t, "test-collector", &deploy)
err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy})
assert.NoError(t, err)

actual := v1.Deployment{}
exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"})

assert.NoError(t, err)
assert.True(t, exists)
assert.Equal(t, instanceUID, actual.OwnerReferences[0].UID)

})

t.Run("should cleanup deployments", func(t *testing.T) {

deploy := deployment("dummy")
createObjectIfNotExists(t, "dummy", &deploy)

err := deleteDeployments(context.Background(), param, []v1.Deployment{expectedDeploy})
assert.NoError(t, err)

actual := v1.Deployment{}
exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "dummy"})

assert.False(t, exists)

})
}

func deployment(name string) v1.Deployment {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For delete test cases - I cant use desiredDaemonSet/desiredDeployment - As I would need to a resource with same labels as the one in Params but with different name.

i.e.

Lables should be

Labels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params().Instance.Name),
},

but name should be different than params.Instance.Name. Its not possible using desiredDaemonSet/desiredDeployment functions. So I would have to retain the daemonset and deployment helper functions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all you need for those cases is a deployment/daemonset with one or two labels (managed-by, I believe). This is what we have in the jaeger-operator:

https://github.com/jaegertracing/jaeger-operator/blob/4693d119aabb550a7a651315cbfce1833bb786fc/pkg/controller/jaeger/daemonset_test.go#L104-L114

The good thing about doing it like this is that we see exactly what has influence on the decision to remove the CR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our case - we are actually creating the object. So would need the entire spec, only namespacedName and labels wont suffice. Basically have to move everything in daemonset function to actual test. Will do the same then

labels := collector.Labels(params().Instance)
labels["app.kubernetes.io/name"] = name
return v1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: "default",
Labels: map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params().Instance.Namespace, params().Instance.Name),
},
},
Spec: v1.DeploymentSpec{
Selector: &metav1.LabelSelector{
MatchLabels: labels,
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
},
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "dummy",
Image: "busybox",
}},
},
},
},
}
}
42 changes: 42 additions & 0 deletions pkg/collector/reconcile/opentelemetry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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 reconcile

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/types"

"github.com/open-telemetry/opentelemetry-operator/api/v1alpha1"
)

func TestSelf(t *testing.T) {
t.Run("should add version to the status", func(t *testing.T) {
instance := params().Instance
createObjectIfNotExists(t, "test", &instance)
err := Self(context.Background(), params())
assert.NoError(t, err)

actual := v1alpha1.OpenTelemetryCollector{}
exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test"})
assert.NoError(t, err)
assert.True(t, exists)

assert.Equal(t, actual.Status.Version, "0.0.0")

})
}
Loading