Repository: knative/pkg
Pull request #1298 (2020-05-05 19:22:24)

Include GetTopLevelConditionType for KRShaped
Issue #1226

- Each type will report its expected happy condition
- KRShaped will detect its happy type by looking at its own conditions.
- Added a unit test

At the moment each implementer either makes a LivingConditionSet manager or a BatchConditionSet manager within each resource. However this allows for polymorphic logic since each type will report its own happy condition.

Longer term we could simplify by just having GetConditionSet(resource) and the manager can figure out what the condition ought to be. This will make it easier to share common logic.

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1298#" title="Author self-approved">whaught</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **n3wscott**
You can assign the PR to them by writing `/assign @n3wscott` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["n3wscott"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1298/pull-knative-pkg-go-coverage/1257752706940932097/artifacts/line-cov.html#file0) | Do not exist | 87.5% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[apis/duck/v1/kresource_type.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1298/pull-knative-pkg-go-coverage/1257777495961243648/artifacts/line-cov.html#file0) | Do not exist | 87.5% | 

n3wscott: /lgtm
/approve
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1298#issuecomment-624312829" title="Approved">n3wscott</a>*, *<a href="https://github.com/knative/pkg/pull/1298#" title="Author self-approved">whaught</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[apis/OWNERS](https://github.com/knative/pkg/blob/master/apis/OWNERS)~~ [n3wscott]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/apis/duck/v1/kresource_type.go b/apis/duck/v1/kresource_type.go
index 7ea2d70c9..b64977fcb 100644
--- a/apis/duck/v1/kresource_type.go
+++ b/apis/duck/v1/kresource_type.go
@@ -33,6 +33,8 @@ type KRShaped interface {
 	GetTypeMeta() *metav1.TypeMeta
 
 	GetStatus() *Status
+
+	GetTopLevelConditionType() apis.ConditionType
 }
 
 // Asserts KResource conformance with KRShaped
@@ -88,3 +90,13 @@ func (t *KResource) GetTypeMeta() *metav1.TypeMeta {
 func (t *KResource) GetStatus() *Status {
 	return &t.Status
 }
+
+// GetTopLevelConditionType retrieves the happy condition of this resource. Implements the KRShaped interface.
+func (t *KResource) GetTopLevelConditionType() apis.ConditionType {
+	// Note: KResources are unmarshalled from existing resources. This will only work properly for resources that
+	// have already been initialized to their type.
+	if cond := t.Status.GetCondition(apis.ConditionSucceeded); cond != nil {
+		return apis.ConditionSucceeded
+	}
+	return apis.ConditionReady
+}


Test patch:
diff --git a/apis/duck/v1/kresource_type_test.go b/apis/duck/v1/kresource_type_test.go
new file mode 100644
index 000000000..357353b14
--- /dev/null
+++ b/apis/duck/v1/kresource_type_test.go
@@ -0,0 +1,43 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import (
+	"testing"
+
+	"knative.dev/pkg/apis"
+)
+
+func TestGetTopLevelCondition(t *testing.T) {
+	resource := KResource{}
+
+	condSet := apis.NewLivingConditionSet("Foo")
+	mgr := condSet.Manage(resource.GetStatus())
+	mgr.InitializeConditions()
+
+	if resource.GetTopLevelConditionType() != apis.ConditionReady {
+		t.Error("Expected Ready as happy condition for living condition set type")
+	}
+
+	condSet = apis.NewBatchConditionSet("Foo")
+	mgr = condSet.Manage(resource.GetStatus())
+	mgr.InitializeConditions()
+
+	if resource.GetTopLevelConditionType() != apis.ConditionSucceeded {
+		t.Error("Expected Succeeded as happy condition for living condition set type")
+	}
+}
diff --git a/apis/test/example/v1alpha1/fiz_types.go b/apis/test/example/v1alpha1/fiz_types.go
index 513366886..334e827a5 100644
--- a/apis/test/example/v1alpha1/fiz_types.go
+++ b/apis/test/example/v1alpha1/fiz_types.go
@@ -103,3 +103,8 @@ func (f *ClusterFiz) GetTypeMeta() *metav1.TypeMeta {
 func (f *ClusterFiz) GetStatus() *duckv1.Status {
 	return &f.Status.Status
 }
+
+// GetTopLevelConditionType retrieves the happy condition of this resource. Implements the KRShaped interface.
+func (*ClusterFiz) GetTopLevelConditionType() apis.ConditionType {
+	return apis.ConditionReady
+}
diff --git a/apis/test/example/v1alpha1/foo_types.go b/apis/test/example/v1alpha1/foo_types.go
index 616a7c6a9..948254353 100644
--- a/apis/test/example/v1alpha1/foo_types.go
+++ b/apis/test/example/v1alpha1/foo_types.go
@@ -102,3 +102,8 @@ func (f *Foo) GetTypeMeta() *metav1.TypeMeta {
 func (f *Foo) GetStatus() *duckv1.Status {
 	return &f.Status.Status
 }
+
+// GetTopLevelConditionType retrieves the happy condition of this resource. Implements the KRShaped interface.
+func (*Foo) GetTopLevelConditionType() apis.ConditionType {
+	return apis.ConditionSucceeded
+}
diff --git a/apis/test/pub/v1alpha1/bar_types.go b/apis/test/pub/v1alpha1/bar_types.go
index 90b371fb7..f27d5f660 100644
--- a/apis/test/pub/v1alpha1/bar_types.go
+++ b/apis/test/pub/v1alpha1/bar_types.go
@@ -102,3 +102,8 @@ func (b *Bar) GetTypeMeta() *metav1.TypeMeta {
 func (b *Bar) GetStatus() *duckv1.Status {
 	return &b.Status.Status
 }
+
+// GetTopLevelConditionType retrieves the happy condition of this resource. Implements the KRShaped interface.
+func (*Bar) GetTopLevelConditionType() apis.ConditionType {
+	return apis.ConditionReady
+}