Skip to content

Commit

Permalink
Verify CRD's condition to ensure it's registered with k8s API
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@dinhxuanvu
dinhxuanvu committed Nov 19, 2018
1 parent 08ea39b commit fc4ff15
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 48 deletions.
6 changes: 4 additions & 2 deletions pkg/api/apis/operators/v1alpha1/clusterserviceversion_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
87 changes: 60 additions & 27 deletions pkg/controller/operators/olm/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -704,7 +737,7 @@ func TestTransitionCSV(t *testing.T) {
),
},
crds: []runtime.Object{
crd("c1", "v1"),
crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
},
},
expected: expected{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1004,7 +1037,7 @@ func TestTransitionCSV(t *testing.T) {
),
},
crds: []runtime.Object{
crd("c1", "v1"),
crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
},
},
expected: expected{
Expand Down Expand Up @@ -1073,7 +1106,7 @@ func TestTransitionCSV(t *testing.T) {
),
},
crds: []runtime.Object{
crd("c1", "v1"),
crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
},
},
expected: expected{
Expand Down Expand Up @@ -1117,7 +1150,7 @@ func TestTransitionCSV(t *testing.T) {
),
},
crds: []runtime.Object{
crd("c1", "v1"),
crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
},
},
expected: expected{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -1699,7 +1732,7 @@ func TestTransitionCSV(t *testing.T) {
),
},
crds: []runtime.Object{
crd("c1", "v1"),
crdWithConditions("c1", "v1", v1beta1.ConditionTrue, v1beta1.ConditionTrue),
},
},
expected: expected{
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
40 changes: 28 additions & 12 deletions pkg/controller/operators/olm/requirements.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -37,26 +38,41 @@ 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
// Check if CRD has successfully registered with k8s API
registered := false
for _, cdt := range crd.Status.Conditions {
switch cdt.Type {
case v1beta1.Established:
if cdt.Status == v1beta1.ConditionTrue {
status.UUID = string(crd.GetUID())
statuses = append(statuses, status)
served = true
registered = true
}
break
}
}

if !served {
status.Status = v1alpha1.RequirementStatusReasonNotPresent
if !registered {
status.Status = v1alpha1.RequirementStatusReasonNotAvailable
met = false
statuses = append(statuses, status)
}
Expand Down
Loading

0 comments on commit fc4ff15

Please sign in to comment.