diff --git a/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go b/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go index 7d343bf930..2df4db5b56 100644 --- a/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go +++ b/pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go @@ -252,8 +252,10 @@ const ( RequirementStatusReasonPresent StatusReason = "Present" RequirementStatusReasonNotPresent StatusReason = "NotPresent" RequirementStatusReasonPresentNotSatisfied StatusReason = "PresentNotSatisfied" - DependentStatusReasonSatisfied StatusReason = "Satisfied" - DependentStatusReasonNotSatisfied StatusReason = "NotSatisfied" + // The CRD is present but the Established condition is False (not available) + RequirementStatusReasonNotAvailable StatusReason = "PresentNotAvailable" + DependentStatusReasonSatisfied StatusReason = "Satisfied" + DependentStatusReasonNotSatisfied StatusReason = "NotSatisfied" ) // DependentStatus is the status for a dependent requirement (to prevent infinite nesting) diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go index 0c265fa40c..1bf502684e 100644 --- a/pkg/controller/operators/olm/operator_test.go +++ b/pkg/controller/operators/olm/operator_test.go @@ -603,6 +603,39 @@ func generateCA(notAfter time.Time, organization string) (*certs.KeyPair, error) return ca, nil } +func crdWithConditions(name string, version string, nameAccepted v1beta1.ConditionStatus, established v1beta1.ConditionStatus) *v1beta1.CustomResourceDefinition { + return &v1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name + "group", + }, + Spec: v1beta1.CustomResourceDefinitionSpec{ + Group: name + "group", + Versions: []v1beta1.CustomResourceDefinitionVersion{ + { + Name: version, + Storage: true, + Served: true, + }, + }, + Names: v1beta1.CustomResourceDefinitionNames{ + Kind: name, + }, + }, + Status: v1beta1.CustomResourceDefinitionStatus{ + Conditions: []v1beta1.CustomResourceDefinitionCondition{ + { + Type: v1beta1.Established, + Status: established, + }, + { + Type: v1beta1.NamesAccepted, + Status: nameAccepted, + }, + }, + }, + } +} + func TestTransitionCSV(t *testing.T) { logrus.SetLevel(logrus.DebugLevel) namespace := "ns" @@ -694,7 +727,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -730,7 +763,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ &corev1.ServiceAccount{ @@ -856,7 +889,7 @@ func TestTransitionCSV(t *testing.T) { ), apis("a1.v1.a1Kind"), nil), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -890,7 +923,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -994,7 +1027,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1063,7 +1096,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1107,7 +1140,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1130,7 +1163,7 @@ func TestTransitionCSV(t *testing.T) { ), apis("a1.v1.a1Kind"), nil), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1197,7 +1230,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1264,7 +1297,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1331,7 +1364,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1398,7 +1431,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1465,7 +1498,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1532,7 +1565,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1599,7 +1632,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1666,7 +1699,7 @@ func TestTransitionCSV(t *testing.T) { clusterRoleBinding("v1.a1-system:auth-delegator", "system:auth-delegator", "sa", namespace), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1689,7 +1722,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, }, expected: expected{ @@ -1712,7 +1745,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1766,7 +1799,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1801,7 +1834,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1837,7 +1870,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1882,7 +1915,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1928,7 +1961,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv1-dep1", namespace, "sa", nil), @@ -1966,7 +1999,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv2-dep1", namespace, "sa", nil), @@ -2003,7 +2036,7 @@ func TestTransitionCSV(t *testing.T) { ), }, crds: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, objs: []runtime.Object{ deployment("csv2-dep1", namespace, "sa", nil), diff --git a/pkg/controller/operators/olm/requirements.go b/pkg/controller/operators/olm/requirements.go index c9f8746da6..bd6180ab7f 100644 --- a/pkg/controller/operators/olm/requirements.go +++ b/pkg/controller/operators/olm/requirements.go @@ -3,11 +3,13 @@ package olm import ( "encoding/json" "fmt" + "github.com/sirupsen/logrus" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" olmErrors "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/errors" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -37,26 +39,47 @@ func (a *Operator) requirementStatus(strategyDetailsDeployment *install.Strategy if crd.Spec.Version == r.Version { status.Status = v1alpha1.RequirementStatusReasonPresent - status.UUID = string(crd.GetUID()) - statuses = append(statuses, status) - continue + } else { + served := false + for _, version := range crd.Spec.Versions { + if version.Name == r.Version { + if version.Served { + status.Status = v1alpha1.RequirementStatusReasonPresent + served = true + } + break + } + } + + if !served { + status.Status = v1alpha1.RequirementStatusReasonNotPresent + met = false + statuses = append(statuses, status) + continue + } } - served := false - for _, version := range crd.Spec.Versions { - if version.Name == r.Version { - if version.Served { - status.Status = v1alpha1.RequirementStatusReasonPresent - status.UUID = string(crd.GetUID()) - statuses = append(statuses, status) - served = true + // Check if CRD has successfully registered with k8s API + established := false + namesAccepted := false + for _, cdt := range crd.Status.Conditions { + switch cdt.Type { + case v1beta1.Established: + if cdt.Status == v1beta1.ConditionTrue { + established = true + } + case v1beta1.NamesAccepted: + if cdt.Status == v1beta1.ConditionTrue { + namesAccepted = true } - break } } - if !served { - status.Status = v1alpha1.RequirementStatusReasonNotPresent + if established && namesAccepted { + status.UUID = string(crd.GetUID()) + statuses = append(statuses, status) + } else { + status.Status = v1alpha1.RequirementStatusReasonNotAvailable met = false statuses = append(statuses, status) } diff --git a/pkg/controller/operators/olm/requirements_test.go b/pkg/controller/operators/olm/requirements_test.go index 2a38edc17a..712d802f6f 100644 --- a/pkg/controller/operators/olm/requirements_test.go +++ b/pkg/controller/operators/olm/requirements_test.go @@ -44,9 +44,9 @@ func TestRequirementAndPermissionStatus(t *testing.T) { nil, v1alpha1.CSVPhasePending, ), - existingObjs: nil, - existingExtObjs: nil, - met: false, + existingObjs: nil, + existingExtObjs: nil, + met: false, expectedRequirementStatuses: nil, expectedError: fmt.Errorf("unexpected end of JSON input"), }, @@ -377,8 +377,8 @@ func TestRequirementAndPermissionStatus(t *testing.T) { }, }, existingExtObjs: []runtime.Object{ - crd("c1", "v1"), - crd("c2", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), + crdWithConditions("c2", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, met: true, expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{ @@ -439,7 +439,7 @@ func TestRequirementAndPermissionStatus(t *testing.T) { ), existingObjs: nil, existingExtObjs: []runtime.Object{ - crd("c1", "v1"), + crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue), }, met: false, expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{ @@ -453,6 +453,58 @@ func TestRequirementAndPermissionStatus(t *testing.T) { }, expectedError: nil, }, + { + description: "RequirementNotMet/NotEstablishedCRDVersion", + csv: csv("csv1", + namespace, + "", + installStrategy("csv1-dep", nil, nil), + []*v1beta1.CustomResourceDefinition{crd("c1", "v2")}, + nil, + v1alpha1.CSVPhasePending, + ), + existingObjs: nil, + existingExtObjs: []runtime.Object{ + crdWithConditions("c1", "v2", v1beta1.ConditionTrue, v1beta1.ConditionFalse), + }, + met: false, + expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{ + {"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition", "c1group"}: { + Group: "apiextensions.k8s.io", + Version: "v1beta1", + Kind: "CustomResourceDefinition", + Name: "c1group", + Status: v1alpha1.RequirementStatusReasonNotAvailable, + }, + }, + expectedError: nil, + }, + { + description: "RequirementNotMet/NamesConflictedCRDVersion", + csv: csv("csv1", + namespace, + "", + installStrategy("csv1-dep", nil, nil), + []*v1beta1.CustomResourceDefinition{crd("c1", "v2")}, + nil, + v1alpha1.CSVPhasePending, + ), + existingObjs: nil, + existingExtObjs: []runtime.Object{ + crdWithConditions("c1", "v2", v1beta1.ConditionFalse, v1beta1.ConditionFalse), + }, + met: false, + expectedRequirementStatuses: map[gvkn]v1alpha1.RequirementStatus{ + {"apiextensions.k8s.io", "v1beta1", "CustomResourceDefinition", "c1group"}: { + Group: "apiextensions.k8s.io", + Version: "v1beta1", + Kind: "CustomResourceDefinition", + Name: "c1group", + Status: v1alpha1.RequirementStatusReasonNotAvailable, + }, + }, + expectedError: nil, + }, } for _, test := range tests {