data_sources: Always run the operand

[lyarwood]

What this PR does / why we need it:

The operand itself deploys DataImportCrons defined by the SSP CR and DataSources based on the provided common-templates bundle. While the latter functionality should likely reside in the common-templates operand the DataImportCrons functionaility is useful in non-OpenShift environments alongside that of the common-instancetypes operand.

This change now ensures the data_sources operand runs on both OpenShift and non-OpenShift environments.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Release note:

The `data_sources` operand is now always deployed, regardless of the underlying environment being OpenShift or OKD.

[lyarwood]

Actually I think I'm going to propose a different approach here with a new operand that isn't dependent on common-templates.

[lyarwood]

Actually I think I'm going to propose a different approach here with a new operand that isn't dependent on common-templates.

Actually I'm not going to have time for that rework for a little while and I'd like to deploy the operand for upstream demos.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[lyarwood]

/retest

[0xFelix]

Other than that DataImportCronTemplates are defined below CommonTemplates in the SSP CR I don't see any dependency between the data_sources operand and common-templates. Is there really that much to rework here?

See https://github.com/kubevirt/ssp-operator/blob/master/api/v1beta1/ssp_types.go#L48

/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0xFelix

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [0xFelix]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[lyarwood]

Other than that DataImportCronTemplates are defined below CommonTemplates in the SSP CR I don't see any dependency between the data_sources operand and common-templates. Is there really that much to rework here?

See https://github.com/kubevirt/ssp-operator/blob/master/api/v1beta1/ssp_types.go#L48

/approve

Yeah apologies, it isn't obvious from the operand itself but we pass templatesBundle.DataSources into New() :

ssp-operator/controllers/setup.go

Line 60 in d6f52be

data_sources.New(templatesBundle.DataSources),

This is created by the following template-bundle code so it's not a direct dependency between the operands but definitely between the template functionality and the data_sources operand:

ssp-operator/internal/template-bundle/bundle.go

Lines 60 to 93 in d6f52be

	func extractDataSources(templates []templatev1.Template) ([]cdiv1beta1.DataSource, error) {
	uniqueNames := map[string]struct{}{}
	var dataSources []cdiv1beta1.DataSource
	for i := range templates {
	template := &templates[i]
	usesDataSources, err := vmTemplateUsesSourceRef(template)
	if err != nil {
	return nil, err
	}
	if !usesDataSources {
	continue
	}
	name, exists := findDataSourceName(template)
	if !exists {
	continue
	}
	namespace, exists := findDataSourceNamespace(template)
	if !exists {
	continue
	}
	namespacedName := namespace + "/" + name
	if _, duplicateName := uniqueNames[namespacedName]; !duplicateName {
	dataSources = append(dataSources, createDataSource(name, namespace))
	uniqueNames[namespacedName] = struct{}{}
	}
	}
	return dataSources, nil
	}

I'd really like to understand the reasoning behind this and the creation of the DataSources outside of CDI, removing the latter if possible allowing us to break this linkage between the two.

[0xFelix]

/lgtm

Then let's enable the operand for now as is and do the clean up in a separate PR.