Skip to content

Commit 3cd174d

Browse files
fabriziopandinifabriziopandini
committed
shift to an interface based implementation (instead of using reflection)
1 parent 9e7ac0f commit 3cd174d

File tree

11 files changed

+440
-703
lines changed

11 files changed

+440
-703
lines changed

‎internal/test/builder/v1beta2_transition.go

+42
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,22 @@ func (o *Phase1Obj) SetConditions(conditions clusterv1.Conditions) {
151151
o.Status.Conditions = conditions
152152
}
153153

154+
// GetV1Beta2Conditions returns the set of conditions for this object.
155+
func (o *Phase1Obj) GetV1Beta2Conditions() []metav1.Condition {
156+
if o.Status.V1Beta2 == nil {
157+
return nil
158+
}
159+
return o.Status.V1Beta2.Conditions
160+
}
161+
162+
// SetV1Beta2Conditions sets conditions for an API object.
163+
func (o *Phase1Obj) SetV1Beta2Conditions(conditions []metav1.Condition) {
164+
if o.Status.V1Beta2 == nil && conditions != nil {
165+
o.Status.V1Beta2 = &Phase1ObjStatusV1Beta2{}
166+
}
167+
o.Status.V1Beta2.Conditions = conditions
168+
}
169+
154170
// Phase2ObjList is a list of Phase2Obj.
155171
// +kubebuilder:object:root=true
156172
type Phase2ObjList struct {
@@ -212,9 +228,25 @@ func (o *Phase2Obj) GetConditions() clusterv1.Conditions {
212228

213229
// SetConditions sets the conditions on this object.
214230
func (o *Phase2Obj) SetConditions(conditions clusterv1.Conditions) {
231+
if o.Status.Deprecated == nil && conditions != nil {
232+
o.Status.Deprecated = &Phase2ObjStatusDeprecated{}
233+
}
234+
if o.Status.Deprecated.V1Beta1 == nil && conditions != nil {
235+
o.Status.Deprecated.V1Beta1 = &Phase2ObjStatusDeprecatedV1Beta1{}
236+
}
215237
o.Status.Deprecated.V1Beta1.Conditions = conditions
216238
}
217239

240+
// GetV1Beta2Conditions returns the set of conditions for this object.
241+
func (o *Phase2Obj) GetV1Beta2Conditions() []metav1.Condition {
242+
return o.Status.Conditions
243+
}
244+
245+
// SetV1Beta2Conditions sets conditions for an API object.
246+
func (o *Phase2Obj) SetV1Beta2Conditions(conditions []metav1.Condition) {
247+
o.Status.Conditions = conditions
248+
}
249+
218250
// Phase3ObjList is a list of Phase3Obj.
219251
// +kubebuilder:object:root=true
220252
type Phase3ObjList struct {
@@ -251,3 +283,13 @@ type Phase3ObjStatus struct {
251283
// +listMapKey=type
252284
Conditions []metav1.Condition `json:"conditions,omitempty"`
253285
}
286+
287+
// GetV1Beta2Conditions returns the set of conditions for this object.
288+
func (o *Phase3Obj) GetV1Beta2Conditions() []metav1.Condition {
289+
return o.Status.Conditions
290+
}
291+
292+
// SetV1Beta2Conditions sets conditions for an API object.
293+
func (o *Phase3Obj) SetV1Beta2Conditions(conditions []metav1.Condition) {
294+
o.Status.Conditions = conditions
295+
}

‎util/conditions/v1beta2/aggregate.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121

2222
"github.com/pkg/errors"
2323
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24-
"k8s.io/apimachinery/pkg/runtime"
2524
)
2625

2726
// AggregateOption is some configuration that modifies options for a aggregate call.
@@ -52,7 +51,7 @@ func (o *AggregateOptions) ApplyOptions(opts []AggregateOption) *AggregateOption
5251
// the TargetConditionType option.
5352
//
5453
// Additionally, it is possible to inject custom merge strategies using the CustomMergeStrategy option.
55-
func NewAggregateCondition(sourceObjs []runtime.Object, sourceConditionType string, opts ...AggregateOption) (*metav1.Condition, error) {
54+
func NewAggregateCondition(sourceObjs []Getter, sourceConditionType string, opts ...AggregateOption) (*metav1.Condition, error) {
5655
if len(sourceObjs) == 0 {
5756
return nil, errors.New("sourceObjs can't be empty")
5857
}
@@ -65,12 +64,7 @@ func NewAggregateCondition(sourceObjs []runtime.Object, sourceConditionType stri
6564

6665
conditionsInScope := make([]ConditionWithOwnerInfo, 0, len(sourceObjs))
6766
for _, obj := range sourceObjs {
68-
conditions, err := getConditionsWithOwnerInfo(obj)
69-
if err != nil {
70-
// Note: considering all sourceObjs are usually of the same type (and thus getConditionsWithOwnerInfo will either pass or fail for all sourceObjs), we are returning at the first error.
71-
// This also avoid to implement fancy error aggregation, which is required to manage a potentially high number of sourceObjs/errors.
72-
return nil, err
73-
}
67+
conditions := getConditionsWithOwnerInfo(obj)
7468

7569
// Drops all the conditions not in scope for the merge operation
7670
hasConditionType := false
@@ -124,10 +118,11 @@ func NewAggregateCondition(sourceObjs []runtime.Object, sourceConditionType stri
124118

125119
// SetAggregateCondition is a convenience method that calls NewAggregateCondition to create an aggregate condition from the source objects,
126120
// and then calls Set to add the new condition to the target object.
127-
func SetAggregateCondition(sourceObjs []runtime.Object, targetObj runtime.Object, conditionType string, opts ...AggregateOption) error {
121+
func SetAggregateCondition(sourceObjs []Getter, targetObj Setter, conditionType string, opts ...AggregateOption) error {
128122
aggregateCondition, err := NewAggregateCondition(sourceObjs, conditionType, opts...)
129123
if err != nil {
130124
return err
131125
}
132-
return Set(targetObj, *aggregateCondition)
126+
Set(targetObj, *aggregateCondition)
127+
return nil
133128
}

‎util/conditions/v1beta2/aggregate_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
. "github.com/onsi/gomega"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25-
"k8s.io/apimachinery/pkg/runtime"
2625

2726
"sigs.k8s.io/cluster-api/internal/test/builder"
2827
)
@@ -251,7 +250,7 @@ func TestAggregate(t *testing.T) {
251250
t.Run(tt.name, func(t *testing.T) {
252251
g := NewWithT(t)
253252

254-
objs := make([]runtime.Object, 0, len(tt.conditions))
253+
objs := make([]Getter, 0, len(tt.conditions))
255254
for i := range tt.conditions {
256255
objs = append(objs, &builder.Phase3Obj{
257256
ObjectMeta: metav1.ObjectMeta{

‎util/conditions/v1beta2/getter.go

+47-147
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ import (
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2626
"k8s.io/apimachinery/pkg/runtime"
27-
28-
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2927
)
3028

3129
// TODO: Move to the API package.
@@ -34,99 +32,88 @@ const (
3432
NoReasonReported = "NoReasonReported"
3533
)
3634

37-
// MustGet is like Get except that it panics on error.
38-
func MustGet(sourceObj runtime.Object, sourceConditionType string) *metav1.Condition {
39-
v, err := Get(sourceObj, sourceConditionType)
40-
if err != nil {
41-
panic(err)
42-
}
43-
return v
35+
// Getter interface defines methods that an API object should implement in order to
36+
// use the conditions package for getting conditions.
37+
type Getter interface {
38+
// GetV1Beta2Conditions returns the list of conditions for a cluster API object.
39+
// Note: GetV1Beta2Conditions will be renamed to GetConditions in a later stage of the transition to V1Beta2.
40+
GetV1Beta2Conditions() []metav1.Condition
4441
}
4542

46-
// Get returns a condition from the sourceObj.
43+
// Get returns a condition from the object implementing the Getter interface.
4744
//
48-
// Get supports retrieving conditions from objects at different stages of the transition to the metav1.Condition type:
49-
// - Objects with clusterv1.Conditions in status.conditions; in this case a best effort conversion
50-
// to metav1.Condition is performed, just enough to allow surfacing a condition from a provider object with Mirror
51-
// - Objects with metav1.Condition in status.v1beta2.conditions
52-
// - Objects with metav1.Condition in status.conditions
53-
//
54-
// Please note that Get also supports reading conditions from unstructured objects; in this case, best effort
55-
// conversion from a map is performed, just enough to allow surfacing a condition from a providers object with Mirror
56-
//
57-
// In case the object does not have metav1.Conditions, Get tries to read clusterv1.Conditions from status.conditions
58-
// and convert them to metav1.Conditions.
59-
func Get(sourceObj runtime.Object, sourceConditionType string) (*metav1.Condition, error) {
60-
conditions, err := GetAll(sourceObj)
61-
if err != nil {
62-
return nil, err
45+
// Please note that Get does not support reading conditions from unstructured objects nor from API types not implementing
46+
// the Getter interface. Eventually, users can implement wrappers on those types implementing this interface and
47+
// taking care of aligning the condition format if necessary.
48+
func Get(sourceObj Getter, sourceConditionType string) *metav1.Condition {
49+
// if obj is nil, the requested condition does not exist.
50+
if sourceObj == nil {
51+
return nil
6352
}
64-
return meta.FindStatusCondition(conditions, sourceConditionType), nil
53+
54+
// if obj is a zero value of its type (a pointer to nil), the requested condition does not exist.
55+
ptr := reflect.ValueOf(sourceObj)
56+
if ptr.Kind() != reflect.Pointer {
57+
// Note: this should never happen, given that Getter is an interface.
58+
return nil
59+
}
60+
61+
elem := ptr.Elem()
62+
if !elem.IsValid() {
63+
return nil
64+
}
65+
66+
// Otherwise get the requested condition.
67+
return meta.FindStatusCondition(sourceObj.GetV1Beta2Conditions(), sourceConditionType)
6568
}
6669

67-
// GetAll returns all the conditions from the object.
70+
// UnstructuredGet returns a condition from an Unstructured object.
6871
//
69-
// GetAll supports retrieving conditions from objects at different stages of the transition to the metav1.Condition type:
72+
// UnstructuredGet supports retrieving conditions from objects at different stages of the transition from
73+
// clusterv1.conditions to the metav1.Condition type:
7074
// - Objects with clusterv1.Conditions in status.conditions; in this case a best effort conversion
71-
// to metav1.Condition is performed, just enough to allow surfacing a condition from a providers object with Mirror
75+
// to metav1.Condition is performed, just enough to allow surfacing a condition from a provider object with Mirror
7276
// - Objects with metav1.Condition in status.v1beta2.conditions
7377
// - Objects with metav1.Condition in status.conditions
74-
//
75-
// Please note that GetAll also supports reading conditions from unstructured objects; in this case, best effort
76-
// conversion from a map is performed, just enough to allow surfacing a condition from a providers object with Mirror
77-
//
78-
// In case the object does not have metav1.Conditions, GetAll tries to read clusterv1.Conditions from status.conditions
79-
// and convert them to metav1.Conditions.
80-
func GetAll(sourceObj runtime.Object) ([]metav1.Condition, error) {
78+
func UnstructuredGet(sourceObj runtime.Unstructured, sourceConditionType string) (*metav1.Condition, error) {
79+
// if obj is nil, the requested condition does not exist.
8180
if sourceObj == nil {
82-
return nil, errors.New("sourceObj cannot be nil")
83-
}
84-
85-
switch sourceObj.(type) {
86-
case runtime.Unstructured:
87-
return getFromUnstructuredObject(sourceObj)
88-
default:
89-
return getFromTypedObject(sourceObj)
90-
}
91-
}
92-
93-
func getFromUnstructuredObject(obj runtime.Object) ([]metav1.Condition, error) {
94-
u, ok := obj.(runtime.Unstructured)
95-
if !ok {
96-
// NOTE: this should not happen due to the type assertion before calling this func
97-
return nil, errors.New("obj cannot be converted to runtime.Unstructured")
81+
return nil, nil
9882
}
9983

100-
if !reflect.ValueOf(u).Elem().IsValid() {
101-
return nil, errors.New("obj cannot be nil")
84+
// if obj is a zero value of its type (a pointer to nil), the requested condition does not exist.
85+
ptr := reflect.ValueOf(sourceObj)
86+
if ptr.Kind() != reflect.Pointer {
87+
// Note: this should never happen, given that Getter is an interface.
88+
return nil, nil
10289
}
10390

104-
ownerInfo := getConditionOwnerInfo(obj)
91+
ownerInfo := getConditionOwnerInfo(sourceObj)
10592

106-
value, exists, err := unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "v1beta2", "conditions")
93+
value, exists, err := unstructured.NestedFieldNoCopy(sourceObj.UnstructuredContent(), "status", "v1beta2", "conditions")
10794
if exists && err == nil {
10895
if conditions, ok := value.([]interface{}); ok {
10996
r, err := convertFromUnstructuredConditions(conditions)
11097
if err != nil {
11198
return nil, errors.Wrapf(err, "failed to convert %s.status.v1beta2.conditions to []metav1.Condition", ownerInfo)
11299
}
113-
return r, nil
100+
return meta.FindStatusCondition(r, sourceConditionType), nil
114101
}
115102
}
116103

117-
value, exists, err = unstructured.NestedFieldNoCopy(u.UnstructuredContent(), "status", "conditions")
104+
value, exists, err = unstructured.NestedFieldNoCopy(sourceObj.UnstructuredContent(), "status", "conditions")
118105
if exists && err == nil {
119106
if conditions, ok := value.([]interface{}); ok {
120107
r, err := convertFromUnstructuredConditions(conditions)
121108
if err != nil {
122109
return nil, errors.Wrapf(err, "failed to convert %s.status.conditions to []metav1.Condition", ownerInfo)
123110
}
124-
return r, nil
111+
return meta.FindStatusCondition(r, sourceConditionType), nil
125112
}
126113
}
127114

128115
// With unstructured, it is not possible to detect if conditions are not set if the type is wrongly defined.
129-
// We assume condition are not set.
116+
// This methods assume condition are not set.
130117
return nil, nil
131118
}
132119

@@ -194,93 +181,6 @@ func convertFromUnstructuredConditions(conditions []interface{}) ([]metav1.Condi
194181
return convertedConditions, nil
195182
}
196183

197-
func getFromTypedObject(obj runtime.Object) ([]metav1.Condition, error) {
198-
ptr := reflect.ValueOf(obj)
199-
if ptr.Kind() != reflect.Pointer {
200-
return nil, errors.New("obj must be a pointer")
201-
}
202-
203-
elem := ptr.Elem()
204-
if !elem.IsValid() {
205-
return nil, errors.New("obj must be a valid value (non zero value of its type)")
206-
}
207-
208-
ownerInfo := getConditionOwnerInfo(obj)
209-
210-
statusField := elem.FieldByName("Status")
211-
if statusField == (reflect.Value{}) {
212-
return nil, errors.Errorf("%s must have a status field", ownerInfo)
213-
}
214-
215-
// Get conditions.
216-
// NOTE: Given that we allow providers to migrate at different speed, it is required to support objects at the different stage of the transition from legacy conditions to metav1.Condition.
217-
// In order to handle this, first try to read Status.V1Beta2.Conditions, then Status.Conditions; for Status.Conditions, also support conversion from legacy conditions.
218-
// The V1Beta2 branch and the conversion from legacy conditions should be dropped when the v1beta1 API is removed.
219-
220-
if v1beta2Field := statusField.FieldByName("V1Beta2"); v1beta2Field != (reflect.Value{}) {
221-
if v1beta2Field.Kind() != reflect.Pointer {
222-
return nil, errors.Errorf("%s.status.v1beta2 must be a pointer", ownerInfo)
223-
}
224-
225-
v1beta2Elem := v1beta2Field.Elem()
226-
if !v1beta2Elem.IsValid() {
227-
// If the field is a zero value of its type, condition list is nil.
228-
return nil, nil
229-
}
230-
231-
if conditionField := v1beta2Elem.FieldByName("Conditions"); conditionField != (reflect.Value{}) {
232-
v1beta2Conditions, ok := conditionField.Interface().([]metav1.Condition)
233-
if !ok {
234-
return nil, errors.Errorf("%s.status.v1beta2.conditions must be of type []metav1.Condition", ownerInfo)
235-
}
236-
return v1beta2Conditions, nil
237-
}
238-
}
239-
240-
if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) {
241-
conditions, ok := conditionField.Interface().([]metav1.Condition)
242-
if ok {
243-
return conditions, nil
244-
}
245-
246-
v1betaConditions, ok := conditionField.Interface().(clusterv1.Conditions)
247-
if !ok {
248-
return nil, errors.Errorf("%s.status.conditions must be of type []metav1.Condition or clusterv1.Conditions", ownerInfo)
249-
}
250-
r, err := convertFromV1Beta1Conditions(v1betaConditions)
251-
if err != nil {
252-
return nil, errors.Wrapf(err, "failed to convert %s.status.conditions to []metav1.Condition", ownerInfo)
253-
}
254-
return r, nil
255-
}
256-
257-
return nil, errors.Errorf("%s.status must have one of conditions or v1beta2.conditions", ownerInfo)
258-
}
259-
260-
// convertFromV1Beta1Conditions converts a clusterv1.Conditions to []metav1.Condition.
261-
// NOTE: this operation is performed at best effort and assuming conditions have been set using Cluster API condition utils.
262-
func convertFromV1Beta1Conditions(v1betaConditions clusterv1.Conditions) ([]metav1.Condition, error) {
263-
if v1betaConditions == nil {
264-
return nil, nil
265-
}
266-
267-
convertedConditions := make([]metav1.Condition, len(v1betaConditions))
268-
for i, c := range v1betaConditions {
269-
convertedConditions[i] = metav1.Condition{
270-
Type: string(c.Type),
271-
Status: metav1.ConditionStatus(c.Status),
272-
LastTransitionTime: c.LastTransitionTime,
273-
Reason: c.Reason,
274-
Message: c.Message,
275-
}
276-
277-
if err := validateAndFixConvertedCondition(&convertedConditions[i]); err != nil {
278-
return nil, err
279-
}
280-
}
281-
return convertedConditions, nil
282-
}
283-
284184
// validateAndFixConvertedCondition validates and fixes a clusterv1.Condition converted to a metav1.Condition.
285185
// this operation assumes conditions have been set using Cluster API condition utils;
286186
// also, only a few, minimal rules are enforced, just enough to allow surfacing a condition from a providers object with Mirror.

0 commit comments

Comments
 (0)