Repository: knative/pkg
Pull request #1307 (2020-05-06 16:36:14)

Switch KRShaped interface to retrieve entire condition set
Issue #1226
- Switch interface to retrieve the entire conditions set

As I looked at integrating this downstream I've realized this will be more flexible so that we can have generic awareness of all the conditions that exist for a resource. This will also allow us to ensure they are initialized.
 

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1307#" 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"]} -->
whaught: /assign @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/1307/pull-knative-pkg-go-coverage/1258073193919287296/artifacts/line-cov.html#file0) | Do not exist | 50.0% | 

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/1307/pull-knative-pkg-go-coverage/1258075585226215424/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/1307/pull-knative-pkg-go-coverage/1258126168742170625/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/1307#issuecomment-624910682" title="Approved">n3wscott</a>*, *<a href="https://github.com/knative/pkg/pull/1307#" 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 b64977fcb..80558428c 100644
--- a/apis/duck/v1/kresource_type.go
+++ b/apis/duck/v1/kresource_type.go
@@ -26,7 +26,7 @@ import (
 	"knative.dev/pkg/apis"
 )
 
-// KRShaped is an interface for retrieving the duck elements of an arbitraty resource.
+// KRShaped is an interface for retrieving the duck elements of an arbitrary resource.
 type KRShaped interface {
 	metav1.ObjectMetaAccessor
 
@@ -34,7 +34,7 @@ type KRShaped interface {
 
 	GetStatus() *Status
 
-	GetTopLevelConditionType() apis.ConditionType
+	GetConditionSet() apis.ConditionSet
 }
 
 // Asserts KResource conformance with KRShaped
@@ -91,12 +91,12 @@ 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 {
+// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
+func (t *KResource) GetConditionSet() apis.ConditionSet {
 	// 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.NewBatchConditionSet()
 	}
-	return apis.ConditionReady
+	return apis.NewLivingConditionSet()
 }


Test patch:
diff --git a/apis/duck/v1/kresource_type_test.go b/apis/duck/v1/kresource_type_test.go
index 357353b14..e5c7f9371 100644
--- a/apis/duck/v1/kresource_type_test.go
+++ b/apis/duck/v1/kresource_type_test.go
@@ -22,22 +22,35 @@ import (
 	"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")
+func TestGetConditionSet(t *testing.T) {
+	testCases := []struct {
+		name                      string
+		condSet                   apis.ConditionSet
+		expectedTopLevelCondition apis.ConditionType
+	}{
+		{
+			name:                      "living set",
+			condSet:                   apis.NewLivingConditionSet(),
+			expectedTopLevelCondition: apis.ConditionReady,
+		},
+		{
+			name:                      "batch set",
+			condSet:                   apis.NewBatchConditionSet(),
+			expectedTopLevelCondition: apis.ConditionSucceeded,
+		},
 	}
 
-	condSet = apis.NewBatchConditionSet("Foo")
-	mgr = condSet.Manage(resource.GetStatus())
-	mgr.InitializeConditions()
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			resource := KResource{}
+			test.condSet.Manage(resource.GetStatus()).InitializeConditions()
+
+			mgr := resource.GetConditionSet().Manage(resource.GetStatus())
 
-	if resource.GetTopLevelConditionType() != apis.ConditionSucceeded {
-		t.Error("Expected Succeeded as happy condition for living condition set type")
+			if mgr.GetTopLevelCondition().Type != test.expectedTopLevelCondition {
+				t.Errorf("wrong top-level condition got=%s want=%s",
+					mgr.GetTopLevelCondition().Type, test.expectedTopLevelCondition)
+			}
+		})
 	}
 }
diff --git a/apis/test/example/v1alpha1/fiz_types.go b/apis/test/example/v1alpha1/fiz_types.go
index 334e827a5..b87559e2e 100644
--- a/apis/test/example/v1alpha1/fiz_types.go
+++ b/apis/test/example/v1alpha1/fiz_types.go
@@ -104,7 +104,7 @@ 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
+// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
+func (*ClusterFiz) GetConditionSet() apis.ConditionSet {
+	return apis.NewLivingConditionSet()
 }
diff --git a/apis/test/example/v1alpha1/foo_types.go b/apis/test/example/v1alpha1/foo_types.go
index 948254353..232612732 100644
--- a/apis/test/example/v1alpha1/foo_types.go
+++ b/apis/test/example/v1alpha1/foo_types.go
@@ -103,7 +103,7 @@ 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
+// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
+func (*Foo) GetConditionSet() apis.ConditionSet {
+	return apis.NewLivingConditionSet()
 }
diff --git a/apis/test/pub/v1alpha1/bar_types.go b/apis/test/pub/v1alpha1/bar_types.go
index f27d5f660..2227a6142 100644
--- a/apis/test/pub/v1alpha1/bar_types.go
+++ b/apis/test/pub/v1alpha1/bar_types.go
@@ -103,7 +103,7 @@ 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
+// GetConditionSet retrieves the condition set for this resource. Implements the KRShaped interface.
+func (*Bar) GetConditionSet() apis.ConditionSet {
+	return apis.NewLivingConditionSet()
 }