Skip to content

Commit 68b4974

Browse files
harishsurfharishsurf
committed
Fix delay when OLM applies label to Namespaces
Bug 1822040: When reconciling OperatorGroup, all the namespaces that appear in targetNamespace or have a matching label selector, there was a delay in OLM applying labels to these namespaces. This was because the og.spec.TargetNamespaces took a long time to update its list of namespaces. Solution: The fix was to use the pre-calculated targetNamespaces instead to compare with og.Status.Namespaces to identify the change in OperatorGroup's namespace list which ensured there was no delay in applying the label. Signed-off-by: Harish <[email protected]>
1 parent 8ad4341 commit 68b4974

File tree

2 files changed

+155
-13
lines changed

2 files changed

+155
-13
lines changed

‎pkg/controller/operators/olm/operatorgroup.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
100100

101101
if namespacesChanged(targetNamespaces, op.Status.Namespaces) {
102102
logger.Debug("OperatorGroup namespaces change detected")
103-
outOfSyncNamespaces := namespacesAddedOrRemoved(op.Spec.TargetNamespaces, op.Status.Namespaces)
103+
outOfSyncNamespaces := namespacesAddedOrRemoved(op.Status.Namespaces, targetNamespaces)
104104

105105
// Update operatorgroup target namespace selection
106106
logger.WithField("targets", targetNamespaces).Debug("namespace change detected")

‎test/e2e/operator_groups_e2e_test.go

Lines changed: 154 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"time"
88

99
"github.com/blang/semver"
10+
. "github.com/onsi/ginkgo"
11+
. "github.com/onsi/gomega"
1012
"github.com/stretchr/testify/require"
1113
authorizationv1 "k8s.io/api/authorization/v1"
1214
corev1 "k8s.io/api/core/v1"
@@ -21,7 +23,6 @@ import (
2123
"k8s.io/client-go/tools/cache"
2224
"k8s.io/client-go/util/retry"
2325

24-
. "github.com/onsi/ginkgo"
2526
v1 "github.com/operator-framework/api/pkg/operators/v1"
2627
"github.com/operator-framework/api/pkg/operators/v1alpha1"
2728
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
@@ -2107,47 +2108,47 @@ var _ = Describe("Operator Group", func() {
21072108
LabelSelector: labels.Set(map[string]string{ogLabel: ""}).String(),
21082109
}
21092110

2110-
namespaceList, err := pollForListCount(c, listOptions, 0)
2111+
namespaceList, err := pollForNamespaceListCount(c, listOptions, 0)
21112112
require.NoError(GinkgoT(), err)
21122113

21132114
// Update the OperatorGroup to include a single namespace
21142115
operatorGroup.Spec.TargetNamespaces = []string{testNamespaceA}
21152116
updateOGSpecFunc := updateOperatorGroupSpecFunc(GinkgoT(), crc, testNamespaceA, operatorGroup.GetName())
21162117
require.NoError(GinkgoT(), retry.RetryOnConflict(retry.DefaultBackoff, updateOGSpecFunc(operatorGroup.Spec)))
21172118

2118-
namespaceList, err = pollForListCount(c, listOptions, 1)
2119+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 1)
21192120
require.NoError(GinkgoT(), err)
21202121
require.True(GinkgoT(), checkForOperatorGroupLabels(operatorGroup, namespaceList.Items))
21212122

21222123
// Update the OperatorGroup to include two namespaces
21232124
operatorGroup.Spec.TargetNamespaces = []string{testNamespaceA, testNamespaceC}
21242125
require.NoError(GinkgoT(), retry.RetryOnConflict(retry.DefaultBackoff, updateOGSpecFunc(operatorGroup.Spec)))
21252126

2126-
namespaceList, err = pollForListCount(c, listOptions, 2)
2127+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 2)
21272128
require.NoError(GinkgoT(), err)
21282129
require.True(GinkgoT(), checkForOperatorGroupLabels(operatorGroup, namespaceList.Items))
21292130

21302131
// Update the OperatorGroup to include three namespaces
21312132
operatorGroup.Spec.TargetNamespaces = []string{testNamespaceA, testNamespaceB, testNamespaceC}
21322133
require.NoError(GinkgoT(), retry.RetryOnConflict(retry.DefaultBackoff, updateOGSpecFunc(operatorGroup.Spec)))
21332134

2134-
namespaceList, err = pollForListCount(c, listOptions, 3)
2135+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 3)
21352136
require.NoError(GinkgoT(), err)
21362137
require.True(GinkgoT(), checkForOperatorGroupLabels(operatorGroup, namespaceList.Items))
21372138

21382139
// Update the OperatorGroup to include two namespaces
21392140
operatorGroup.Spec.TargetNamespaces = []string{testNamespaceA, testNamespaceC}
21402141
require.NoError(GinkgoT(), retry.RetryOnConflict(retry.DefaultBackoff, updateOGSpecFunc(operatorGroup.Spec)))
21412142

2142-
namespaceList, err = pollForListCount(c, listOptions, 2)
2143+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 2)
21432144
require.NoError(GinkgoT(), err)
21442145
require.True(GinkgoT(), checkForOperatorGroupLabels(operatorGroup, namespaceList.Items))
21452146

21462147
// Make the OperatorGroup a Cluster OperatorGroup.
21472148
operatorGroup.Spec.TargetNamespaces = []string{}
21482149
require.NoError(GinkgoT(), retry.RetryOnConflict(retry.DefaultBackoff, updateOGSpecFunc(operatorGroup.Spec)))
21492150

2150-
namespaceList, err = pollForListCount(c, listOptions, 0)
2151+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 0)
21512152
require.NoError(GinkgoT(), err)
21522153
})
21532154
It("CleanupDeletedOperatorGroupLabels", func() {
@@ -2202,7 +2203,7 @@ var _ = Describe("Operator Group", func() {
22022203
LabelSelector: labels.Set(map[string]string{ogLabel: ""}).String(),
22032204
}
22042205

2205-
namespaceList, err := pollForListCount(c, listOptions, 3)
2206+
namespaceList, err := pollForNamespaceListCount(c, listOptions, 3)
22062207
require.NoError(GinkgoT(), err)
22072208
require.True(GinkgoT(), checkForOperatorGroupLabels(operatorGroup, namespaceList.Items))
22082209

@@ -2211,9 +2212,149 @@ var _ = Describe("Operator Group", func() {
22112212
require.NoError(GinkgoT(), err)
22122213

22132214
// Check that no namespaces have the OperatorGroup.
2214-
namespaceList, err = pollForListCount(c, listOptions, 0)
2215+
namespaceList, err = pollForNamespaceListCount(c, listOptions, 0)
22152216
require.NoError(GinkgoT(), err)
22162217
})
2218+
2219+
Context("Given a set of Namespaces", func() {
2220+
2221+
var (
2222+
c operatorclient.ClientInterface
2223+
crc versioned.Interface
2224+
testNamespaces []string
2225+
testNamespaceA string
2226+
)
2227+
2228+
BeforeEach(func() {
2229+
c = newKubeClient()
2230+
crc = newCRClient()
2231+
2232+
// Create the namespaces that will have an OperatorGroup Label applied.
2233+
testNamespaceA = genName("namespace-a-")
2234+
testNamespaceB := genName("namespace-b-")
2235+
testNamespaceC := genName("namespace-c-")
2236+
testNamespaces = []string{
2237+
testNamespaceA, testNamespaceB, testNamespaceC,
2238+
}
2239+
2240+
// Create the namespaces
2241+
for _, namespace := range testNamespaces {
2242+
_, err := c.KubernetesInterface().CoreV1().Namespaces().Create(context.TODO(), &corev1.Namespace{
2243+
ObjectMeta: metav1.ObjectMeta{
2244+
Name: namespace,
2245+
},
2246+
}, metav1.CreateOptions{})
2247+
Expect(err).ToNot(HaveOccurred())
2248+
}
2249+
})
2250+
2251+
AfterEach(func() {
2252+
// Cleanup namespaces
2253+
for _, namespace := range testNamespaces {
2254+
err := c.KubernetesInterface().CoreV1().Namespaces().Delete(context.TODO(), namespace, metav1.DeleteOptions{})
2255+
Expect(err).ToNot(HaveOccurred())
2256+
}
2257+
})
2258+
2259+
Context("Associating these Namespaces with a label", func() {
2260+
2261+
var (
2262+
matchingLabel map[string]string
2263+
)
2264+
2265+
BeforeEach(func() {
2266+
2267+
matchingLabel = map[string]string{"foo": "bar"}
2268+
2269+
// Updating Namespace with labels
2270+
for _, namespace := range testNamespaces {
2271+
_, err := c.KubernetesInterface().CoreV1().Namespaces().Update(context.TODO(), &corev1.Namespace{
2272+
ObjectMeta: metav1.ObjectMeta{
2273+
Name: namespace,
2274+
Labels: matchingLabel,
2275+
},
2276+
}, metav1.UpdateOptions{})
2277+
Expect(err).ToNot(HaveOccurred())
2278+
}
2279+
2280+
})
2281+
2282+
When("an OperatorGroup is created having matching label selector defined", func() {
2283+
var operatorGroup *v1.OperatorGroup
2284+
2285+
BeforeEach(func() {
2286+
2287+
// Creating operator group
2288+
operatorGroup = &v1.OperatorGroup{
2289+
ObjectMeta: metav1.ObjectMeta{
2290+
Name: genName("e2e-operator-group-"),
2291+
Namespace: testNamespaceA,
2292+
},
2293+
Spec: v1.OperatorGroupSpec{
2294+
Selector: &metav1.LabelSelector{
2295+
MatchLabels: matchingLabel,
2296+
},
2297+
},
2298+
}
2299+
var err error
2300+
operatorGroup, err = crc.OperatorsV1().OperatorGroups(testNamespaceA).Create(context.TODO(), operatorGroup, metav1.CreateOptions{})
2301+
Expect(err).ToNot(HaveOccurred())
2302+
})
2303+
2304+
It("OLM applies labels to Namespaces that are associated with an OperatorGroup", func() {
2305+
ogLabel, err := getOGLabelKey(operatorGroup)
2306+
Expect(err).ToNot(HaveOccurred())
2307+
2308+
// Create list options
2309+
listOptions := metav1.ListOptions{
2310+
LabelSelector: labels.Set(map[string]string{ogLabel: ""}).String(),
2311+
}
2312+
2313+
// Verify that all the namespaces listed in targetNamespaces field of OperatorGroup have labels applied on them
2314+
namespaceList, err := pollForNamespaceListCount(c, listOptions, 3)
2315+
Expect(err).ToNot(HaveOccurred())
2316+
Expect(checkForOperatorGroupLabels(operatorGroup, namespaceList.Items)).Should(BeTrue())
2317+
})
2318+
})
2319+
})
2320+
2321+
When("an OperatorGroup is created having above Namespaces defined under targetNamespaces field", func() {
2322+
var operatorGroup *v1.OperatorGroup
2323+
2324+
BeforeEach(func() {
2325+
// Create an OperatorGroup with three target namespaces.
2326+
operatorGroup = &v1.OperatorGroup{
2327+
ObjectMeta: metav1.ObjectMeta{
2328+
Name: genName("e2e-operator-group-"),
2329+
Namespace: testNamespaceA,
2330+
},
2331+
Spec: v1.OperatorGroupSpec{
2332+
TargetNamespaces: testNamespaces,
2333+
},
2334+
}
2335+
var err error
2336+
operatorGroup, err = crc.OperatorsV1().OperatorGroups(testNamespaceA).Create(context.TODO(), operatorGroup, metav1.CreateOptions{})
2337+
Expect(err).ToNot(HaveOccurred())
2338+
})
2339+
2340+
It("OLM applies labels to Namespaces that are associated with an OperatorGroup", func() {
2341+
2342+
ogLabel, err := getOGLabelKey(operatorGroup)
2343+
Expect(err).ToNot(HaveOccurred())
2344+
2345+
// Create list options
2346+
listOptions := metav1.ListOptions{
2347+
LabelSelector: labels.Set(map[string]string{ogLabel: ""}).String(),
2348+
}
2349+
2350+
// Verify that all the namespaces listed in targetNamespaces field of OperatorGroup have labels applied on them
2351+
namespaceList, err := pollForNamespaceListCount(c, listOptions, 3)
2352+
Expect(err).ToNot(HaveOccurred())
2353+
Expect(checkForOperatorGroupLabels(operatorGroup, namespaceList.Items)).Should(BeTrue())
2354+
2355+
})
2356+
})
2357+
})
22172358
})
22182359

22192360
func checkOperatorGroupAnnotations(obj metav1.Object, op *v1.OperatorGroup, checkTargetNamespaces bool, targetNamespaces string) error {
@@ -2308,8 +2449,9 @@ func updateOperatorGroupSpecFunc(t GinkgoTInterface, crc versioned.Interface, na
23082449
}
23092450
}
23102451

2311-
func pollForListCount(c operatorclient.ClientInterface, listOptions metav1.ListOptions, expectedLength int) (list *corev1.NamespaceList, err error) {
2312-
wait.PollImmediate(pollInterval, pollDuration, func() (bool, error) {
2452+
func pollForNamespaceListCount(c operatorclient.ClientInterface, listOptions metav1.ListOptions, expectedLength int) (list *corev1.NamespaceList, err error) {
2453+
2454+
Eventually(func() (bool, error) {
23132455
list, err = c.KubernetesInterface().CoreV1().Namespaces().List(context.TODO(), listOptions)
23142456
if err != nil {
23152457
return false, err
@@ -2318,7 +2460,7 @@ func pollForListCount(c operatorclient.ClientInterface, listOptions metav1.ListO
23182460
return true, nil
23192461
}
23202462
return false, nil
2321-
})
2463+
}).Should(BeTrue())
23222464
return
23232465
}
23242466

0 commit comments

Comments
 (0)