Skip to content

Commit e42f5c2

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1079 from perdasilva/e2e-stability-419-2
OCPBUGS-61363: [4.19] e2e stability fixes
2 parents 5c83a8a + 73f0b4d commit e42f5c2

File tree

5 files changed

+50
-67
lines changed

5 files changed

+50
-67
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
@@ -681,18 +681,18 @@ func (c *ConfigMapUnpacker) ensureConfigmap(csRef *corev1.ObjectReference, name
681681
return
682682
}
683683

684-
func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (job *batchv1.Job, err error) {
684+
func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath string, secrets []corev1.LocalObjectReference, timeout time.Duration, unpackRetryInterval time.Duration) (*batchv1.Job, error) {
685685
fresh := c.job(cmRef, bundlePath, secrets, timeout)
686686
var jobs, toDelete []*batchv1.Job
687-
jobs, err = c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
687+
jobs, err := c.jobLister.Jobs(fresh.GetNamespace()).List(k8slabels.ValidatedSetSelector{bundleUnpackRefLabel: cmRef.Name})
688688
if err != nil {
689-
return
689+
return nil, err
690690
}
691691

692692
// This is to ensure that we account for any existing unpack jobs that may be missing the label
693693
jobWithoutLabel, err := c.jobLister.Jobs(fresh.GetNamespace()).Get(cmRef.Name)
694694
if err != nil && !apierrors.IsNotFound(err) {
695-
return
695+
return nil, err
696696
}
697697
if jobWithoutLabel != nil {
698698
_, labelExists := jobWithoutLabel.Labels[bundleUnpackRefLabel]
@@ -702,12 +702,11 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
702702
}
703703

704704
if len(jobs) == 0 {
705-
job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
706-
return
705+
return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
707706
}
708707

709-
maxRetainedJobs := 5 // TODO: make this configurable
710-
job, toDelete = sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
708+
maxRetainedJobs := 5 // TODO: make this configurable
709+
job, toDelete := sortUnpackJobs(jobs, maxRetainedJobs) // choose latest or on-failed job attempt
711710

712711
// only check for retries if an unpackRetryInterval is specified
713712
if unpackRetryInterval > 0 {
@@ -716,26 +715,23 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
716715
if cond, failed := getCondition(job, batchv1.JobFailed); failed {
717716
if time.Now().After(cond.LastTransitionTime.Time.Add(unpackRetryInterval)) {
718717
fresh.SetName(names.SimpleNameGenerator.GenerateName(fresh.GetName()))
719-
job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
718+
return c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
720719
}
721720
}
722721

723722
// cleanup old failed jobs, but don't clean up successful jobs to avoid repeat unpacking
724723
for _, j := range toDelete {
725724
_ = c.client.BatchV1().Jobs(j.GetNamespace()).Delete(context.TODO(), j.GetName(), metav1.DeleteOptions{})
726725
}
727-
return
728726
}
729727
}
730728

731729
if equality.Semantic.DeepDerivative(fresh.GetOwnerReferences(), job.GetOwnerReferences()) && equality.Semantic.DeepDerivative(fresh.Spec, job.Spec) {
732-
return
730+
return job, nil
733731
}
734732

735733
// TODO: Decide when to fail-out instead of deleting the job
736-
err = c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
737-
job = nil
738-
return
734+
return nil, c.client.BatchV1().Jobs(job.GetNamespace()).Delete(context.TODO(), job.GetName(), metav1.DeleteOptions{})
739735
}
740736

741737
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", Label("CatalogSource"), fun
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", Label("CatalogSource"), fun
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/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)