Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composite Receiver Functions for Conditions #738

Merged
merged 1 commit into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
negz marked this conversation as resolved.
Show resolved Hide resolved

// 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
Loading