diff --git a/util/conditions/v1beta2/patch.go b/util/conditions/v1beta2/patch.go new file mode 100644 index 000000000000..85a21c919ec4 --- /dev/null +++ b/util/conditions/v1beta2/patch.go @@ -0,0 +1,248 @@ +/* +Copyright 2024 The Kubernetes 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 v1beta2 + +import ( + "reflect" + + "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + "sigs.k8s.io/cluster-api/util" +) + +// Patch defines a list of operations to change a list of conditions into another. +type Patch []PatchOperation + +// PatchOperation define an operation that changes a single condition. +type PatchOperation struct { + Before *metav1.Condition + After *metav1.Condition + Op PatchOperationType +} + +// PatchOperationType defines patch operation types. +type PatchOperationType string + +const ( + // AddConditionPatch defines an add condition patch operation. + AddConditionPatch PatchOperationType = "Add" + + // ChangeConditionPatch defines an change condition patch operation. + ChangeConditionPatch PatchOperationType = "Change" + + // RemoveConditionPatch defines a remove condition patch operation. + RemoveConditionPatch PatchOperationType = "Remove" +) + +// NewPatch returns the Patch required to align source conditions to after conditions. +func NewPatch(before, after runtime.Object) (Patch, error) { + var patch Patch + + if util.IsNil(before) { + return nil, errors.New("error creating patch: before object is nil") + } + beforeConditions, err := GetAll(before) + if err != nil { + return nil, errors.Wrap(err, "error creating patch: failed to get conditions from before object") + } + + if util.IsNil(after) { + return nil, errors.New("error creating patch: after object is nil") + } + afterConditions, err := GetAll(after) + if err != nil { + return nil, errors.Wrap(err, "error creating patch: failed to get conditions from after object") + } + + // Identify AddCondition and ModifyCondition changes. + for i := range afterConditions { + afterCondition := afterConditions[i] + beforeCondition := meta.FindStatusCondition(beforeConditions, afterCondition.Type) + if beforeCondition == nil { + patch = append(patch, PatchOperation{Op: AddConditionPatch, After: &afterCondition}) + continue + } + + if !reflect.DeepEqual(&afterCondition, beforeCondition) { + patch = append(patch, PatchOperation{Op: ChangeConditionPatch, After: &afterCondition, Before: beforeCondition}) + } + } + + // Identify RemoveCondition changes. + for i := range beforeConditions { + beforeCondition := beforeConditions[i] + afterCondition := meta.FindStatusCondition(afterConditions, beforeCondition.Type) + if afterCondition == nil { + patch = append(patch, PatchOperation{Op: RemoveConditionPatch, Before: &beforeCondition}) + } + } + return patch, nil +} + +// applyOptions allows to set strategies for patch apply. +type applyOptions struct { + ownedConditions []string + forceOverwrite bool +} + +func (o *applyOptions) isOwnedCondition(t string) bool { + for _, i := range o.ownedConditions { + if i == t { + return true + } + } + return false +} + +// ApplyOption defines an option for applying a condition patch. +type ApplyOption func(*applyOptions) + +// WithOwnedConditions allows to define condition types owned by the controller. +// In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. +func WithOwnedConditions(t ...string) ApplyOption { + return func(c *applyOptions) { + c.ownedConditions = t + } +} + +// WithForceOverwrite In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. +func WithForceOverwrite(v bool) ApplyOption { + return func(c *applyOptions) { + c.forceOverwrite = v + } +} + +// Apply executes a three-way merge of a list of Patch. +// When merge conflicts are detected (latest deviated from before in an incompatible way), an error is returned. +func (p Patch) Apply(latest runtime.Object, options ...ApplyOption) error { + if p.IsZero() { + return nil + } + + if util.IsNil(latest) { + return errors.New("error patching conditions: latest object is nil") + } + latestConditions, err := GetAll(latest) + if err != nil { + return errors.Wrap(err, "error creating patch: failed to get conditions from latest object") + } + + applyOpt := &applyOptions{} + for _, o := range options { + if util.IsNil(o) { + return errors.New("error patching conditions: ApplyOption is nil") + } + o(applyOpt) + } + + for _, conditionPatch := range p { + switch conditionPatch.Op { + case AddConditionPatch: + // If the conditions is owned, always keep the after value. + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) { + meta.SetStatusCondition(&latestConditions, *conditionPatch.After) + continue + } + + // If the condition is already on latest, check if latest and after agree on the change; if not, this is a conflict. + if latestCondition := meta.FindStatusCondition(latestConditions, conditionPatch.After.Type); latestCondition != nil { + // If latest and after agree on the change, then it is a conflict. + if !hasSameState(latestCondition, conditionPatch.After) { + return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After)) + } + // otherwise, the latest is already as intended. + // NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value. + continue + } + // If the condition does not exists on the latest, add the new after condition. + meta.SetStatusCondition(&latestConditions, *conditionPatch.After) + + case ChangeConditionPatch: + // If the conditions is owned, always keep the after value. + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.After.Type) { + meta.SetStatusCondition(&latestConditions, *conditionPatch.After) + continue + } + + latestCondition := meta.FindStatusCondition(latestConditions, conditionPatch.After.Type) + + // If the condition does not exist anymore on the latest, this is a conflict. + if latestCondition == nil { + return errors.Errorf("error patching conditions: The condition %q was deleted by a different process and this caused a merge/ChangeCondition conflict", conditionPatch.After.Type) + } + + // If the condition on the latest is different from the base condition, check if + // the after state corresponds to the desired value. If not this is a conflict (unless we should ignore conflicts for this condition type). + if !reflect.DeepEqual(latestCondition, conditionPatch.Before) { + if !hasSameState(latestCondition, conditionPatch.After) { + return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/ChangeCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After)) + } + // Otherwise the latest is already as intended. + // NOTE: We are preserving LastTransitionTime from the latest in order to avoid altering the existing value. + continue + } + // Otherwise apply the new after condition. + meta.SetStatusCondition(&latestConditions, *conditionPatch.After) + + case RemoveConditionPatch: + // If latestConditions is nil or empty, nothing to remove. + if len(latestConditions) == 0 { + continue + } + + // If the conditions is owned, always keep the after value (condition should be deleted). + if applyOpt.forceOverwrite || applyOpt.isOwnedCondition(conditionPatch.Before.Type) { + meta.RemoveStatusCondition(&latestConditions, conditionPatch.Before.Type) + continue + } + + // If the condition is still on the latest, check if it is changed in the meantime; + // if so then this is a conflict. + if latestCondition := meta.FindStatusCondition(latestConditions, conditionPatch.Before.Type); latestCondition != nil { + if !hasSameState(latestCondition, conditionPatch.Before) { + return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/RemoveCondition conflict: %v", conditionPatch.Before.Type, cmp.Diff(latestCondition, conditionPatch.Before)) + } + } + // Otherwise the latest and after agreed on the delete operation, so there's nothing to change. + meta.RemoveStatusCondition(&latestConditions, conditionPatch.Before.Type) + } + } + + return SetAll(latest, latestConditions) +} + +// IsZero returns true if the patch is nil or has no changes. +func (p Patch) IsZero() bool { + if p == nil { + return true + } + return len(p) == 0 +} + +// hasSameState returns true if a condition has the same state of another; state is defined +// by the union of following fields: Type, Status, Reason, ObservedGeneration and Message (it excludes LastTransitionTime). +func hasSameState(i, j *metav1.Condition) bool { + return i.Type == j.Type && + i.Status == j.Status && + i.ObservedGeneration == j.ObservedGeneration && + i.Reason == j.Reason && + i.Message == j.Message +} diff --git a/util/conditions/v1beta2/patch_test.go b/util/conditions/v1beta2/patch_test.go new file mode 100644 index 000000000000..23b9efb70513 --- /dev/null +++ b/util/conditions/v1beta2/patch_test.go @@ -0,0 +1,362 @@ +/* +Copyright 2024 The Kubernetes 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 v1beta2 + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + "sigs.k8s.io/cluster-api/internal/test/builder" +) + +func TestNewPatch(t *testing.T) { + fooTrue := metav1.Condition{Type: "foo", Status: metav1.ConditionTrue} + fooFalse := metav1.Condition{Type: "foo", Status: metav1.ConditionFalse} + + tests := []struct { + name string + before runtime.Object + after runtime.Object + want Patch + wantErr bool + }{ + { + name: "nil before return error", + before: nil, + after: objectWithConditions(), + wantErr: true, + }, + { + name: "nil after return error", + before: objectWithConditions(), + after: nil, + wantErr: true, + }, + { + name: "nil Interface before return error", + before: nilObject(), + after: objectWithConditions(), + wantErr: true, + }, + { + name: "nil Interface after return error", + before: objectWithConditions(), + after: nilObject(), + wantErr: true, + }, + { + name: "No changes return empty patch", + before: objectWithConditions(), + after: objectWithConditions(), + want: nil, + wantErr: false, + }, + + { + name: "No changes return empty patch", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooTrue), + want: nil, + }, + { + name: "Detects AddConditionPatch", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + want: Patch{ + { + Before: nil, + After: &fooTrue, + Op: AddConditionPatch, + }, + }, + }, + { + name: "Detects ChangeConditionPatch", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + want: Patch{ + { + Before: &fooTrue, + After: &fooFalse, + Op: ChangeConditionPatch, + }, + }, + }, + { + name: "Detects RemoveConditionPatch", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + want: Patch{ + { + Before: &fooTrue, + After: nil, + Op: RemoveConditionPatch, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := NewPatch(tt.before, tt.after) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).To(Not(HaveOccurred())) + g.Expect(got).To(BeComparableTo(tt.want)) + }) + } +} + +func TestApply(t *testing.T) { + fooTrue := metav1.Condition{Type: "foo", Status: metav1.ConditionTrue} + fooFalse := metav1.Condition{Type: "foo", Status: metav1.ConditionFalse} + fooFalse2 := metav1.Condition{Type: "foo", Status: metav1.ConditionFalse, Reason: "Something else"} + + tests := []struct { + name string + before runtime.Object + after runtime.Object + latest runtime.Object + options []ApplyOption + want []metav1.Condition + wantErr bool + }{ + { + name: "error with nil interface object", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: nilObject(), + want: []metav1.Condition{fooTrue}, + wantErr: true, + }, + { + name: "error with nil latest object", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: nil, + want: []metav1.Condition{fooTrue}, + wantErr: true, + }, + { + name: "No patch return same list", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooTrue), + want: []metav1.Condition{fooTrue}, + wantErr: false, + }, + { + name: "Add: When a condition does not exists, it should add", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(), + want: []metav1.Condition{fooTrue}, + wantErr: false, + }, + { + name: "Add: When a condition already exists but without conflicts, it should add", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooTrue), + want: []metav1.Condition{fooTrue}, + wantErr: false, + }, + { + name: "Add: When a condition already exists but with conflicts, it should error", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooFalse), + want: nil, + wantErr: true, + }, + { + name: "Add: When a condition already exists but with conflicts, it should not error if force override is set", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooFalse), + options: []ApplyOption{WithForceOverwrite(true)}, + want: []metav1.Condition{fooTrue}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Add: When a condition already exists but with conflicts, it should not error if the condition is owned", + before: objectWithConditions(), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooFalse), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: []metav1.Condition{fooTrue}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Remove: When a condition was already deleted, it should pass", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + latest: objectWithConditions(), + want: []metav1.Condition{}, + wantErr: false, + }, + { + name: "Remove: When a condition already exists but without conflicts, it should delete", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + latest: objectWithConditions(fooTrue), + want: []metav1.Condition{}, + wantErr: false, + }, + { + name: "Remove: When a condition already exists but with conflicts, it should error", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + latest: objectWithConditions(fooFalse), + want: nil, + wantErr: true, + }, + { + name: "Remove: When a condition already exists but with conflicts, it should not error if force override is set", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + latest: objectWithConditions(fooFalse), + options: []ApplyOption{WithForceOverwrite(true)}, + want: []metav1.Condition{}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Remove: When a condition already exists but with conflicts, it should not error if the condition is owned", + before: objectWithConditions(fooTrue), + after: objectWithConditions(), + latest: objectWithConditions(fooFalse), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: []metav1.Condition{}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Change: When a condition exists without conflicts, it should change", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: objectWithConditions(fooTrue), + want: []metav1.Condition{fooFalse}, + wantErr: false, + }, + { + name: "Change: When a condition exists with conflicts but there is agreement on the final state, it should change", + before: objectWithConditions(fooFalse), + after: objectWithConditions(fooTrue), + latest: objectWithConditions(fooTrue), + want: []metav1.Condition{fooTrue}, + wantErr: false, + }, + { + name: "Change: When a condition exists with conflicts but there is no agreement on the final state, it should error", + before: objectWithConditions(fooFalse), + after: objectWithConditions(fooFalse2), + latest: objectWithConditions(fooTrue), + want: nil, + wantErr: true, + }, + { + name: "Change: When a condition exists with conflicts but there is no agreement on the final state, it should not error if force override is set", + before: objectWithConditions(fooFalse), + after: objectWithConditions(fooFalse2), + latest: objectWithConditions(fooTrue), + options: []ApplyOption{WithForceOverwrite(true)}, + want: []metav1.Condition{fooFalse2}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Change: When a condition exists with conflicts but there is no agreement on the final state, it should not error if the condition is owned", + before: objectWithConditions(fooFalse), + after: objectWithConditions(fooFalse2), + latest: objectWithConditions(fooTrue), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: []metav1.Condition{fooFalse2}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Change: When a condition was deleted, it should error", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: objectWithConditions(), + want: nil, + wantErr: true, + }, + { + name: "Change: When a condition was deleted, it should not error if force override is set", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: objectWithConditions(), + options: []ApplyOption{WithForceOverwrite(true)}, + want: []metav1.Condition{fooFalse}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Change: When a condition was deleted, it should not error if the condition is owned", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: objectWithConditions(), + options: []ApplyOption{WithOwnedConditions("foo")}, + want: []metav1.Condition{fooFalse}, // after condition should be kept in case of error + wantErr: false, + }, + { + name: "Error when nil passed as an ApplyOption", + before: objectWithConditions(fooTrue), + after: objectWithConditions(fooFalse), + latest: objectWithConditions(), + options: []ApplyOption{nil}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + // Ignore the error here to allow testing of patch.Apply with a nil patch + patch, _ := NewPatch(tt.before, tt.after) + + err := patch.Apply(tt.latest, tt.options...) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + + gotConditions, err := GetAll(tt.latest) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(gotConditions).To(MatchConditions(tt.want, IgnoreLastTransitionTime(true))) + }) + } +} + +func objectWithConditions(conditions ...metav1.Condition) runtime.Object { + obj := &builder.Phase3Obj{} + obj.Status.Conditions = conditions + return obj +} + +func nilObject() runtime.Object { + var obj runtime.Object + return obj +} diff --git a/util/patch/options.go b/util/patch/options.go index 9c2adffbdce3..7086bb6404ef 100644 --- a/util/patch/options.go +++ b/util/patch/options.go @@ -37,6 +37,10 @@ type HelperOptions struct { // OwnedConditions defines condition types owned by the controller. // In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. OwnedConditions []clusterv1.ConditionType + + // OwnedV1Beta1Conditions defines condition types owned by the controller. + // In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. + OwnedV1Beta1Conditions []string } // WithForceOverwriteConditions allows the patch helper to overwrite conditions in case of conflicts. @@ -67,3 +71,14 @@ type WithOwnedConditions struct { func (w WithOwnedConditions) ApplyToHelper(in *HelperOptions) { in.OwnedConditions = w.Conditions } + +// WithOwnedV1Beta1Conditions allows to define condition types owned by the controller. +// In case of conflicts for the owned conditions, the patch helper will always use the value provided by the controller. +type WithOwnedV1Beta1Conditions struct { + Conditions []string +} + +// ApplyToHelper applies this configuration to the given HelperOptions. +func (w WithOwnedV1Beta1Conditions) ApplyToHelper(in *HelperOptions) { + in.OwnedV1Beta1Conditions = w.Conditions +} diff --git a/util/patch/patch.go b/util/patch/patch.go index 7a4fefc12d6e..2d9a061f8965 100644 --- a/util/patch/patch.go +++ b/util/patch/patch.go @@ -36,6 +36,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" + v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2" ) // Helper is a utility for ensuring the proper patching of objects. @@ -47,7 +48,8 @@ type Helper struct { after *unstructured.Unstructured changes sets.Set[string] - isConditionsSetter bool + metav1ConditionsFields []string + clusterv1ConditionsFields []string } // NewHelper returns an initialized Helper. Use NewHelper before changing @@ -65,21 +67,32 @@ func NewHelper(obj client.Object, crClient client.Client) (*Helper, error) { return nil, errors.Wrapf(err, "failed to create patch helper for object %s", klog.KObj(obj)) } + metav1ConditionsFields, clusterv1ConditionsFields, err := identifySupportedConditions(obj) + if err != nil { + return nil, errors.Wrapf(err, "failed to identify condition fields for object %s", klog.KObj(obj)) + } + + // Check if the object satisfies the Cluster API conditions contract; if not, ignore condition fields for clusterv1.Conditions. + if _, canInterfaceConditions := obj.(conditions.Setter); !canInterfaceConditions { + clusterv1ConditionsFields = nil + } + if _, canInterfaceConditions := obj.(conditions.Getter); !canInterfaceConditions { + clusterv1ConditionsFields = nil + } + // Convert the object to unstructured to compare against our before copy. unstructuredObj, err := toUnstructured(obj, gvk) if err != nil { return nil, errors.Wrapf(err, "failed to create patch helper for %s %s: failed to convert object to Unstructured", gvk.Kind, klog.KObj(obj)) } - // Check if the object satisfies the Cluster API conditions contract. - _, canInterfaceConditions := obj.(conditions.Setter) - return &Helper{ - client: crClient, - gvk: gvk, - before: unstructuredObj, - beforeObject: obj.DeepCopyObject().(client.Object), - isConditionsSetter: canInterfaceConditions, + client: crClient, + gvk: gvk, + before: unstructuredObj, + beforeObject: obj.DeepCopyObject().(client.Object), + metav1ConditionsFields: metav1ConditionsFields, + clusterv1ConditionsFields: clusterv1ConditionsFields, }, nil } @@ -139,7 +152,7 @@ func (h *Helper) Patch(ctx context.Context, obj client.Object, opts ...Option) e // Given that we pass in metadata.resourceVersion to perform a 3-way-merge conflict resolution, // patching conditions first avoids an extra loop if spec or status patch succeeds first // given that causes the resourceVersion to mutate. - if err := h.patchStatusConditions(ctx, obj, options.ForceOverwriteConditions, options.OwnedConditions); err != nil { + if err := h.patchStatusConditions(ctx, obj, options.ForceOverwriteConditions, options.OwnedConditions, options.OwnedV1Beta1Conditions); err != nil { errs = append(errs, err) } // Then proceed to patch the rest of the object. @@ -192,39 +205,73 @@ func (h *Helper) patchStatus(ctx context.Context, obj client.Object) error { // // Condition changes are then applied to the latest version of the object, and if there are // no unresolvable conflicts, the patch is sent again. -func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, forceOverwrite bool, ownedConditions []clusterv1.ConditionType) error { - // Nothing to do if the object isn't a condition patcher. - if !h.isConditionsSetter { +func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, forceOverwrite bool, ownedConditions []clusterv1.ConditionType, ownedV1beta2Conditions []string) error { + // Nothing to do if the object doesn't have conditions. + if len(h.clusterv1ConditionsFields) == 0 && len(h.metav1ConditionsFields) == 0 { return nil } - // Make sure our before/after objects satisfy the proper interface before continuing. - // - // NOTE: The checks and error below are done so that we don't panic if any of the objects don't satisfy the - // interface any longer, although this shouldn't happen because we already check when creating the patcher. - before, ok := h.beforeObject.(conditions.Getter) - if !ok { - return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", before.GetObjectKind()) - } - after, ok := obj.(conditions.Getter) - if !ok { - return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", after.GetObjectKind()) - } - - // Store the diff from the before/after object, and return early if there are no changes. - diff, err := conditions.NewPatch( - before, - after, - ) - if err != nil { - return errors.Wrapf(err, "object can not be patched") + // If the object has clusterv1 conditions, create a function applying corresponding changes if any. + var clusterv1ApplyPatch func(client.Object) error + if len(h.clusterv1ConditionsFields) > 0 { + // Make sure our before/after objects satisfy the proper interface before continuing. + // + // NOTE: The checks and error below are done so that we don't panic if any of the objects don't satisfy the + // interface any longer, although this shouldn't happen because we already check when creating the patcher. + before, ok := h.beforeObject.(conditions.Getter) + if !ok { + return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", before.GetObjectKind()) + } + after, ok := obj.(conditions.Getter) + if !ok { + return errors.Errorf("object %s doesn't satisfy conditions.Getter, cannot patch", after.GetObjectKind()) + } + + // Store the diff from the before/after object, and return early if there are no changes. + diff, err := conditions.NewPatch( + before, + after, + ) + if err != nil { + return errors.Wrapf(err, "object can not be patched") + } + if !diff.IsZero() { + clusterv1ApplyPatch = func(latest client.Object) error { + latestSetter, ok := latest.(conditions.Setter) + if !ok { + return errors.Errorf("object %s doesn't satisfy conditions.Setter, cannot patch", before.GetObjectKind()) + } + + return diff.Apply(latestSetter, conditions.WithForceOverwrite(forceOverwrite), conditions.WithOwnedConditions(ownedConditions...)) + } + } + } + + // If the object has metav1 conditions, create a function applying corresponding changes if any. + var metav1ApplyPatch func(client.Object) error + if len(h.metav1ConditionsFields) > 0 { + // Store the diff from the before/after object, and return early if there are no changes. + diff, err := v1beta2conditions.NewPatch( + h.beforeObject, + obj, + ) + if err != nil { + return errors.Wrapf(err, "object can not be patched") + } + + if !diff.IsZero() { + metav1ApplyPatch = func(latest client.Object) error { + return diff.Apply(latest, v1beta2conditions.WithForceOverwrite(forceOverwrite), v1beta2conditions.WithOwnedConditions(ownedV1beta2Conditions...)) + } + } } - if diff.IsZero() { + + if clusterv1ApplyPatch == nil && metav1ApplyPatch == nil { return nil } // Make a copy of the object and store the key used if we have conflicts. - key := client.ObjectKeyFromObject(after) + key := client.ObjectKeyFromObject(obj) // Define and start a backoff loop to handle conflicts // between controllers working on the same object. @@ -238,7 +285,7 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f // Start the backoff loop and return errors if any. return wait.ExponentialBackoff(backoff, func() (bool, error) { - latest, ok := before.DeepCopyObject().(conditions.Setter) + latest, ok := h.beforeObject.DeepCopyObject().(client.Object) if !ok { return false, errors.Errorf("object %s doesn't satisfy conditions.Setter, cannot patch", latest.GetObjectKind()) } @@ -249,11 +296,18 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f } // Create the condition patch before merging conditions. - conditionsPatch := client.MergeFromWithOptions(latest.DeepCopyObject().(conditions.Setter), client.MergeFromWithOptimisticLock{}) + conditionsPatch := client.MergeFromWithOptions(latest.DeepCopyObject().(client.Object), client.MergeFromWithOptimisticLock{}) // Set the condition patch previously created on the new object. - if err := diff.Apply(latest, conditions.WithForceOverwrite(forceOverwrite), conditions.WithOwnedConditions(ownedConditions...)); err != nil { - return false, err + if clusterv1ApplyPatch != nil { + if err := clusterv1ApplyPatch(latest); err != nil { + return false, err + } + } + if metav1ApplyPatch != nil { + if err := metav1ApplyPatch(latest); err != nil { + return false, err + } } // Issue the patch. @@ -273,8 +327,8 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f // calculatePatch returns the before/after objects to be given in a controller-runtime patch, scoped down to the absolute necessary. func (h *Helper) calculatePatch(afterObj client.Object, focus patchType) (client.Object, client.Object, error) { // Get a shallow unsafe copy of the before/after object in unstructured form. - before := unsafeUnstructuredCopy(h.before, focus, h.isConditionsSetter) - after := unsafeUnstructuredCopy(h.after, focus, h.isConditionsSetter) + before := unsafeUnstructuredCopy(h.before, focus, h.metav1ConditionsFields, h.clusterv1ConditionsFields) + after := unsafeUnstructuredCopy(h.after, focus, h.metav1ConditionsFields, h.clusterv1ConditionsFields) // We've now applied all modifications to local unstructured objects, // make copies of the original objects and convert them back. diff --git a/util/patch/patch_test.go b/util/patch/patch_test.go index 81a326b20eb2..bc13b71e329b 100644 --- a/util/patch/patch_test.go +++ b/util/patch/patch_test.go @@ -31,8 +31,10 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/cluster-api/internal/test/builder" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" + v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2" ) func TestPatchHelper(t *testing.T) { @@ -997,3 +999,1683 @@ func TestNewHelperNil(t *testing.T) { _, err = NewHelper(nil, nil) g.Expect(err).To(HaveOccurred()) } + +func TestPatchHelperForV1beta2Transition(t *testing.T) { + t.Run("Should patch conditions on a v1beta1 object with conditions (phase 0)", func(t *testing.T) { + obj := &builder.Phase0Obj{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: metav1.NamespaceDefault, + }, + } + + t.Run("should mark it ready", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + err := env.Create(ctx, obj) + g.Expect(err).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() clusterv1.Conditions { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Conditions{} + } + return objAfter.Status.Conditions + }, timeout).Should(conditions.MatchConditions(obj.Status.Conditions)) + }) + + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a custom condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("TestCondition"), "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy := conditions.Get(objCopy, "TestCondition") + testConditionAfter := conditions.Get(objAfter, "TestCondition") + ok, err := conditions.MatchCondition(*testConditionCopy).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should recover if there is a resolvable conflict, incl. patch spec and status", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a custom condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("TestCondition"), "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Changing the object spec, status, and adding Ready=True condition") + obj.Spec.Foo = "foo" + obj.Status.Bar = "bat" + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + objAfter := obj.DeepCopy() + g.Eventually(func() bool { + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy := conditions.Get(objCopy, "TestCondition") + testConditionAfter := conditions.Get(objAfter, "TestCondition") + ok, err := conditions.MatchCondition(*testConditionCopy).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return obj.Spec.Foo == objAfter.Spec.Foo && + obj.Status.Bar == objAfter.Status.Bar + }, timeout).Should(BeTrue(), cmp.Diff(obj, objAfter)) + }) + + t.Run("should return an error if there is an unresolvable conflict", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a custom condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + for _, afterCondition := range objAfter.Status.Conditions { + ok, err := conditions.MatchCondition(objCopy.Status.Conditions[0]).Match(afterCondition) + if err == nil && ok { + return true + } + } + + return false + }, timeout).Should(BeTrue()) + }) + + t.Run("should not return an error if there is an unresolvable conflict but the conditions is owned by the controller", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a custom condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithOwnedConditions{Conditions: []clusterv1.ConditionType{clusterv1.ReadyCondition}})).To(Succeed()) + + t.Log("Validating the object has been updated") + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + g.Eventually(func() clusterv1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Condition{} + } + + return *conditions.Get(objAfter, clusterv1.ReadyCondition) + }, timeout).Should(conditions.MatchCondition(*readyBefore)) + }) + + t.Run("should not return an error if there is an unresolvable conflict when force overwrite is enabled", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a custom condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithForceOverwriteConditions{})).To(Succeed()) + + t.Log("Validating the object has been updated") + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + g.Eventually(func() clusterv1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Condition{} + } + + return *conditions.Get(objAfter, clusterv1.ReadyCondition) + }, timeout).Should(conditions.MatchCondition(*readyBefore)) + }) + }) + + t.Run("Should patch conditions on a v1beta1 object with both clusterv1.conditions and metav1.conditions (phase 1)", func(t *testing.T) { + obj := &builder.Phase1Obj{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: metav1.NamespaceDefault, + }, + } + + t.Run("should mark it ready", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + err := env.Create(ctx, obj) + g.Expect(err).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking clusterv1.conditions and metav1.conditions Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() clusterv1.Conditions { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Conditions{} + } + return objAfter.Status.Conditions + }, timeout).Should(conditions.MatchConditions(obj.Status.Conditions)) + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.V1Beta2.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.V1Beta2.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking clusterv1.conditions and metav1.conditions Test=False") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("Test"), "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking clusterv1.conditions and metav1.conditions Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy := conditions.Get(objCopy, "Test") + testConditionAfter := conditions.Get(objAfter, "Test") + ok, err := conditions.MatchCondition(*testConditionCopy).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + testV1Beta2ConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testV1Beta2ConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*testV1Beta2ConditionCopy).Match(*testV1Beta2ConditionAfter) + if err != nil || !ok { + return false + } + + readyV1Beta2Before, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyV1Beta2After, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyV1Beta2Before, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyV1Beta2After) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should recover if there is a resolvable conflict, incl. patch spec and status", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking clusterv1.conditions and metav1.conditions Test=False") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("Test"), "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Changing the object spec, status, and marking clusterv1.condition and metav1.conditions Ready=True") + obj.Spec.Foo = "foo" + obj.Status.Bar = "bat" + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + objAfter := obj.DeepCopy() + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy := conditions.Get(objCopy, "Test") + testConditionAfter := conditions.Get(objAfter, "Test") + ok, err := conditions.MatchCondition(*testConditionCopy).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + testV1Beta2ConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testV1Beta2ConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*testV1Beta2ConditionCopy, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*testV1Beta2ConditionAfter) + if err != nil || !ok { + return false + } + + readyV1Beta2Before, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyV1Beta2After, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyV1Beta2Before, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyV1Beta2After) + if err != nil || !ok { + return false + } + + return obj.Spec.Foo == objAfter.Spec.Foo && + obj.Status.Bar == objAfter.Status.Bar + }, timeout).Should(BeTrue(), cmp.Diff(obj, objAfter)) + }) + + t.Run("should return an error if there is an unresolvable conflict on conditions", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() clusterv1.Conditions { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Conditions{} + } + return objAfter.Status.Conditions + }, timeout).Should(conditions.MatchConditions(objCopy.Status.Conditions)) + }) + + t.Run("should return an error if there is an unresolvable conflict on v1beta2.conditions", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + err := v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition Ready=True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.V1Beta2.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(objCopy.Status.V1Beta2.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should not return an error if there is an unresolvable conflict but the conditions is owned by the controller", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready clusterv1.condition and metav1.conditions to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready clusterv1.condition and metav1.conditions True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithOwnedConditions{Conditions: []clusterv1.ConditionType{clusterv1.ReadyCondition}}, WithOwnedV1Beta1Conditions{Conditions: []string{"Ready"}})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err := conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + readyV1Beta2Before, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyV1Beta2After, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyV1Beta2Before, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyV1Beta2After) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should not return an error if there is an unresolvable conflict when force overwrite is enabled", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready clusterv1.condition and metav1.conditions to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready clusterv1.condition and metav1.conditions True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithForceOverwriteConditions{})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err := conditions.MatchCondition(*readyBefore).Match(*readyAfter) + if err != nil || !ok { + return false + } + + readyV1Beta2Before, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyV1Beta2After, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyV1Beta2Before, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyV1Beta2After) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + }) + + t.Run("Should patch conditions on a v1beta2 object with both conditions and backward compatible conditions (phase 2)", func(t *testing.T) { + obj := &builder.Phase2Obj{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: metav1.NamespaceDefault, + }, + } + + t.Run("should mark it ready", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + err := env.Create(ctx, obj) + g.Expect(err).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition and back compatibility condition Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() clusterv1.Conditions { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Conditions{} + } + return objAfter.Status.Deprecated.V1Beta1.Conditions + }, timeout).Should(conditions.MatchConditions(obj.Status.Deprecated.V1Beta1.Conditions)) + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking condition and back compatibility condition Test=False") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("Test"), "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition and back compatibility condition Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testBackCompatibilityCopy := conditions.Get(objCopy, "Test") + testBackCompatibilityAfter := conditions.Get(objAfter, "Test") + ok, err := conditions.MatchCondition(*testBackCompatibilityCopy).Match(*testBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + readyBackCompatibilityBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyBackCompatibilityAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBackCompatibilityBefore).Match(*readyBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + testConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*testConditionCopy, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should recover if there is a resolvable conflict, incl. patch spec and status", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking condition and back compatibility condition Test=False") + conditions.MarkFalse(objCopy, clusterv1.ConditionType("Test"), "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Changing the object spec, status, and marking condition and back compatibility condition Ready=True") + obj.Spec.Foo = "foo" + obj.Status.Bar = "bat" + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + objAfter := obj.DeepCopy() + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testBackCompatibilityCopy := conditions.Get(objCopy, "Test") + testBackCompatibilityAfter := conditions.Get(objAfter, "Test") + ok, err := conditions.MatchCondition(*testBackCompatibilityCopy).Match(*testBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + readyBackCompatibilityBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyBackCompatibilityAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err = conditions.MatchCondition(*readyBackCompatibilityBefore).Match(*readyBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + testConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*testConditionCopy, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return obj.Spec.Foo == objAfter.Spec.Foo && + obj.Status.Bar == objAfter.Status.Bar + }, timeout).Should(BeTrue(), cmp.Diff(obj, objAfter)) + }) + + t.Run("should return an error if there is an unresolvable conflict on back compatibility conditions", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready=True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() clusterv1.Conditions { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return clusterv1.Conditions{} + } + return objAfter.Status.Deprecated.V1Beta1.Conditions + }, timeout).Should(conditions.MatchConditions(objCopy.Status.Deprecated.V1Beta1.Conditions)) + }) + + t.Run("should return an error if there is an unresolvable conflict on conditions", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + err := v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition Ready=True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(objCopy.Status.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should not return an error if there is an unresolvable conflict but the conditions is owned by the controller", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition and back compatibility condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready condition and back compatibility condition True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithOwnedConditions{Conditions: []clusterv1.ConditionType{clusterv1.ReadyCondition}}, WithOwnedV1Beta1Conditions{Conditions: []string{"Ready"}})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBackCompatibilityBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyBackCompatibilityAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err := conditions.MatchCondition(*readyBackCompatibilityBefore).Match(*readyBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should not return an error if there is an unresolvable conflict when force overwrite is enabled", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition and back compatibility condition to be false") + conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready condition and back compatibility condition True") + conditions.MarkTrue(obj, clusterv1.ReadyCondition) + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithForceOverwriteConditions{})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBackCompatibilityBefore := conditions.Get(obj, clusterv1.ReadyCondition) + readyBackCompatibilityAfter := conditions.Get(objAfter, clusterv1.ReadyCondition) + ok, err := conditions.MatchCondition(*readyBackCompatibilityBefore).Match(*readyBackCompatibilityAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + }) + + t.Run("Should patch conditions on a v1beta2 object with conditions (phase 3)", func(t *testing.T) { + obj := &builder.Phase3Obj{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: metav1.NamespaceDefault, + }, + } + + t.Run("should mark it ready", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + err := env.Create(ctx, obj) + g.Expect(err).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition Ready=True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking condition Test=False") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition Ready=True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err := v1beta2conditions.MatchCondition(*testConditionCopy, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should recover if there is a resolvable conflict, incl. patch spec and status", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking condition Test=False") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Test", Status: metav1.ConditionFalse, Reason: "reason", Message: "message"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Changing the object spec, status, and marking condition Ready=True") + obj.Spec.Foo = "foo" + obj.Status.Bar = "bat" + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).To(Succeed()) + + t.Log("Validating the object has been updated") + objAfter := obj.DeepCopy() + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + testConditionCopy, err := v1beta2conditions.Get(objCopy, "Test") + if err != nil { + return false + } + testConditionAfter, err := v1beta2conditions.Get(objAfter, "Test") + if err != nil { + return false + } + ok, err := v1beta2conditions.MatchCondition(*testConditionCopy, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*testConditionAfter) + if err != nil || !ok { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err = v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return obj.Spec.Foo == objAfter.Spec.Foo && + obj.Status.Bar == objAfter.Status.Bar + }, timeout).Should(BeTrue(), cmp.Diff(obj, objAfter)) + }) + + t.Run("should return an error if there is an unresolvable conflict on conditions", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + err := v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking condition Ready=True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj)).NotTo(Succeed()) + + t.Log("Validating the object has not been updated") + g.Eventually(func() []metav1.Condition { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return nil + } + return objAfter.Status.Conditions + }, timeout).Should(v1beta2conditions.MatchConditions(objCopy.Status.Conditions, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + + t.Run("should not return an error if there is an unresolvable conflict but the conditions is owned by the controller", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready condition True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithOwnedConditions{Conditions: []clusterv1.ConditionType{clusterv1.ReadyCondition}}, WithOwnedV1Beta1Conditions{Conditions: []string{"Ready"}})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err := v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + + t.Run("should not return an error if there is an unresolvable conflict when force overwrite is enabled", func(t *testing.T) { + g := NewWithT(t) + + obj := obj.DeepCopy() + + t.Log("Creating the object") + g.Expect(env.Create(ctx, obj)).To(Succeed()) + defer func() { + g.Expect(env.Delete(ctx, obj)).To(Succeed()) + }() + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + t.Log("Checking that the object has been created") + g.Eventually(func() error { + obj := obj.DeepCopy() + return env.Get(ctx, key, obj) + }).Should(Succeed()) + + objCopy := obj.DeepCopy() + + t.Log("Marking a Ready condition to be false") + err := v1beta2conditions.Set(objCopy, metav1.Condition{Type: "Ready", Status: metav1.ConditionFalse, Reason: "NotGood"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed()) + + t.Log("Validating that the local object's resource version is behind") + g.Expect(obj.ResourceVersion).NotTo(Equal(objCopy.ResourceVersion)) + + t.Log("Creating a new patch helper") + patcher, err := NewHelper(obj, env) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Marking Ready condition True") + err = v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"}) + g.Expect(err).ToNot(HaveOccurred()) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, WithForceOverwriteConditions{})).To(Succeed()) + + t.Log("Validating the object has been updated") + g.Eventually(func() bool { + objAfter := obj.DeepCopy() + if err := env.Get(ctx, key, objAfter); err != nil { + return false + } + + readyBefore, err := v1beta2conditions.Get(obj, "Ready") + if err != nil { + return false + } + readyAfter, err := v1beta2conditions.Get(objAfter, "Ready") + if err != nil { + return false + } + ok, err := v1beta2conditions.MatchCondition(*readyBefore, v1beta2conditions.IgnoreLastTransitionTime(true)).Match(*readyAfter) + if err != nil || !ok { + return false + } + + return true + }, timeout).Should(BeTrue()) + }) + }) +} diff --git a/util/patch/utils.go b/util/patch/utils.go index 1a35291c5f49..92046b89628e 100644 --- a/util/patch/utils.go +++ b/util/patch/utils.go @@ -17,9 +17,15 @@ limitations under the License. package patch import ( + "reflect" + + "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) type patchType string @@ -65,7 +71,7 @@ func toUnstructured(obj runtime.Object, gvk schema.GroupVersionKind) (*unstructu // It copies the common fields such as `kind`, `apiVersion`, `metadata` and the patchType specified. // // It's not safe to modify any of the keys in the returned unstructured object, the result should be treated as read-only. -func unsafeUnstructuredCopy(obj *unstructured.Unstructured, focus patchType, isConditionsSetter bool) *unstructured.Unstructured { +func unsafeUnstructuredCopy(obj *unstructured.Unstructured, focus patchType, metav1ConditionsFields, clusterv1ConditionsFields []string) *unstructured.Unstructured { // Create the return focused-unstructured object with a preallocated map. res := &unstructured.Unstructured{Object: make(map[string]interface{}, len(obj.Object))} @@ -92,18 +98,80 @@ func unsafeUnstructuredCopy(obj *unstructured.Unstructured, focus patchType, isC res.Object[key] = value } - // If we've determined that we're able to interface with conditions.Setter interface, - // when dealing with the status patch, remove the status.conditions sub-field from the object. - if isConditionsSetter && focus == statusPatch { - // NOTE: Removing status.conditions changes the incoming object! This is safe because the condition patch - // doesn't use the unstructured fields, and it runs before any other patch. - // - // If we want to be 100% safe, we could make a copy of the incoming object before modifying it, although - // copies have a high cpu and high memory usage, therefore we intentionally choose to avoid extra copies - // given that the ordering of operations and safety is handled internally by the patch helper. - unstructured.RemoveNestedField(res.Object, "status", "conditions") + // If we've determined that new or old condition must be set, + // when dealing with the status patch, remove corresponding sub-fields from the object. + // NOTE: Removing conditions sub-fields changes the incoming object! This is safe because the condition patch + // doesn't use the unstructured fields, and it runs before any other patch. + // + // If we want to be 100% safe, we could make a copy of the incoming object before modifying it, although + // copies have a high cpu and high memory usage, therefore we intentionally choose to avoid extra copies + // given that the ordering of operations and safety is handled internally by the patch helper. + + if len(metav1ConditionsFields) > 0 { + unstructured.RemoveNestedField(res.Object, metav1ConditionsFields...) + } + + if len(clusterv1ConditionsFields) > 0 { + unstructured.RemoveNestedField(res.Object, clusterv1ConditionsFields...) } } return res } + +var ( + clusterv1ConditionsType = reflect.TypeOf(clusterv1.Conditions{}) + metav1ConditionsType = reflect.TypeOf([]metav1.Condition{}) +) + +func identifySupportedConditions(obj runtime.Object) ([]string, []string, error) { + if obj == nil { + return nil, nil, errors.New("cannot identify conditions on a nil object") + } + + ptr := reflect.ValueOf(obj) + if ptr.Kind() != reflect.Pointer { + return nil, nil, errors.New("cannot identify conditions on a object that is not a pointer") + } + + elem := ptr.Elem() + if !elem.IsValid() { + return nil, nil, errors.New("obj must be a valid value (non zero value of its type)") + } + + statusField := elem.FieldByName("Status") + if statusField == (reflect.Value{}) { + return nil, nil, nil + } + + // 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 clusterv1.conditions to metav1.conditions. + // In order to handle this, it is required to identify where conditions are supported (metav1 or clusterv1 and where they are located. + + var metav1ConditionsFields []string + var clusterv1ConditionsFields []string + + if v1beta2Field := statusField.FieldByName("V1Beta2"); v1beta2Field != (reflect.Value{}) { + if conditionField := v1beta2Field.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + metav1ConditionsFields = []string{"status", "v1beta2", "conditions"} + } + } + + if conditionField := statusField.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + if conditionField.Type() == metav1ConditionsType { + metav1ConditionsFields = []string{"status", "conditions"} + } + if conditionField.Type() == clusterv1ConditionsType { + clusterv1ConditionsFields = []string{"status", "conditions"} + } + } + + if deprecatedField := statusField.FieldByName("Deprecated"); deprecatedField != (reflect.Value{}) { + if v1beta1Field := deprecatedField.FieldByName("V1Beta1"); v1beta1Field != (reflect.Value{}) { + if conditionField := v1beta1Field.FieldByName("Conditions"); conditionField != (reflect.Value{}) { + clusterv1ConditionsFields = []string{"status", "deprecated", "v1beta1", "conditions"} + } + } + } + + return metav1ConditionsFields, clusterv1ConditionsFields, nil +} diff --git a/util/patch/utils_test.go b/util/patch/utils_test.go index df43575d6f15..1dc6c2519380 100644 --- a/util/patch/utils_test.go +++ b/util/patch/utils_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/internal/test/builder" ) func TestToUnstructured(t *testing.T) { @@ -128,7 +129,7 @@ func TestUnsafeFocusedUnstructured(t *testing.T) { }, } - newObj := unsafeUnstructuredCopy(obj, specPatch, true) + newObj := unsafeUnstructuredCopy(obj, specPatch, nil, []string{"status", "conditions"}) // Validate that common fields are always preserved. g.Expect(newObj.Object["apiVersion"]).To(Equal(obj.Object["apiVersion"])) @@ -170,7 +171,7 @@ func TestUnsafeFocusedUnstructured(t *testing.T) { }, } - newObj := unsafeUnstructuredCopy(obj, statusPatch, true) + newObj := unsafeUnstructuredCopy(obj, statusPatch, nil, []string{"status", "conditions"}) // Validate that common fields are always preserved. g.Expect(newObj.Object["apiVersion"]).To(Equal(obj.Object["apiVersion"])) @@ -219,7 +220,7 @@ func TestUnsafeFocusedUnstructured(t *testing.T) { }, } - newObj := unsafeUnstructuredCopy(obj, statusPatch, false) + newObj := unsafeUnstructuredCopy(obj, statusPatch, nil, nil) // Validate that spec is nil in the new object, but still exists in the old copy. g.Expect(newObj.Object["spec"]).To(BeNil()) @@ -241,3 +242,42 @@ func TestUnsafeFocusedUnstructured(t *testing.T) { g.Expect(obj.Object["status"].(map[string]interface{})["conditions"]).ToNot(BeNil()) }) } + +func TestIdentifySupportedConditions(t *testing.T) { + t.Run("v1beta1 object with conditions (phase 0)", func(t *testing.T) { + g := NewWithT(t) + + obj := &builder.Phase0Obj{} + metav1ConditionsFields, clusterv1ConditionsFields, err := identifySupportedConditions(obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(metav1ConditionsFields).To(BeEmpty()) + g.Expect(clusterv1ConditionsFields).To(Equal([]string{"status", "conditions"})) + }) + t.Run("v1beta1 object with both clusterv1.conditions and metav1.conditions (phase 1)", func(t *testing.T) { + g := NewWithT(t) + + obj := &builder.Phase1Obj{} + metav1ConditionsFields, clusterv1ConditionsFields, err := identifySupportedConditions(obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(metav1ConditionsFields).To(Equal([]string{"status", "v1beta2", "conditions"})) + g.Expect(clusterv1ConditionsFields).To(Equal([]string{"status", "conditions"})) + }) + t.Run("v1beta2 object with both clusterv1.conditions and metav1.conditions conditions (phase 2)", func(t *testing.T) { + g := NewWithT(t) + + obj := &builder.Phase2Obj{} + metav1ConditionsFields, clusterv1ConditionsFields, err := identifySupportedConditions(obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(metav1ConditionsFields).To(Equal([]string{"status", "conditions"})) + g.Expect(clusterv1ConditionsFields).To(Equal([]string{"status", "deprecated", "v1beta1", "conditions"})) + }) + t.Run("v1beta2 object with metav1.conditions (phase 3)", func(t *testing.T) { + g := NewWithT(t) + + obj := &builder.Phase3Obj{} + metav1ConditionsFields, clusterv1ConditionsFields, err := identifySupportedConditions(obj) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(metav1ConditionsFields).To(Equal([]string{"status", "conditions"})) + g.Expect(clusterv1ConditionsFields).To(BeEmpty()) + }) +}