Verify CRD's condition to ensure it's registered with k8s API

OLM needs to verify CRD's condition to make sure it is registered with k8s API during CSV control loop before installing operator. This verification will help to avoid errors related to the unavailability of an API when installing an operator via OLM. Jira: https://jira.coreos.com/browse/ALM-775 Signed-off-by: Vu Dinh <vdinh@redhat.com>
operator-framework · Nov 27, 2018 · f0d4823 · f0d4823
1 parent 08ea39b
commit f0d4823
Show file tree

Hide file tree

Showing 4 changed files with 158 additions and 49 deletions.
diff --git 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
@@ -22,6 +22,7 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
+	aextv1beta1 "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -33,7 +34,6 @@ import (
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	apiregistrationfake "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake"
 	kagg "k8s.io/kube-aggregator/pkg/client/informers/externalversions"
-	aextv1beta1 "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions"
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha2"
@@ -155,7 +155,7 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
 	namespaceInformer := informerFactory.Core().V1().Namespaces()
 	apiServiceInformer := kagg.NewSharedInformerFactory(opClientFake.ApiregistrationV1Interface(), wakeupInterval).Apiregistration().V1().APIServices()
 	customResourceDefinitionInformer := aextv1beta1.NewSharedInformerFactory(opClientFake.ApiextensionsV1beta1Interface(), wakeupInterval).Apiextensions().V1beta1().CustomResourceDefinitions()
-	
+
 	// Register informers
 	informerList := []cache.SharedIndexInformer{
 		roleInformer.Informer(),
@@ -613,6 +613,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) {
 	log.SetLevel(log.DebugLevel)
 	namespace := "ns"
@@ -704,7 +737,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 			},
 			expected: expected{
@@ -740,7 +773,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 				objs: []runtime.Object{
 					&corev1.ServiceAccount{
@@ -866,7 +899,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{
@@ -900,7 +933,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),
@@ -1004,7 +1037,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 			},
 			expected: expected{
@@ -1073,7 +1106,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 			},
 			expected: expected{
@@ -1117,7 +1150,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 			},
 			expected: expected{
@@ -1140,7 +1173,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{
@@ -1207,7 +1240,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{
@@ -1274,7 +1307,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{
@@ -1341,7 +1374,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{
@@ -1408,7 +1441,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{
@@ -1475,7 +1508,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{
@@ -1542,7 +1575,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{
@@ -1609,7 +1642,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{
@@ -1676,7 +1709,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{
@@ -1699,7 +1732,7 @@ func TestTransitionCSV(t *testing.T) {
 					),
 				},
 				crds: []runtime.Object{
-					crd("c1", "v1"),
+					crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
 				},
 			},
 			expected: expected{
@@ -1722,7 +1755,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),
@@ -1776,7 +1809,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),
@@ -1811,7 +1844,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),
@@ -1847,7 +1880,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),
@@ -1892,7 +1925,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),
@@ -1938,7 +1971,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),
@@ -1976,7 +2009,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),
@@ -2013,7 +2046,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
@@ -8,6 +8,7 @@ import (
 	olmErrors "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/errors"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
 	log "github.com/sirupsen/logrus"
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -37,26 +38,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)
 		}