test(integration): Added custom pod label tests for global configuration attributes

[chan-tim-sumo]

Overview

• github issue: Add tests for global configuration attributes #2699
• jira: https://sumologic.atlassian.net/browse/OSC-766
• added pod label tests for global configs

3 more to go:

 sumologic.pullSecrets
 sumologic.podAnnotations
 sumologic.serviceAccount.annotations

Checklist

• Changelog updated or skip changelog label added
• Documentation updated
• Template tests added for new features
• Integration tests added or modified for major features

[sumo-drosiek]

Is that actually works? I'm trying to understand the flow and for me it looks like you are trying to check if rendered pod definition (which for example looks like this: https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/main/tests/helm/testdata/goldenfile/events_otc_statefulset/basic.output.yaml) contains sumologic.podLabels key.

The code should look like more like

sumologic-kubernetes-collection/tests/helm/common_test.go

Lines 129 to 180 in e52a34c

	func TestNodeSelector(t *testing.T) {
	t.Parallel()
	valuesFilePath := path.Join(testDataDirectory, "node-selector.yaml")
	renderedYamlString := RenderTemplate(
	t,
	&helm.Options{
	ValuesFiles: []string{valuesFilePath},
	SetStrValues: map[string]string{
	"sumologic.accessId": "accessId",
	"sumologic.accessKey": "accessKey",
	},
	Logger: logger.Discard, // the log output is noisy and doesn't help much
	},
	chartDirectory,
	releaseName,
	[]string{},
	true,
	"--namespace",
	defaultNamespace,
	)
	// split the rendered Yaml into individual documents and unmarshal them into K8s objects
	// we could use the yaml decoder directly, but we'd have to implement our own unmarshaling logic then
	renderedObjects := UnmarshalMultipleFromYaml[unstructured.Unstructured](t, renderedYamlString)
	for _, renderedObject := range renderedObjects {
	nodeSelector, err := GetNodeSelector(renderedObject)
	require.NoError(t, err)
	// If we ever set some default nodeSelector anywhere, this might stop working,
	// as this assumes that the only nodeSelector will be the one in node-selector.yaml
	if nodeSelector != nil {
	nsv, ok := nodeSelector[nodeSelectorKey]
	assert.True(
	t,
	ok,
	"%s should have %s nodeSelector",
	renderedObject.GetName(),
	nodeSelectorKey,
	)
	assert.True(
	t,
	nsv == nodeSelectorValue,
	"%s should have nodeSelector %s set to %s, found %s instead",
	renderedObject.GetName(),
	nodeSelectorKey,
	nodeSelectorValue,
	nsv,
	)
	}
	}
	}

Because aim of the task is to check if all pods/serviceaccounts/etc have the labels set correctly

The jira is not accessible outside of the organization so it would be much more Open Source to link correlated github issue instead

I would also split this PR per individual change (podLabels, podAnnotations, etc)

[chan-tim-sumo]

Is that actually works? I'm trying to understand the flow and for me it looks like you are trying to check if rendered pod definition (which for example looks like this: https://github.com/SumoLogic/sumologic-kubernetes-collection/blob/main/tests/helm/testdata/goldenfile/events_otc_statefulset/basic.output.yaml) contains sumologic.podLabels key.

The code should look like more like

sumologic-kubernetes-collection/tests/helm/common_test.go

Lines 129 to 180 in e52a34c

	func TestNodeSelector(t *testing.T) {
	t.Parallel()
	valuesFilePath := path.Join(testDataDirectory, "node-selector.yaml")
	renderedYamlString := RenderTemplate(
	t,
	&helm.Options{
	ValuesFiles: []string{valuesFilePath},
	SetStrValues: map[string]string{
	"sumologic.accessId": "accessId",
	"sumologic.accessKey": "accessKey",
	},
	Logger: logger.Discard, // the log output is noisy and doesn't help much
	},
	chartDirectory,
	releaseName,
	[]string{},
	true,
	"--namespace",
	defaultNamespace,
	)
	// split the rendered Yaml into individual documents and unmarshal them into K8s objects
	// we could use the yaml decoder directly, but we'd have to implement our own unmarshaling logic then
	renderedObjects := UnmarshalMultipleFromYaml[unstructured.Unstructured](t, renderedYamlString)
	for _, renderedObject := range renderedObjects {
	nodeSelector, err := GetNodeSelector(renderedObject)
	require.NoError(t, err)
	// If we ever set some default nodeSelector anywhere, this might stop working,
	// as this assumes that the only nodeSelector will be the one in node-selector.yaml
	if nodeSelector != nil {
	nsv, ok := nodeSelector[nodeSelectorKey]
	assert.True(
	t,
	ok,
	"%s should have %s nodeSelector",
	renderedObject.GetName(),
	nodeSelectorKey,
	)
	assert.True(
	t,
	nsv == nodeSelectorValue,
	"%s should have nodeSelector %s set to %s, found %s instead",
	renderedObject.GetName(),
	nodeSelectorKey,
	nodeSelectorValue,
	nsv,
	)
	}
	}
	}

Because aim of the task is to check if all pods/serviceaccounts/etc have the labels set correctly

The jira is not accessible outside of the organization so it would be much more Open Source to link correlated github issue instead

I would also split this PR per individual change (podLabels, podAnnotations, etc)

i see, i thought we needed to test to make sure labels are in the rendered objects, if it is to check if all pods/everything have the labels set correctly, i guess just need to check if the labels arent empty ?

ref: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/

if so, i'm confused, isn't there a test for labels on here?

sumologic-kubernetes-collection/tests/helm/common_test.go

Lines 48 to 83 in e52a34c

	func TestBuiltinLabels(t *testing.T) {
	valuesFilePath := path.Join(testDataDirectory, "everything-enabled.yaml")
	chartVersion, err := GetChartVersion()
	require.NoError(t, err)
	renderedYamlString := RenderTemplate(
	t,
	&helm.Options{
	ValuesFiles: []string{valuesFilePath},
	SetStrValues: map[string]string{
	"sumologic.accessId": "accessId",
	"sumologic.accessKey": "accessKey",
	},
	Logger: logger.Discard, // the log output is noisy and doesn't help much
	},
	chartDirectory,
	releaseName,
	[]string{},
	true,
	"--namespace",
	defaultNamespace,
	)
	// split the rendered Yaml into individual documents and unmarshal them into K8s objects
	// we could use the yaml decoder directly, but we'd have to implement our own unmarshaling logic then
	renderedObjects := UnmarshalMultipleFromYaml[unstructured.Unstructured](t, renderedYamlString)
	for _, renderedObject := range renderedObjects {
	if !isSubchartObject(&renderedObject) && renderedObject.GetKind() != "List" {
	object := renderedObject
	objectName := fmt.Sprintf("%s/%s", object.GetKind(), object.GetName())
	t.Run(objectName, func(t *testing.T) {
	checkBuiltinLabels(t, &object, chartVersion)
	})
	}
	}
	}

[sumo-drosiek]

if so, i'm confused, isn't there a test for labels on here?

It tests for Helm built-in labels:

sumologic-kubernetes-collection/tests/helm/common_test.go

Lines 332 to 341 in e52a34c

	// check the built-in labels added to all K8s objects created by the chart
	func checkBuiltinLabels(t *testing.T, object metav1.Object, chartVersion string) {
	labels := object.GetLabels()
	require.Contains(t, labels, "chart")
	require.Contains(t, labels, "heritage")
	require.Contains(t, labels, "release")
	assert.Equal(t, fmt.Sprintf("%s-%s", chartName, chartVersion), labels["chart"])
	assert.Equal(t, releaseName, labels["release"])
	assert.Equal(t, "Helm", labels["heritage"])
	}

if it is to check if all pods/everything have the labels set correctly, i guess just need to check if the labels arent empty?

Yes, but we want to check for exact value. We need to ensure that we are able to set additional pod labels, pod annotations and so on for every pod. Maybe we need to use multiple configuration keys to achieve it, but this is want we want to test and ensure. We want to test the following feature: https://help.sumologic.com/docs/send-data/kubernetes/best-practices/#pod-labels

[chan-tim-sumo]

if so, i'm confused, isn't there a test for labels on here?

It tests for Helm built-in labels:

sumologic-kubernetes-collection/tests/helm/common_test.go

Lines 332 to 341 in e52a34c

	// check the built-in labels added to all K8s objects created by the chart
	func checkBuiltinLabels(t *testing.T, object metav1.Object, chartVersion string) {
	labels := object.GetLabels()
	require.Contains(t, labels, "chart")
	require.Contains(t, labels, "heritage")
	require.Contains(t, labels, "release")
	assert.Equal(t, fmt.Sprintf("%s-%s", chartName, chartVersion), labels["chart"])
	assert.Equal(t, releaseName, labels["release"])
	assert.Equal(t, "Helm", labels["heritage"])
	}

if it is to check if all pods/everything have the labels set correctly, i guess just need to check if the labels arent empty?

Yes, but we want to check for exact value. We need to ensure that we are able to set additional pod labels, pod annotations and so on for every pod. Maybe we need to use multiple configuration keys to achieve it, but this is want we want to test and ensure. We want to test the following feature: https://help.sumologic.com/docs/send-data/kubernetes/best-practices/#pod-labels

just to make sure i'm understanding this, if we want to check for EXACT values for ALL/additional pod labels, do you mean testing a custom pod label? since currently there is a test for BUILT-IN label only? 🤔

[sumo-drosiek]

just to make sure i'm understanding this, if we want to check for EXACT values for ALL/additional pod labels, do you mean testing a custom pod label? since currently there is a test for BUILT-IN label only?

Yes, we want to check exact values for additional pod labels

[sumo-drosiek]

We should enable optional components as well. see https://github.com/SumoLogic/sumologic-openshift-images/blob/main/scripts/values.yaml

[sumo-drosiek]

It can be simplified to something like this:

Suggested change

	func GetPodTemplateSpec(object unstructured.Unstructured) (*corev1.PodTemplateSpec, error) {
	switch object.GetKind() {
	case "Deployment":
	deployment := &appsv1.Deployment{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, deployment)
	if err != nil {
	return nil, err
	}
	return &deployment.Spec.Template, nil
	case "StatefulSet":
	statefulset := &appsv1.StatefulSet{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, statefulset)
	if err != nil {
	return nil, err
	}
	return &statefulset.Spec.Template, nil
	case "DaemonSet":
	daemonset := &appsv1.DaemonSet{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, daemonset)
	if err != nil {
	return nil, err
	}
	return &daemonset.Spec.Template, nil
	case "Job":
	job := &batchv1.Job{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, job)
	if err != nil {
	return nil, err
	}
	return &job.Spec.Template, nil
	case "CronJob":
	cronJob := &batchv1.CronJob{}
	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, cronJob)
	if err != nil {
	return nil, err
	}
	return &cronJob.Spec.JobTemplate.Spec.Template, nil
	default:
	return nil, nil
	}
	}
	func GetPodTemplateSpec(object unstructured.Unstructured) (*corev1.PodTemplateSpec, error) {
	spec, err := GetPodSpec(object)
	if err != nil {
	return nil, err
	}
	return spec.Template, nil
	}

[chan-tim-sumo]

since most of the comments are here:

another question i have is, I see pullSecrets and imagePullSecrets, are they the same?

pod annotation and service account annotation is pretty intuitive.

[sumo-drosiek]

I see pullSecrets and imagePullSecrets, are they the same?

It seems like that. We cannot change the subcharts configuration keys :(

[sumo-drosiek]

LGTM. We should add missing configuration options (if any) to https://help.sumologic.com/docs/send-data/kubernetes/best-practices/#pod-labels