Skip to content

Commit 69bccf3

Browse files
dinhxuanvudinhxuanvu
authored
fix(og): Fix missing MultiOperatorGroups condition in some cases (#2305)
In some special cases, the MultiOperatorGroups condition is missing even though there are multiple OGs in the same namespace. The process of adding this condition happens during syncNamespace which sometimes doesn't happen if syncOperatorGroups fails prematurely due to other errors. Moving the MultiOperatorGroups condition to syncOperatorGroups to ensure it will be run everytime. Signed-off-by: Vu Dinh <[email protected]>
1 parent d555939 commit 69bccf3

File tree

3 files changed

+47
-57
lines changed

3 files changed

+47
-57
lines changed

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

-36
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
rbacv1 "k8s.io/api/rbac/v1"
1616
extinf "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
1717
k8serrors "k8s.io/apimachinery/pkg/api/errors"
18-
meta "k8s.io/apimachinery/pkg/api/meta"
1918
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2019
"k8s.io/apimachinery/pkg/labels"
2120
"k8s.io/apimachinery/pkg/runtime"
@@ -880,41 +879,6 @@ func (a *Operator) syncNamespace(obj interface{}) error {
880879
return err
881880
}
882881

883-
// Query OG in this namespace
884-
groups, err := a.lister.OperatorsV1().OperatorGroupLister().OperatorGroups(namespace.GetName()).List(labels.Everything())
885-
if err != nil {
886-
logger.WithError(err).Warn("failed to list OperatorGroups in the namespace")
887-
return err
888-
}
889-
890-
// Check if there is a stale multiple OG condition and clear it if existed.
891-
if len(groups) == 1 {
892-
og := groups[0]
893-
if c := meta.FindStatusCondition(og.Status.Conditions, v1.MutlipleOperatorGroupCondition); c != nil {
894-
meta.RemoveStatusCondition(&og.Status.Conditions, v1.MutlipleOperatorGroupCondition)
895-
_, err = a.client.OperatorsV1().OperatorGroups(namespace.GetName()).UpdateStatus(context.TODO(), og, metav1.UpdateOptions{})
896-
if err != nil {
897-
logger.Warnf("fail to upgrade operator group status og=%s with condition %+v: %s", og.GetName(), c, err.Error())
898-
}
899-
}
900-
} else if len(groups) > 1 {
901-
// Add to all OG's status conditions to indicate they're multiple OGs in the
902-
// same namespace which is not allowed.
903-
cond := metav1.Condition{
904-
Type: v1.MutlipleOperatorGroupCondition,
905-
Status: metav1.ConditionTrue,
906-
Reason: v1.MultipleOperatorGroupsReason,
907-
Message: "Multiple OperatorGroup found in the same namespace",
908-
}
909-
for _, og := range groups {
910-
meta.SetStatusCondition(&og.Status.Conditions, cond)
911-
_, err = a.client.OperatorsV1().OperatorGroups(namespace.GetName()).UpdateStatus(context.TODO(), og, metav1.UpdateOptions{})
912-
if err != nil {
913-
logger.Warnf("fail to upgrade operator group status og=%s with condition %+v: %s", og.GetName(), cond, err.Error())
914-
}
915-
}
916-
}
917-
918882
for _, group := range operatorGroupList {
919883
namespaceSet := resolver.NewNamespaceSet(group.Status.Namespaces)
920884

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

+1-20
Original file line numberDiff line numberDiff line change
@@ -4543,11 +4543,6 @@ func TestOperatorGroupConditions(t *testing.T) {
45434543
clockFake := utilclock.NewFakeClock(time.Date(2006, time.January, 2, 15, 4, 5, 0, time.FixedZone("MST", -7*3600)))
45444544

45454545
operatorNamespace := "operator-ns"
4546-
opNamespace := &corev1.Namespace{
4547-
ObjectMeta: metav1.ObjectMeta{
4548-
Name: operatorNamespace,
4549-
},
4550-
}
45514546
serviceAccount := serviceAccount("sa", operatorNamespace)
45524547

45534548
type initial struct {
@@ -4665,7 +4660,7 @@ func TestOperatorGroupConditions(t *testing.T) {
46654660
UID: "cdc9643e-7c52-4f7c-ae75-28ccb6aec97d",
46664661
},
46674662
Spec: v1.OperatorGroupSpec{
4668-
TargetNamespaces: []string{operatorNamespace},
4663+
TargetNamespaces: []string{operatorNamespace, "some-namespace"},
46694664
},
46704665
},
46714666
},
@@ -4720,20 +4715,6 @@ func TestOperatorGroupConditions(t *testing.T) {
47204715
require.NoError(t, err)
47214716
}
47224717

4723-
// wait on operator group updated status to be in the cache
4724-
err = wait.PollImmediate(1*time.Millisecond, 5*time.Second, func() (bool, error) {
4725-
og, err := op.lister.OperatorsV1().OperatorGroupLister().OperatorGroups(tt.initial.operatorGroup.GetNamespace()).Get(tt.initial.operatorGroup.GetName())
4726-
if err != nil || og == nil {
4727-
return false, err
4728-
}
4729-
return true, nil
4730-
})
4731-
require.NoError(t, err)
4732-
4733-
// sync namespace
4734-
err = op.syncNamespace(opNamespace)
4735-
require.NoError(t, err)
4736-
47374718
operatorGroup, err := op.client.OperatorsV1().OperatorGroups(tt.initial.operatorGroup.GetNamespace()).Get(context.TODO(), tt.initial.operatorGroup.GetName(), metav1.GetOptions{})
47384719
require.NoError(t, err)
47394720
assert.Equal(t, len(tt.expectedConditions), len(operatorGroup.Status.Conditions))

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

+46-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
corev1 "k8s.io/api/core/v1"
1414
rbacv1 "k8s.io/api/rbac/v1"
1515
k8serrors "k8s.io/apimachinery/pkg/api/errors"
16+
meta "k8s.io/apimachinery/pkg/api/meta"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/labels"
1819
"k8s.io/apimachinery/pkg/util/errors"
@@ -64,8 +65,51 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
6465
"namespace": op.GetNamespace(),
6566
})
6667

68+
// Query OG in this namespace
69+
groups, err := a.lister.OperatorsV1().OperatorGroupLister().OperatorGroups(op.GetNamespace()).List(labels.Everything())
70+
if err != nil {
71+
logger.WithError(err).Warnf("failed to list OperatorGroups in the namespace")
72+
}
73+
74+
// Check if there is a stale multiple OG condition and clear it if existed.
75+
if len(groups) == 1 {
76+
og := groups[0]
77+
if c := meta.FindStatusCondition(og.Status.Conditions, v1.MutlipleOperatorGroupCondition); c != nil {
78+
meta.RemoveStatusCondition(&og.Status.Conditions, v1.MutlipleOperatorGroupCondition)
79+
if og.GetName() == op.GetName() {
80+
meta.RemoveStatusCondition(&op.Status.Conditions, v1.MutlipleOperatorGroupCondition)
81+
}
82+
_, err = a.client.OperatorsV1().OperatorGroups(op.GetNamespace()).UpdateStatus(context.TODO(), og, metav1.UpdateOptions{})
83+
if err != nil {
84+
logger.Warnf("fail to upgrade operator group status og=%s with condition %+v: %s", og.GetName(), c, err.Error())
85+
}
86+
}
87+
} else if len(groups) > 1 {
88+
// Add to all OG's status conditions to indicate they're multiple OGs in the
89+
// same namespace which is not allowed.
90+
cond := metav1.Condition{
91+
Type: v1.MutlipleOperatorGroupCondition,
92+
Status: metav1.ConditionTrue,
93+
Reason: v1.MultipleOperatorGroupsReason,
94+
Message: "Multiple OperatorGroup found in the same namespace",
95+
}
96+
for _, og := range groups {
97+
if c := meta.FindStatusCondition(og.Status.Conditions, v1.MutlipleOperatorGroupCondition); c != nil {
98+
continue
99+
}
100+
meta.SetStatusCondition(&og.Status.Conditions, cond)
101+
if og.GetName() == op.GetName() {
102+
meta.SetStatusCondition(&op.Status.Conditions, cond)
103+
}
104+
_, err = a.client.OperatorsV1().OperatorGroups(op.GetNamespace()).UpdateStatus(context.TODO(), og, metav1.UpdateOptions{})
105+
if err != nil {
106+
logger.Warnf("fail to upgrade operator group status og=%s with condition %+v: %s", og.GetName(), cond, err.Error())
107+
}
108+
}
109+
}
110+
67111
previousRef := op.Status.ServiceAccountRef.DeepCopy()
68-
op, err := a.serviceAccountSyncer.SyncOperatorGroup(op)
112+
op, err = a.serviceAccountSyncer.SyncOperatorGroup(op)
69113
if err != nil {
70114
logger.Errorf("error updating service account - %v", err)
71115
return err
@@ -109,6 +153,7 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
109153
op.Status = v1.OperatorGroupStatus{
110154
Namespaces: targetNamespaces,
111155
LastUpdated: a.now(),
156+
Conditions: op.Status.Conditions,
112157
}
113158

114159
if _, err = a.client.OperatorsV1().OperatorGroups(op.GetNamespace()).UpdateStatus(context.TODO(), op, metav1.UpdateOptions{}); err != nil && !k8serrors.IsNotFound(err) {

0 commit comments

Comments
 (0)