Skip to content

Commit 61486be

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #614 from jpeeler/jeff-533
Verify CRD's condition to ensure it's registered with k8s API (rebased)
2 parents c53c51a + 42ed784 commit 61486be

File tree

4 files changed

+158
-20
lines changed

4 files changed

+158
-20
lines changed

‎pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,10 @@ const (
254254
RequirementStatusReasonPresent StatusReason = "Present"
255255
RequirementStatusReasonNotPresent StatusReason = "NotPresent"
256256
RequirementStatusReasonPresentNotSatisfied StatusReason = "PresentNotSatisfied"
257-
DependentStatusReasonSatisfied StatusReason = "Satisfied"
258-
DependentStatusReasonNotSatisfied StatusReason = "NotSatisfied"
257+
// The CRD is present but the Established condition is False (not available)
258+
RequirementStatusReasonNotAvailable StatusReason = "PresentNotAvailable"
259+
DependentStatusReasonSatisfied StatusReason = "Satisfied"
260+
DependentStatusReasonNotSatisfied StatusReason = "NotSatisfied"
259261
)
260262

261263
// DependentStatus is the status for a dependent requirement (to prevent infinite nesting)
@@ -274,6 +276,7 @@ type RequirementStatus struct {
274276
Kind string `json:"kind"`
275277
Name string `json:"name"`
276278
Status StatusReason `json:"status"`
279+
Message string `json:"message"`
277280
UUID string `json:"uuid,omitempty"`
278281
Dependents []DependentStatus `json:"dependents,omitempty"`
279282
}
@@ -445,3 +448,4 @@ func (csv ClusterServiceVersion) GetOwnedAPIServiceDescriptions() []APIServiceDe
445448

446449
return descs
447450
}
451+

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
131131
}
132132

133133
// Create the new operator
134-
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake, logrus.New())
134+
queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake, logrus.StandardLogger())
135135
op := &Operator{
136136
Operator: queueOperator,
137137
client: clientFake,
@@ -566,6 +566,18 @@ func crd(name string, version string) *v1beta1.CustomResourceDefinition {
566566
Kind: name,
567567
},
568568
},
569+
Status: v1beta1.CustomResourceDefinitionStatus{
570+
Conditions: []v1beta1.CustomResourceDefinitionCondition{
571+
{
572+
Type: v1beta1.Established,
573+
Status: v1beta1.ConditionTrue,
574+
},
575+
{
576+
Type: v1beta1.NamesAccepted,
577+
Status: v1beta1.ConditionTrue,
578+
},
579+
},
580+
},
569581
}
570582
}
571583

@@ -2103,6 +2115,7 @@ func TestTransitionCSV(t *testing.T) {
21032115
}
21042116

21052117
func TestSyncOperatorGroups(t *testing.T) {
2118+
logrus.SetLevel(logrus.DebugLevel)
21062119
nowTime := metav1.Date(2006, time.January, 2, 15, 4, 5, 0, time.FixedZone("MST", -7*3600))
21072120
timeNow = func() metav1.Time { return nowTime }
21082121

@@ -2171,6 +2184,7 @@ func TestSyncOperatorGroups(t *testing.T) {
21712184
Kind: "CustomResourceDefinition",
21722185
Name: crd.GetName(),
21732186
Status: v1alpha1.RequirementStatusReasonPresent,
2187+
Message: "CRD is present and Established condition is true",
21742188
},
21752189
{
21762190
Group: "",

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

Lines changed: 47 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
1010
olmErrors "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/errors"
1111
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
12+
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1314
)
1415

@@ -29,36 +30,61 @@ func (a *Operator) requirementStatus(strategyDetailsDeployment *install.Strategy
2930
// check if CRD exists - this verifies group, version, and kind, so no need for GVK check via discovery
3031
crd, err := a.lister.APIExtensionsV1beta1().CustomResourceDefinitionLister().Get(r.Name)
3132
if err != nil {
32-
a.Log.Debugf("Setting 'met' to false, %v with err: %v", r.Name, err)
3333
status.Status = v1alpha1.RequirementStatusReasonNotPresent
34+
status.Message = "CRD is not present"
35+
a.Log.Debugf("Setting 'met' to false, %v with status %v, with err: %v", r.Name, status, err)
3436
met = false
3537
statuses = append(statuses, status)
3638
continue
3739
}
3840

39-
if crd.Spec.Version == r.Version {
40-
status.Status = v1alpha1.RequirementStatusReasonPresent
41-
status.UUID = string(crd.GetUID())
42-
statuses = append(statuses, status)
43-
continue
41+
if crd.Spec.Version != r.Version {
42+
served := false
43+
for _, version := range crd.Spec.Versions {
44+
if version.Name == r.Version {
45+
if version.Served {
46+
served = true
47+
}
48+
break
49+
}
50+
}
51+
52+
if !served {
53+
status.Status = v1alpha1.RequirementStatusReasonNotPresent
54+
status.Message = "CRD version not served"
55+
a.Log.Debugf("Setting 'met' to false, %v with status %v, CRD version %v not found", r.Name, status, r.Version)
56+
met = false
57+
statuses = append(statuses, status)
58+
continue
59+
}
4460
}
4561

46-
served := false
47-
for _, version := range crd.Spec.Versions {
48-
if version.Name == r.Version {
49-
if version.Served {
50-
status.Status = v1alpha1.RequirementStatusReasonPresent
51-
status.UUID = string(crd.GetUID())
52-
statuses = append(statuses, status)
53-
served = true
62+
// Check if CRD has successfully registered with k8s API
63+
established := false
64+
namesAccepted := false
65+
for _, cdt := range crd.Status.Conditions {
66+
switch cdt.Type {
67+
case v1beta1.Established:
68+
if cdt.Status == v1beta1.ConditionTrue {
69+
established = true
70+
}
71+
case v1beta1.NamesAccepted:
72+
if cdt.Status == v1beta1.ConditionTrue {
73+
namesAccepted = true
5474
}
55-
break
5675
}
5776
}
5877

59-
if !served {
60-
status.Status = v1alpha1.RequirementStatusReasonNotPresent
78+
if established && namesAccepted {
79+
status.Status = v1alpha1.RequirementStatusReasonPresent
80+
status.Message = "CRD is present and Established condition is true"
81+
status.UUID = string(crd.GetUID())
82+
statuses = append(statuses, status)
83+
} else {
84+
status.Status = v1alpha1.RequirementStatusReasonNotAvailable
85+
status.Message = "CRD is present but the Established condition is False (not available)"
6186
met = false
87+
a.Log.Debugf("Setting 'met' to false, %v with status %v, established=%v, namesAccepted=%v", r.Name, status, established, namesAccepted)
6288
statuses = append(statuses, status)
6389
}
6490
}
@@ -139,11 +165,13 @@ func (a *Operator) requirementStatus(strategyDetailsDeployment *install.Strategy
139165

140166
if err := a.isGVKRegistered(r.Group, r.Version, r.Kind); err != nil {
141167
status.Status = v1alpha1.RequirementStatusReasonNotPresent
168+
status.Message = "Native API does not exist"
142169
met = false
143170
statuses = append(statuses, status)
144171
continue
145172
} else {
146173
status.Status = v1alpha1.RequirementStatusReasonPresent
174+
status.Message = "Native API exists"
147175
statuses = append(statuses, status)
148176
continue
149177
}
@@ -181,6 +209,7 @@ func (a *Operator) permissionStatus(strategyDetailsDeployment *install.StrategyD
181209
if err != nil {
182210
met = false
183211
status.Status = v1alpha1.RequirementStatusReasonNotPresent
212+
status.Message = "Service account does not exist"
184213
statusesSet[saName] = status
185214
continue
186215
}
@@ -216,6 +245,7 @@ func (a *Operator) permissionStatus(strategyDetailsDeployment *install.StrategyD
216245
met = false
217246
dependent.Status = v1alpha1.DependentStatusReasonNotSatisfied
218247
status.Status = v1alpha1.RequirementStatusReasonPresentNotSatisfied
248+
status.Message = "Policy rule not satisfied for service account"
219249
} else {
220250
dependent.Status = v1alpha1.DependentStatusReasonSatisfied
221251
}

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,96 @@ func TestRequirementAndPermissionStatus(t *testing.T) {
453453
},
454454
expectedError: nil,
455455
},
456+
{
457+
description: "RequirementNotMet/NotEstablishedCRDVersion",
458+
csv: csv("csv1",
459+
namespace,
460+
"",
461+
installStrategy("csv1-dep", nil, nil),
462+
[]*v1beta1.CustomResourceDefinition{crd("c1", "version-not-found")},
463+
nil,
464+
v1alpha1.CSVPhasePending,
465+
),
466+
existingObjs: nil,
467+
existingExtObjs: []runtime.Object{
468+
crd("c1", "v2"),
469+
},
470+
met: false,
471+
expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{
472+
{"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition", "c1group"}: {
473+
Group: "apiextensions.k8s.io",
474+
Version: "v1beta1",
475+
Kind: "CustomResourceDefinition",
476+
Name: "c1group",
477+
Status: v1alpha1.RequirementStatusReasonNotAvailable,
478+
},
479+
},
480+
expectedError: nil,
481+
},
482+
{
483+
description: "RequirementNotMet/NamesConflictedCRD",
484+
csv: csv("csv1",
485+
namespace,
486+
"",
487+
installStrategy("csv1-dep", nil, nil),
488+
[]*v1beta1.CustomResourceDefinition{crd("c1", "v2")},
489+
nil,
490+
v1alpha1.CSVPhasePending,
491+
),
492+
existingObjs: nil,
493+
existingExtObjs: []runtime.Object{
494+
func() *v1beta1.CustomResourceDefinition {
495+
newCRD := crd("c1", "v2")
496+
// condition order: established, name accepted
497+
newCRD.Status.Conditions[0].Status = v1beta1.ConditionTrue
498+
newCRD.Status.Conditions[1].Status = v1beta1.ConditionFalse
499+
return newCRD
500+
}(),
501+
},
502+
met: false,
503+
expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{
504+
{"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition", "c1group"}: {
505+
Group: "apiextensions.k8s.io",
506+
Version: "v1beta1",
507+
Kind: "CustomResourceDefinition",
508+
Name: "c1group",
509+
Status: v1alpha1.RequirementStatusReasonNotAvailable,
510+
},
511+
},
512+
expectedError: nil,
513+
},
514+
{
515+
description: "RequirementNotMet/CRDResourceInactive",
516+
csv: csv("csv1",
517+
namespace,
518+
"",
519+
installStrategy("csv1-dep", nil, nil),
520+
[]*v1beta1.CustomResourceDefinition{crd("c1", "v2")},
521+
nil,
522+
v1alpha1.CSVPhasePending,
523+
),
524+
existingObjs: nil,
525+
existingExtObjs: []runtime.Object{
526+
func() *v1beta1.CustomResourceDefinition {
527+
newCRD := crd("c1", "v2")
528+
// condition order: established, name accepted
529+
newCRD.Status.Conditions[0].Status = v1beta1.ConditionFalse
530+
newCRD.Status.Conditions[1].Status = v1beta1.ConditionTrue
531+
return newCRD
532+
}(),
533+
},
534+
met: false,
535+
expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{
536+
{"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition", "c1group"}: {
537+
Group: "apiextensions.k8s.io",
538+
Version: "v1beta1",
539+
Kind: "CustomResourceDefinition",
540+
Name: "c1group",
541+
Status: v1alpha1.RequirementStatusReasonNotAvailable,
542+
},
543+
},
544+
expectedError: nil,
545+
},
456546
}
457547

458548
for _, test := range tests {

0 commit comments

Comments
 (0)