feat(resources): Add Resource Config for Main Deployment

[jaadbarg]

fixes: #332

[ebaron]

Hey John, I had a new idea about unit testing this feature. We could add a new test Context for testing the case when a user specifies resource requirements, such as:

Context("with resource requirements", func() {
	BeforeEach(func() {
		t.objs = append(t.objs, test.NewCryostatWithResources())
	})
	It("should create expected deployment", func() {
		t.expectDeployment()
	})
})

Then you'd need to add a NewCryostatWithResources function inside test/resources.go. It can call NewCryostat, but add in resource requirements for each type of container before returning.

To add expectations about the resource requirements present in the deployment, you can modify checkMainDeployment:

cryostat-operator/internal/controllers/cryostat_controller_test.go

Lines 1657 to 1714 in 3948e7d

	func (t *cryostatTestInput) checkMainDeployment() {
	deployment := &appsv1.Deployment{}
	err := t.Client.Get(context.Background(), types.NamespacedName{Name: "cryostat", Namespace: "default"}, deployment)
	Expect(err).ToNot(HaveOccurred())
	cr := &operatorv1beta1.Cryostat{}
	err = t.Client.Get(context.Background(), types.NamespacedName{Name: "cryostat", Namespace: "default"}, cr)
	Expect(err).ToNot(HaveOccurred())
	Expect(deployment.Name).To(Equal("cryostat"))
	Expect(deployment.Namespace).To(Equal("default"))
	Expect(deployment.Annotations).To(Equal(map[string]string{
	"app.openshift.io/connects-to": "cryostat-operator-controller-manager",
	}))
	Expect(deployment.Labels).To(Equal(map[string]string{
	"app": "cryostat",
	"kind": "cryostat",
	"component": "cryostat",
	"app.kubernetes.io/name": "cryostat",
	}))
	Expect(metav1.IsControlledBy(deployment, cr)).To(BeTrue())
	Expect(deployment.Spec.Selector).To(Equal(test.NewMainDeploymentSelector()))
	// compare Pod template
	template := deployment.Spec.Template
	Expect(template.Name).To(Equal("cryostat"))
	Expect(template.Namespace).To(Equal("default"))
	Expect(template.Labels).To(Equal(map[string]string{
	"app": "cryostat",
	"kind": "cryostat",
	"component": "cryostat",
	}))
	Expect(template.Spec.Volumes).To(Equal(test.NewVolumes(t.minimal, t.TLS)))
	Expect(template.Spec.SecurityContext).To(Equal(test.NewPodSecurityContext()))
	// Check that the networking environment variables are set correctly
	coreContainer := template.Spec.Containers[0]
	var reportsUrl string
	if t.reportReplicas == 0 {
	reportsUrl = ""
	} else {
	reportsUrl = "http://cryostat-reports:10000"
	}
	checkCoreContainer(&coreContainer, t.minimal, t.TLS, t.externalTLS, t.EnvCoreImageTag, t.controller.IsOpenShift, reportsUrl)
	if !t.minimal {
	// Check that Grafana is configured properly, depending on the environment
	grafanaContainer := template.Spec.Containers[1]
	checkGrafanaContainer(&grafanaContainer, t.TLS, t.EnvGrafanaImageTag)
	// Check that JFR Datasource is configured properly
	datasourceContainer := template.Spec.Containers[2]
	checkDatasourceContainer(&datasourceContainer, t.EnvDatasourceImageTag)
	}
	// Check that the proper Service Account is set
	Expect(template.Spec.ServiceAccountName).To(Equal("cryostat"))
	}

Before the call to checkCoreContainer, the Cryostat instance is available in the variable cr. You can add a new parameter to checkCoreContainer of type corev1.ResourceRequirements. Then pass the core resource requirements from cr to this function. Then you can add an expectation comparing the argument to the resource requirements found in the container definition. Repeat this for checkDatasourceContainer and checkGrafanaContainer.

[ebaron]

This should be:

Suggested change

	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:resourceRequirements"}
	// +operator-sdk:csv:customresourcedefinitions:type=spec

Having the descriptor here prevents the Core/DataSource/Grafana resources from showing in the UI.

[ebaron]

This will need another make bundle after changing.

[jaadbarg]

@ebaron for some reason when I run make bundle in cryostat-operator.clusterserviceversion.yaml,

operators.operatorframework.io/builder: operator-sdk-v1.4.0+git operators.operatorframework.io/internal-objects: '["flightrecorders.operator.cryostat.io","recordings.operator.cryostat.io"]'

gets changed to

operators.operatorframework.io/builder: operator-sdk-v1.5.0

and that's causing the merge conflict

[ebaron]

@jaadbarg the operators.operatorframework.io/builder annotation change I think is just due to a platform inconsistency. Opearator SDK 1.5.0 reports 1.4.0+git on Linux. Don't worry too much about this.

The operators.operatorframework.io/internal-objects annotation issue is probably just a matter of rebasing your branch onto the latest main branch.

git checkout main
git pull origin main
git checkout #332
git rebase main
git push -f <name of your remote fork> #332

[ebaron]

Actually, you'll need to make bundle after rebasing. So the process would be something like:

git checkout main
git pull origin main
git checkout #332
git rebase main
make bundle
git commit -a -m 'Regenerate bundle'
git push -f <name of your remote fork> #332

[ebaron]

Looks good, thanks John!