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 · Dec 10, 2018 · a75c00f · a75c00f
1 parent af566ee
commit a75c00f
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 47 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
@@ -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
@@ -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)
 		}