diff --git a/util/patch/patch.go b/util/patch/patch.go index 7a0bde9fdaa1..0af7a7f06864 100644 --- a/util/patch/patch.go +++ b/util/patch/patch.go @@ -84,14 +84,6 @@ func NewHelper(obj client.Object, crClient client.Client) (*Helper, error) { 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 { - clusterv1ConditionsFieldPath = nil - } - if _, canInterfaceConditions := obj.(conditions.Getter); !canInterfaceConditions { - clusterv1ConditionsFieldPath = nil - } - // Convert the object to unstructured to compare against our before copy. unstructuredObj, err := toUnstructured(obj, gvk) if err != nil { @@ -132,10 +124,10 @@ func (h *Helper) Patch(ctx context.Context, obj client.Object, opts ...Option) e // If condition field path override have been provided, propagate them to the helper for usage in various places of this func. if len(options.Clusterv1ConditionsFieldPath) > 0 { - h.clusterv1ConditionsFieldPath = nil + h.clusterv1ConditionsFieldPath = options.Clusterv1ConditionsFieldPath } if len(options.Metav1ConditionsFieldPath) > 0 { - h.metav1ConditionsFieldPath = nil + h.metav1ConditionsFieldPath = options.Metav1ConditionsFieldPath } // Check if the object satisfies the Cluster API contract setter interfaces; if not, ignore condition field path entirely. @@ -246,11 +238,11 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f // // 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.Setter) + before, ok := h.beforeObject.(conditions.Getter) if !ok { return errors.Errorf("%s %s doesn't satisfy conditions.Getter, cannot patch", h.gvk.Kind, klog.KObj(h.beforeObject)) } - after, ok := obj.(conditions.Setter) + after, ok := obj.(conditions.Getter) if !ok { return errors.Errorf("%s %s doesn't satisfy conditions.Getter, cannot compute patch", h.gvk.Kind, klog.KObj(obj)) } diff --git a/util/patch/patch_test.go b/util/patch/patch_test.go index cad221cc4cd2..6dbc0d386c30 100644 --- a/util/patch/patch_test.go +++ b/util/patch/patch_test.go @@ -1050,6 +1050,45 @@ func TestPatchHelperForV1beta2Transition(t *testing.T) { }, timeout).Should(conditions.MatchConditions(obj.Status.Conditions)) }) + t.Run("should mark it ready when passing Clusterv1ConditionsFieldPath", 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, Clusterv1ConditionsFieldPath{"status", "conditions"})).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) @@ -1386,6 +1425,53 @@ func TestPatchHelperForV1beta2Transition(t *testing.T) { }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.V1Beta2.Conditions)) }) + t.Run("should mark it ready when passing Clusterv1ConditionsFieldPath and Metav1ConditionsFieldPath", 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) + v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood", LastTransitionTime: now}) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, Clusterv1ConditionsFieldPath{"status", "conditions"}, Metav1ConditionsFieldPath{"status", "v1beta2", "conditions"})).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)) + }) + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { g := NewWithT(t) @@ -1827,6 +1913,53 @@ func TestPatchHelperForV1beta2Transition(t *testing.T) { }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.Conditions)) }) + t.Run("should mark it ready when passing Clusterv1ConditionsFieldPath and Metav1ConditionsFieldPath", 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) + v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood", LastTransitionTime: now}) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, Clusterv1ConditionsFieldPath{"status", "deprecated", "v1beta1", "conditions"}, Metav1ConditionsFieldPath{"status", "conditions"})).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)) + }) + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { g := NewWithT(t) @@ -2257,6 +2390,45 @@ func TestPatchHelperForV1beta2Transition(t *testing.T) { }, timeout).Should(v1beta2conditions.MatchConditions(obj.Status.Conditions)) }) + t.Run("should mark it ready when passing Metav1ConditionsFieldPath", 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") + v1beta2conditions.Set(obj, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood", LastTransitionTime: now}) + + t.Log("Patching the object") + g.Expect(patcher.Patch(ctx, obj, Metav1ConditionsFieldPath{"status", "conditions"})).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)) + }) + t.Run("should recover if there is a resolvable conflict", func(t *testing.T) { g := NewWithT(t)