Skip to content

Commit 5748166

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request openshift#679 from openshift-bot/synchronize-upstream
OCPBUGS-24587,OCPBUGS-28744: Synchronize From Upstream Repositories
2 parents ebfa3c1 + 249da52 commit 5748166

File tree

12 files changed

+315
-69
lines changed

12 files changed

+315
-69
lines changed

‎staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
186186
return nil, err
187187
}
188188

189-
canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient)
189+
canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient, labeller.IdentityCatalogOperator)
190190
if err != nil {
191191
return nil, err
192192
}
@@ -541,6 +541,49 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
541541
return nil, err
542542
}
543543

544+
{
545+
gvr := servicesgvk
546+
informer := serviceInformer.Informer()
547+
548+
logger := op.logger.WithFields(logrus.Fields{"gvr": gvr.String()})
549+
logger.Info("registering owner reference fixer")
550+
551+
queue := workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
552+
Name: gvr.String(),
553+
})
554+
queueInformer, err := queueinformer.NewQueueInformer(
555+
ctx,
556+
queueinformer.WithQueue(queue),
557+
queueinformer.WithLogger(op.logger),
558+
queueinformer.WithInformer(informer),
559+
queueinformer.WithSyncer(queueinformer.LegacySyncHandler(func(obj interface{}) error {
560+
service, ok := obj.(*corev1.Service)
561+
if !ok {
562+
err := fmt.Errorf("wrong type %T, expected %T: %#v", obj, new(*corev1.Service), obj)
563+
logger.WithError(err).Error("casting failed")
564+
return fmt.Errorf("casting failed: %w", err)
565+
}
566+
567+
deduped := deduplicateOwnerReferences(service.OwnerReferences)
568+
if len(deduped) != len(service.OwnerReferences) {
569+
localCopy := service.DeepCopy()
570+
localCopy.OwnerReferences = deduped
571+
if _, err := op.opClient.KubernetesInterface().CoreV1().Services(service.Namespace).Update(ctx, localCopy, metav1.UpdateOptions{}); err != nil {
572+
return err
573+
}
574+
}
575+
return nil
576+
}).ToSyncer()),
577+
)
578+
if err != nil {
579+
return nil, err
580+
}
581+
582+
if err := op.RegisterQueueInformer(queueInformer); err != nil {
583+
return nil, err
584+
}
585+
}
586+
544587
// Wire Pods for CatalogSource
545588
catsrcReq, err := labels.NewRequirement(reconciler.CatalogSourceLabelKey, selection.Exists, nil)
546589
if err != nil {
@@ -1393,16 +1436,14 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
13931436
}
13941437

13951438
// Make sure that we no longer indicate unpacking progress
1396-
subs = o.setSubsCond(subs, v1alpha1.SubscriptionCondition{
1397-
Type: v1alpha1.SubscriptionBundleUnpacking,
1398-
Status: corev1.ConditionFalse,
1399-
})
1439+
o.removeSubsCond(subs, v1alpha1.SubscriptionBundleUnpacking)
14001440

14011441
// Remove BundleUnpackFailed condition from subscriptions
14021442
o.removeSubsCond(subs, v1alpha1.SubscriptionBundleUnpackFailed)
14031443

14041444
// Remove resolutionfailed condition from subscriptions
14051445
o.removeSubsCond(subs, v1alpha1.SubscriptionResolutionFailed)
1446+
14061447
newSub := true
14071448
for _, updatedSub := range updatedSubs {
14081449
updatedSub.Status.RemoveConditions(v1alpha1.SubscriptionResolutionFailed)
@@ -1679,13 +1720,9 @@ func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, cond v1alpha1.Subs
16791720
return subList
16801721
}
16811722

1682-
// removeSubsCond will remove the condition to the subscription if it exists
1683-
// Only return the list of updated subscriptions
1684-
func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) []*v1alpha1.Subscription {
1685-
var (
1686-
lastUpdated = o.now()
1687-
)
1688-
var subList []*v1alpha1.Subscription
1723+
// removeSubsCond removes the given condition from all of the subscriptions in the input
1724+
func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) {
1725+
lastUpdated := o.now()
16891726
for _, sub := range subs {
16901727
cond := sub.Status.GetCondition(condType)
16911728
// if status is ConditionUnknown, the condition doesn't exist. Just skip
@@ -1694,9 +1731,7 @@ func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alph
16941731
}
16951732
sub.Status.LastUpdated = lastUpdated
16961733
sub.Status.RemoveConditions(condType)
1697-
subList = append(subList, sub)
16981734
}
1699-
return subList
17001735
}
17011736

17021737
func (o *Operator) updateSubscriptionStatuses(subs []*v1alpha1.Subscription) ([]*v1alpha1.Subscription, error) {
@@ -2860,3 +2895,7 @@ func (o *Operator) apiresourceFromGVK(gvk schema.GroupVersionKind) (metav1.APIRe
28602895
logger.Info("couldn't find GVK in api discovery")
28612896
return metav1.APIResource{}, olmerrors.GroupVersionKindNotFoundError{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind}
28622897
}
2898+
2899+
func deduplicateOwnerReferences(refs []metav1.OwnerReference) []metav1.OwnerReference {
2900+
return sets.New(refs...).UnsortedList()
2901+
}

‎staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator_test.go

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,13 +1261,6 @@ func TestSyncResolvingNamespace(t *testing.T) {
12611261
Status: v1alpha1.SubscriptionStatus{
12621262
CurrentCSV: "",
12631263
State: "",
1264-
Conditions: []v1alpha1.SubscriptionCondition{
1265-
{
1266-
Type: v1alpha1.SubscriptionBundleUnpacking,
1267-
Status: corev1.ConditionFalse,
1268-
},
1269-
},
1270-
LastUpdated: now,
12711264
},
12721265
},
12731266
},
@@ -1399,6 +1392,62 @@ func TestSyncResolvingNamespace(t *testing.T) {
13991392
},
14001393
wantErr: fmt.Errorf("some error"),
14011394
},
1395+
{
1396+
name: "HadErrorShouldClearError",
1397+
fields: fields{
1398+
clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)},
1399+
existingOLMObjs: []runtime.Object{
1400+
&v1alpha1.Subscription{
1401+
TypeMeta: metav1.TypeMeta{
1402+
Kind: v1alpha1.SubscriptionKind,
1403+
APIVersion: v1alpha1.SchemeGroupVersion.String(),
1404+
},
1405+
ObjectMeta: metav1.ObjectMeta{
1406+
Name: "sub",
1407+
Namespace: testNamespace,
1408+
},
1409+
Spec: &v1alpha1.SubscriptionSpec{
1410+
CatalogSource: "src",
1411+
CatalogSourceNamespace: testNamespace,
1412+
},
1413+
Status: v1alpha1.SubscriptionStatus{
1414+
InstalledCSV: "sub-csv",
1415+
State: "AtLatestKnown",
1416+
Conditions: []v1alpha1.SubscriptionCondition{
1417+
{
1418+
Type: v1alpha1.SubscriptionResolutionFailed,
1419+
Reason: "ConstraintsNotSatisfiable",
1420+
Message: "constraints not satisfiable: no operators found from catalog src in namespace testNamespace referenced by subscrition sub, subscription sub exists",
1421+
Status: corev1.ConditionTrue,
1422+
},
1423+
},
1424+
},
1425+
},
1426+
},
1427+
resolveErr: nil,
1428+
},
1429+
wantSubscriptions: []*v1alpha1.Subscription{
1430+
{
1431+
TypeMeta: metav1.TypeMeta{
1432+
Kind: v1alpha1.SubscriptionKind,
1433+
APIVersion: v1alpha1.SchemeGroupVersion.String(),
1434+
},
1435+
ObjectMeta: metav1.ObjectMeta{
1436+
Name: "sub",
1437+
Namespace: testNamespace,
1438+
},
1439+
Spec: &v1alpha1.SubscriptionSpec{
1440+
CatalogSource: "src",
1441+
CatalogSourceNamespace: testNamespace,
1442+
},
1443+
Status: v1alpha1.SubscriptionStatus{
1444+
InstalledCSV: "sub-csv",
1445+
State: "AtLatestKnown",
1446+
LastUpdated: now,
1447+
},
1448+
},
1449+
},
1450+
},
14021451
}
14031452
for _, tt := range tests {
14041453
t.Run(tt.name, func(t *testing.T) {

‎staging/operator-lifecycle-manager/pkg/controller/operators/catalog/ownerrefs_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package catalog
2+
3+
import (
4+
"testing"
5+
6+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
)
8+
9+
func TestDedupe(t *testing.T) {
10+
yes := true
11+
refs := []metav1.OwnerReference{
12+
{
13+
APIVersion: "apiversion",
14+
Kind: "kind",
15+
Name: "name",
16+
UID: "uid",
17+
Controller: &yes,
18+
BlockOwnerDeletion: &yes,
19+
},
20+
{
21+
APIVersion: "apiversion",
22+
Kind: "kind",
23+
Name: "name",
24+
UID: "uid",
25+
Controller: &yes,
26+
BlockOwnerDeletion: &yes,
27+
},
28+
{
29+
APIVersion: "apiversion",
30+
Kind: "kind",
31+
Name: "name",
32+
UID: "uid",
33+
Controller: &yes,
34+
BlockOwnerDeletion: &yes,
35+
},
36+
}
37+
deduped := deduplicateOwnerReferences(refs)
38+
t.Logf("got %d deduped from %d", len(deduped), len(refs))
39+
if len(deduped) == len(refs) {
40+
t.Errorf("didn't dedupe: %#v", deduped)
41+
}
42+
}

‎staging/operator-lifecycle-manager/pkg/controller/operators/labeller/filters.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,39 @@ var filters = map[schema.GroupVersionResource]func(metav1.Object) bool{
8080
},
8181
}
8282

83-
func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface) (bool, error) {
83+
type Identity string
84+
85+
const (
86+
IdentityCatalogOperator = "catalog-operator"
87+
IdentityOLMOperator = "olm-operator"
88+
)
89+
90+
func gvrRelevant(identity Identity) func(schema.GroupVersionResource) bool {
91+
switch identity {
92+
case IdentityCatalogOperator:
93+
return sets.New(
94+
rbacv1.SchemeGroupVersion.WithResource("roles"),
95+
rbacv1.SchemeGroupVersion.WithResource("rolebindings"),
96+
corev1.SchemeGroupVersion.WithResource("serviceaccounts"),
97+
corev1.SchemeGroupVersion.WithResource("services"),
98+
corev1.SchemeGroupVersion.WithResource("pods"),
99+
corev1.SchemeGroupVersion.WithResource("configmaps"),
100+
batchv1.SchemeGroupVersion.WithResource("jobs"),
101+
apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
102+
).Has
103+
case IdentityOLMOperator:
104+
return sets.New(
105+
appsv1.SchemeGroupVersion.WithResource("deployments"),
106+
rbacv1.SchemeGroupVersion.WithResource("clusterroles"),
107+
rbacv1.SchemeGroupVersion.WithResource("clusterrolebindings"),
108+
apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
109+
).Has
110+
default:
111+
panic("programmer error: invalid identity")
112+
}
113+
}
114+
115+
func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface, identity Identity) (bool, error) {
84116
okLock := sync.Mutex{}
85117
ok := true
86118
g, ctx := errgroup.WithContext(ctx)
@@ -125,6 +157,10 @@ func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadat
125157
})
126158

127159
for gvr, filter := range allFilters {
160+
if !gvrRelevant(identity)(gvr) {
161+
logger.WithField("gvr", gvr.String()).Info("skipping irrelevant gvr")
162+
continue
163+
}
128164
gvr, filter := gvr, filter
129165
g.Go(func() error {
130166
list, err := metadataClient.Resource(gvr).List(ctx, metav1.ListOptions{})

‎staging/operator-lifecycle-manager/pkg/controller/operators/olm/operator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
183183
return nil, err
184184
}
185185

186-
canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient)
186+
canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient, labeller.IdentityOLMOperator)
187187
if err != nil {
188188
return nil, err
189189
}

‎staging/operator-lifecycle-manager/pkg/lib/ownerutil/util.go

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,20 +167,13 @@ func AddNonBlockingOwner(object metav1.Object, owner Owner) {
167167
ownerRefs = []metav1.OwnerReference{}
168168
}
169169

170-
// Infer TypeMeta for the target
171-
if err := InferGroupVersionKind(owner); err != nil {
172-
log.Warn(err.Error())
173-
}
174-
gvk := owner.GetObjectKind().GroupVersionKind()
175-
170+
nonBlockingOwner := NonBlockingOwner(owner)
176171
for _, item := range ownerRefs {
177-
if item.Kind == gvk.Kind {
178-
if item.Name == owner.GetName() && item.UID == owner.GetUID() {
179-
return
180-
}
172+
if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
173+
return
181174
}
182175
}
183-
ownerRefs = append(ownerRefs, NonBlockingOwner(owner))
176+
ownerRefs = append(ownerRefs, nonBlockingOwner)
184177
object.SetOwnerReferences(ownerRefs)
185178
}
186179

@@ -284,14 +277,20 @@ func AddOwner(object metav1.Object, owner Owner, blockOwnerDeletion, isControlle
284277
}
285278
gvk := owner.GetObjectKind().GroupVersionKind()
286279
apiVersion, kind := gvk.ToAPIVersionAndKind()
287-
ownerRefs = append(ownerRefs, metav1.OwnerReference{
280+
ownerRef := metav1.OwnerReference{
288281
APIVersion: apiVersion,
289282
Kind: kind,
290283
Name: owner.GetName(),
291284
UID: owner.GetUID(),
292285
BlockOwnerDeletion: &blockOwnerDeletion,
293286
Controller: &isController,
294-
})
287+
}
288+
for _, ref := range ownerRefs {
289+
if ref.Kind == ownerRef.Kind && ref.Name == ownerRef.Name && ref.UID == ownerRef.UID {
290+
return
291+
}
292+
}
293+
ownerRefs = append(ownerRefs, ownerRef)
295294
object.SetOwnerReferences(ownerRefs)
296295
}
297296

‎staging/operator-lifecycle-manager/pkg/lib/testobj/runtime.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,21 @@ func WithOwner(owner, obj RuntimeMetaObject) RuntimeMetaObject {
135135
out := obj.DeepCopyObject().(RuntimeMetaObject)
136136
gvk := owner.GetObjectKind().GroupVersionKind()
137137
apiVersion, kind := gvk.ToAPIVersionAndKind()
138-
refs := append(out.GetOwnerReferences(), metav1.OwnerReference{
138+
139+
nonBlockingOwner := metav1.OwnerReference{
139140
APIVersion: apiVersion,
140141
Kind: kind,
141142
Name: owner.GetName(),
142143
UID: owner.GetUID(),
143144
BlockOwnerDeletion: &dontBlockOwnerDeletion,
144145
Controller: &notController,
145-
})
146+
}
147+
for _, item := range out.GetOwnerReferences() {
148+
if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
149+
return out
150+
}
151+
}
152+
refs := append(out.GetOwnerReferences(), nonBlockingOwner)
146153
out.SetOwnerReferences(refs)
147154

148155
return out

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2445,7 +2445,7 @@ var _ = Describe("Subscription", func() {
24452445
},
24462446
5*time.Minute,
24472447
interval,
2448-
).Should(Equal(corev1.ConditionFalse))
2448+
).Should(Equal(corev1.ConditionUnknown))
24492449

24502450
By("verifying that the subscription is not reporting unpacking errors")
24512451
Eventually(
@@ -2806,7 +2806,7 @@ properties:
28062806
if err != nil {
28072807
return err
28082808
}
2809-
if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpacking); cond.Status != corev1.ConditionFalse {
2809+
if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpacking); cond.Status != corev1.ConditionUnknown {
28102810
return fmt.Errorf("subscription condition %s has unexpected value %s, expected %s", v1alpha1.SubscriptionBundleUnpacking, cond.Status, corev1.ConditionFalse)
28112811
}
28122812
if cond := fetched.Status.GetCondition(v1alpha1.SubscriptionBundleUnpackFailed); cond.Status != corev1.ConditionUnknown {

0 commit comments

Comments
 (0)