Skip to content

Commit 12fbe1b

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1081 from perdasilva/e2e-stability-417
OCPBUGS-57092: [release-4.17] e2e stability fixes
2 parents 062eb82 + ca3d423 commit 12fbe1b

File tree

6 files changed

+51
-68
lines changed

6 files changed

+51
-68
lines changed

‎staging/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -662,18 +662,18 @@ func (c *ConfigMapUnpacker) ensureConfigmap(csRef *corev1.ObjectReference, name
662662
return
663663
}
664664

665-
func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (job *batchv1.Job, err error) {
665+
func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (*batchv1.Job, error) {
666666
fresh := c.job(cmRef, bundlePath, secrets, timeout)
667667
var jobs, toDelete []*batchv1.Job
668-
jobs, err = c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
668+
jobs, err := c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
669669
if err != nil {
670-
return
670+
return nil, err
671671
}
672672

673673
// This is to ensure that we account for any existing unpack jobs that may be missing the label
674674
jobWithoutLabel, err := c.jobLister.Jobs(fresh.GetNamespace()).Get(cmRef.Name)
675675
if err != nil && !apierrors.IsNotFound(err) {
676-
return
676+
return nil, err
677677
}
678678
if jobWithoutLabel != nil {
679679
_, labelExists := jobWithoutLabel.Labels[bundleUnpackRefLabel]
@@ -683,12 +683,11 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
683683
}
684684

685685
if len(jobs) == 0 {
686-
job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
687-
return
686+
return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
688687
}
689688

690-
maxRetainedJobs := 5 // TODO: make this configurable
691-
job, toDelete = sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
689+
maxRetainedJobs := 5 // TODO: make this configurable
690+
job, toDelete := sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
692691

693692
// only check for retries if an unpackRetryInterval is specified
694693
if unpackRetryInterval > 0 {
@@ -697,26 +696,23 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
697696
if cond, failed := getCondition(job, batchv1.JobFailed); failed {
698697
if time.Now().After(cond.LastTransitionTime.Time.Add(unpackRetryInterval)) {
699698
fresh.SetName(names.SimpleNameGenerator.GenerateName(fresh.GetName()))
700-
job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
699+
return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
701700
}
702701
}
703702

704703
// cleanup old failed jobs, but don't clean up successful jobs to avoid repeat unpacking
705704
for _, j := range toDelete {
706705
_ = c.client.BatchV1().Jobs(j.GetNamespace()).Delete(context.TODO(), j.GetName(), metav1.DeleteOptions{})
707706
}
708-
return
709707
}
710708
}
711709

712710
if equality.Semantic.DeepDerivative(fresh.GetOwnerReferences(), job.GetOwnerReferences()) && equality.Semantic.DeepDerivative(fresh.Spec, job.Spec) {
713-
return
711+
return job, nil
714712
}
715713

716714
// TODO: Decide when to fail-out instead of deleting the job
717-
err = c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
718-
job = nil
719-
return
715+
return nil, c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
720716
}
721717

722718
func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rbacv1.Role, err error) {

‎staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go‎

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,29 +1467,19 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
14671467
})
14681468
})
14691469
})
1470-
When("The namespace is labled as Pod Security Admission policy enforce:restricted", func() {
1470+
When("The namespace is labeled as Pod Security Admission policy enforce:restricted", func() {
14711471
BeforeEach(func() {
1472-
var err error
1473-
testNS := &corev1.Namespace{}
14741472
Eventually(func() error {
1475-
testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
1473+
testNS, err := c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
14761474
if err != nil {
14771475
return err
14781476
}
1479-
return nil
1480-
}).Should(BeNil())
1481-
1482-
testNS.ObjectMeta.Labels = map[string]string{
1483-
"pod-security.kubernetes.io/enforce": "restricted",
1484-
"pod-security.kubernetes.io/enforce-version": "latest",
1485-
}
1486-
1487-
Eventually(func() error {
1488-
_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
1489-
if err != nil {
1490-
return err
1477+
testNS.ObjectMeta.Labels = map[string]string{
1478+
"pod-security.kubernetes.io/enforce": "restricted",
1479+
"pod-security.kubernetes.io/enforce-version": "latest",
14911480
}
1492-
return nil
1481+
_, err = c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
1482+
return err
14931483
}).Should(BeNil())
14941484
})
14951485
When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created with spec.GrpcPodConfig.SecurityContextConfig set to restricted", func() {
@@ -1540,27 +1530,17 @@ var _ = Describe("Starting CatalogSource e2e tests", func() {
15401530
})
15411531
When("The namespace is labled as Pod Security Admission policy enforce:baseline", func() {
15421532
BeforeEach(func() {
1543-
var err error
1544-
testNS := &corev1.Namespace{}
15451533
Eventually(func() error {
1546-
testNS, err = c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
1534+
testNS, err := c.KubernetesInterface().CoreV1().Namespaces().Get(context.TODO(), generatedNamespace.GetName(), metav1.GetOptions{})
15471535
if err != nil {
15481536
return err
15491537
}
1550-
return nil
1551-
}).Should(BeNil())
1552-
1553-
testNS.ObjectMeta.Labels = map[string]string{
1554-
"pod-security.kubernetes.io/enforce": "baseline",
1555-
"pod-security.kubernetes.io/enforce-version": "latest",
1556-
}
1557-
1558-
Eventually(func() error {
1559-
_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
1560-
if err != nil {
1561-
return err
1538+
testNS.ObjectMeta.Labels = map[string]string{
1539+
"pod-security.kubernetes.io/enforce": "baseline",
1540+
"pod-security.kubernetes.io/enforce-version": "latest",
15621541
}
1563-
return nil
1542+
_, err = c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), testNS, metav1.UpdateOptions{})
1543+
return err
15641544
}).Should(BeNil())
15651545
})
15661546
When("A CatalogSource built with opm v1.21.0 (<v1.23.2)is created with spec.GrpcPodConfig.SecurityContextConfig set to legacy", func() {

‎staging/operator-lifecycle-manager/test/e2e/downstream_csv_namespace_labeler_plugin_test.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package e2e
22

33
import (
44
"context"
5-
65
"github.com/blang/semver/v4"
76
. "github.com/onsi/ginkgo/v2"
87
. "github.com/onsi/gomega"
@@ -88,11 +87,18 @@ var _ = Describe("CSV Namespace Labeler Plugin", func() {
8887
}).Should(HaveKeyWithValue(plugins.NamespaceLabelSyncerLabelKey, "true"))
8988

9089
// delete label
91-
ns := &v1.Namespace{}
92-
Expect(determinedE2eClient.Get(context.Background(), k8scontrollerclient.ObjectKeyFromObject(&testNamespace), ns)).To(Succeed())
93-
nsCopy := ns.DeepCopy()
94-
delete(nsCopy.Annotations, plugins.NamespaceLabelSyncerLabelKey)
95-
Expect(determinedE2eClient.Update(context.Background(), nsCopy)).To(Succeed())
90+
// NOTE: not using the determined client here because it shouldn't be used for update operations due to
91+
// race conditions (the updated resource could change b/w 'get' and 'update' operations
92+
c := ctx.Ctx().E2EClient()
93+
Eventually(func() error {
94+
ns := &v1.Namespace{}
95+
if err := c.Get(context.Background(), k8scontrollerclient.ObjectKeyFromObject(&testNamespace), ns); err != nil {
96+
return err
97+
}
98+
nsCopy := ns.DeepCopy()
99+
delete(nsCopy.Annotations, plugins.NamespaceLabelSyncerLabelKey)
100+
return c.Update(context.Background(), nsCopy)
101+
}).Should(BeNil())
96102

97103
// namespace should be labeled
98104
Eventually(func() (map[string]string, error) {

‎staging/operator-lifecycle-manager/test/e2e/fail_forward_e2e_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ var _ = Describe("Fail Forward Upgrades", func() {
321321
Expect(subscription.Status.InstallPlanRef.Name).To(Equal(failedInstallPlanRef.Name))
322322
})
323323
})
324-
When("a CSV resource is in a failed state", func() {
324+
XWhen("a CSV resource is in a failed state (https://github.com/operator-framework/operator-lifecycle-manager/issues/3573)", func() {
325325

326326
var (
327327
catalogSourceName string

‎staging/operator-lifecycle-manager/test/e2e/util/e2e_determined_client.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ func (m *DeterminedE2EClient) Create(context context.Context, obj k8scontrollerc
2828
return nil
2929
}
3030

31+
// Update retries update operation until success or timeout
32+
//
33+
// Deprecation: do not use this method as it's not resilient to the case where the resource has changed out of band
34+
// it will conflict until it times out.
35+
// There's no priority to fix this client implementation - please use regular client instead
3136
func (m *DeterminedE2EClient) Update(context context.Context, obj k8scontrollerclient.Object, options ...k8scontrollerclient.UpdateOption) error {
3237
m.keepTrying(func() error {
3338
return m.E2EKubeClient.Update(context, obj, options...)

‎vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle/bundle_unpacker.go‎

Lines changed: 10 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)