Skip to content

Commit

Permalink
Composite receiver functions for conditions.
Browse files Browse the repository at this point in the history
Signed-off-by: Dalton Hill <[email protected]>
  • Loading branch information
dalton hill authored and dalton-hill-0 committed Jun 26, 2024
1 parent 77ead40 commit 2d6e2ae
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 21 deletions.
21 changes: 21 additions & 0 deletions apis/common/v1/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ const (
// TypeSynced resources are believed to be in sync with the
// Kubernetes resources that manage their lifecycle.
TypeSynced ConditionType = "Synced"

// TypeHealthy resources are believed to be in a healthy state and to have all
// of their child resources in a healthy state. For example, a claim is
// healthy when the claim is synced and the underlying composite resource is
// both synced and healthy. A composite resource is healthy when the composite
// resource is synced and all composed resources are synced and, if
// applicable, healthy (e.g., the composed resource is a composite resource).
// TODO: This condition is not yet implemented. It is currently just reserved
// as a system condition. See the tracking issue for more details
// https://github.com/crossplane/crossplane/issues/5643.
TypeHealthy ConditionType = "Healthy"
)

// A ConditionReason represents the reason a resource is in a condition.
Expand Down Expand Up @@ -107,6 +118,16 @@ func (c Condition) WithObservedGeneration(gen int64) Condition {
return c
}

// IsSystemConditionType returns true if the condition is owned by the
// Crossplane system (e.g, Ready, Synced, Healthy).
func IsSystemConditionType(t ConditionType) bool {
switch t {
case TypeReady, TypeSynced, TypeHealthy:
return true
}
return false
}

// NOTE(negz): Conditions are implemented as a slice rather than a map to comply
// with Kubernetes API conventions. Ideally we'd comply by using a map that
// marshalled to a JSON array, but doing so confuses the CRD schema generator.
Expand Down
32 changes: 32 additions & 0 deletions apis/common/v1/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,35 @@ func TestConditionWithObservedGeneration(t *testing.T) {
})
}
}

func TestIsSystemConditionType(t *testing.T) {
cases := map[string]struct {
c Condition
want bool
}{
"SystemReady": {
c: Condition{Type: ConditionType("Ready")},
want: true,
},
"SystemSynced": {
c: Condition{Type: ConditionType("Synced")},
want: true,
},
"SystemHealthy": {
c: Condition{Type: ConditionType("Healthy")},
want: true,
},
"Custom": {
c: Condition{Type: ConditionType("Custom")},
want: false,
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
if diff := cmp.Diff(tc.want, IsSystemConditionType(tc.c.Type)); diff != "" {
t.Errorf("IsSystemConditionType(tc.c.Type): -want, +got:\n%s", diff)
}
})
}
}
39 changes: 39 additions & 0 deletions pkg/resource/unstructured/composite/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"

xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
"github.com/crossplane/crossplane-runtime/pkg/errors"
"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
)
Expand Down Expand Up @@ -223,6 +224,14 @@ func (c *Unstructured) SetConditions(conditions ...xpv1.Condition) {
_ = fieldpath.Pave(c.Object).SetValue("status.conditions", conditioned.Conditions)
}

// GetConditions of this Composite resource.
func (c *Unstructured) GetConditions() []xpv1.Condition {
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
_ = fieldpath.Pave(c.Object).GetValueInto("status", &conditioned)
return conditioned.Conditions
}

// GetConnectionDetailsLastPublishedTime of this Composite resource.
func (c *Unstructured) GetConnectionDetailsLastPublishedTime() *metav1.Time {
out := &metav1.Time{}
Expand Down Expand Up @@ -273,3 +282,33 @@ func (c *Unstructured) GetObservedGeneration() int64 {
_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
return status.GetObservedGeneration()
}

// SetClaimConditionTypes of this Composite resource. You cannot set system
// condition types such as Ready, Synced or Healthy as claim conditions.
func (c *Unstructured) SetClaimConditionTypes(in ...xpv1.ConditionType) error {
ts := c.GetClaimConditionTypes()
m := make(map[xpv1.ConditionType]bool, len(ts))
for _, t := range ts {
m[t] = true
}

for _, t := range in {
if xpv1.IsSystemConditionType(t) {
return errors.Errorf("cannot set system condition %s as a claim condition", t)
}
if m[t] {
continue
}
m[t] = true
ts = append(ts, t)
}
_ = fieldpath.Pave(c.Object).SetValue("status.claimConditionTypes", ts)
return nil
}

// GetClaimConditionTypes of this Composite resource.
func (c *Unstructured) GetClaimConditionTypes() []xpv1.ConditionType {
cs := []xpv1.ConditionType{}
_ = fieldpath.Pave(c.Object).GetValueInto("status.claimConditionTypes", &cs)
return cs
}
128 changes: 107 additions & 21 deletions pkg/resource/unstructured/composite/composite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package composite

import (
"encoding/json"
"errors"
"testing"
"time"

Expand All @@ -30,6 +31,7 @@ import (

xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
"github.com/crossplane/crossplane-runtime/pkg/test"
)

var _ client.Object = &Unstructured{}
Expand Down Expand Up @@ -69,34 +71,38 @@ func TestWithGroupVersionKind(t *testing.T) {

func TestConditions(t *testing.T) {
cases := map[string]struct {
reason string
u *Unstructured
set []xpv1.Condition
get xpv1.ConditionType
want xpv1.Condition
reason string
u *Unstructured
set []xpv1.Condition
get xpv1.ConditionType
want xpv1.Condition
wantAll []xpv1.Condition
}{
"NewCondition": {
reason: "It should be possible to set a condition of an empty Unstructured.",
u: New(),
set: []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
get: xpv1.TypeReady,
want: xpv1.Available(),
reason: "It should be possible to set a condition of an empty Unstructured.",
u: New(),
set: []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
get: xpv1.TypeReady,
want: xpv1.Available(),
wantAll: []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
},
"ExistingCondition": {
reason: "It should be possible to overwrite a condition that is already set.",
u: New(WithConditions(xpv1.Creating())),
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Available(),
reason: "It should be possible to overwrite a condition that is already set.",
u: New(WithConditions(xpv1.Creating())),
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Available(),
wantAll: []xpv1.Condition{xpv1.Available()},
},
"WeirdStatus": {
reason: "It should not be possible to set a condition when status is not an object.",
u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
"status": "wat",
}}},
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Condition{},
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Condition{},
wantAll: nil,
},
"WeirdStatusConditions": {
reason: "Conditions should be overwritten if they are not an object.",
Expand All @@ -105,19 +111,99 @@ func TestConditions(t *testing.T) {
"conditions": "wat",
},
}}},
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Available(),
set: []xpv1.Condition{xpv1.Available()},
get: xpv1.TypeReady,
want: xpv1.Available(),
wantAll: []xpv1.Condition{xpv1.Available()},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
tc.u.SetConditions(tc.set...)

got := tc.u.GetCondition(tc.get)
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("\n%s\nu.GetCondition(%s): -want, +got:\n%s", tc.reason, tc.get, diff)
}

gotAll := tc.u.GetConditions()
if diff := cmp.Diff(tc.wantAll, gotAll); diff != "" {
t.Errorf("\n%s\nu.GetConditions(): -want, +got:\n%s", tc.reason, diff)
}
})
}
}

func TestClaimConditionTypes(t *testing.T) {
cases := map[string]struct {
reason string
u *Unstructured
set []xpv1.ConditionType
want []xpv1.ConditionType
wantErr error
}{
"CannotSetSystemConditionTypes": {
reason: "Claim conditions API should fail to set conditions if a system condition is detected.",
u: New(),
set: []xpv1.ConditionType{
xpv1.ConditionType("DatabaseReady"),
xpv1.ConditionType("NetworkReady"),
// system condition
xpv1.ConditionType("Ready"),
},
want: []xpv1.ConditionType{},
wantErr: errors.New("cannot set system condition Ready as a claim condition"),
},
"SetSingleCustomConditionType": {
reason: "Claim condition API should work with a single custom condition type.",
u: New(),
set: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
want: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
},
"SetMultipleCustomConditionTypes": {
reason: "Claim condition API should work with multiple custom condition types.",
u: New(),
set: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("NetworkReady")},
want: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("NetworkReady")},
},
"SetMultipleOfTheSameCustomConditionTypes": {
reason: "Claim condition API not add more than one of the same condition.",
u: New(),
set: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("DatabaseReady")},
want: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
},
"WeirdStatus": {
reason: "It should not be possible to set a condition when status is not an object.",
u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
"status": "wat",
}}},
set: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
want: []xpv1.ConditionType{},
},
"WeirdStatusClaimConditionTypes": {
reason: "Claim conditions should be overwritten if they are not an object.",
u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
"status": map[string]any{
"claimConditionTypes": "wat",
},
}}},
set: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
want: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
gotErr := tc.u.SetClaimConditionTypes(tc.set...)
if diff := cmp.Diff(tc.wantErr, gotErr, test.EquateErrors()); diff != "" {
t.Errorf("\n%s\nu.SetClaimConditionTypes(): -want, +got:\n%s", tc.reason, diff)
}

got := tc.u.GetClaimConditionTypes()
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("\n%s\nu.GetClaimConditionTypes(): -want, +got:\n%s", tc.reason, diff)
}
})
}
}
Expand Down

0 comments on commit 2d6e2ae

Please sign in to comment.