Skip to content
Open
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
38 changes: 34 additions & 4 deletions openshift/tests-extension/test/olmv1-catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -76,12 +77,40 @@ func verifyCatalogEndpoint(ctx SpecContext, catalog, endpoint, query string) {
strings.ReplaceAll(endpoint, "?", ""),
strings.ReplaceAll(catalog, "-", ""))

job := buildCurlJob(jobNamePrefix, "default", serviceURL)
// Create the ServiceAccount first
serviceAccount := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: jobNamePrefix,
Namespace: "default",
},
}

err = k8sClient.Create(ctx, serviceAccount)
Expect(err).NotTo(HaveOccurred(), "failed to create ServiceAccount")

// Create the Job
job := buildCurlJob(jobNamePrefix, "default", serviceURL, serviceAccount.Name)
err = k8sClient.Create(ctx, job)
Expect(err).NotTo(HaveOccurred(), "failed to create Job")
Copy link
Contributor

Choose a reason for hiding this comment

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

as it stands, you might have the service account hanging around if there is an issue creating the job. If we have something deleting the namespace, then that will likely clean up the service account. We may want to either add a comment to call this out, or if there is an error creating the job, delete the service account before raising the error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it is possible to delete the default ns.

ehearne-mac:~ ehearne$ kubectl delete ns default
Error from server (Forbidden): namespaces "default" is forbidden: this namespace may not be deleted

If we were to move the job to a separate namespace just for running the jobs, and then when they are all done, delete the ns, then we can clean up completely.

Otherwise we could delete the service account when there is an error creating.


DeferCleanup(func(ctx SpecContext) {
_ = k8sClient.Delete(ctx, job)
// Force delete job with zero grace period to ensure cleanup doesn't hang
// Use Foreground propagation to ensure Pods are deleted before the Job is removed,
// guaranteeing the ServiceAccount isn't deleted while Pods are still using it
deletePolicy := metav1.DeletePropagationForeground
gracePeriod := int64(0)
err := k8sClient.Delete(ctx, job, &client.DeleteOptions{
GracePeriodSeconds: &gracePeriod,
PropagationPolicy: &deletePolicy,
})
if err != nil && !apierrors.IsNotFound(err) {
Expect(err).NotTo(HaveOccurred(), "failed to delete Job")
}
Eventually(func(g Gomega) {
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(job), &batchv1.Job{})
g.Expect(err).To(HaveOccurred())
}).WithTimeout(helpers.DefaultTimeout).WithPolling(helpers.DefaultPolling).Should(Succeed())
_ = k8sClient.Delete(ctx, serviceAccount)
})

By("Waiting for Job to succeed")
Expand Down Expand Up @@ -203,7 +232,7 @@ var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1
})
})

func buildCurlJob(prefix, namespace, url string) *batchv1.Job {
func buildCurlJob(prefix, namespace, url, serviceAccountName string) *batchv1.Job {
backoff := int32(1)
// This means the k8s garbage collector will automatically delete the job 5 minutes
// after it has completed or failed.
Expand Down Expand Up @@ -232,7 +261,8 @@ func buildCurlJob(prefix, namespace, url string) *batchv1.Job {
BackoffLimit: &backoff,
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
ServiceAccountName: serviceAccountName,
RestartPolicy: corev1.RestartPolicyNever,
Containers: []corev1.Container{{
Name: "api-tester",
Image: "registry.redhat.io/rhel8/httpd-24:latest",
Expand Down