From 840afd155e327c1317eca363d38b287347801471 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Thu, 13 Apr 2023 18:03:41 +0200 Subject: [PATCH 1/2] Clean up and comment some desiredset funcs --- pkg/apply/desiredset_apply.go | 12 ++++++++++++ pkg/apply/desiredset_compare.go | 11 +++++++++-- pkg/apply/desiredset_process.go | 5 ++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/apply/desiredset_apply.go b/pkg/apply/desiredset_apply.go index cf4ee232..f39b793a 100644 --- a/pkg/apply/desiredset_apply.go +++ b/pkg/apply/desiredset_apply.go @@ -80,6 +80,7 @@ func (o *desiredSet) apply() error { return err } + // Create maps for labels and annotations, contains identifying hash used for rate limitting labelSet, annotationSet, err := GetLabelsAndAnnotations(o.setID, o.owner) if err != nil { return o.err(err) @@ -141,6 +142,7 @@ func (o *desiredSet) debugID() string { }) } +// collect converts a list of runtime objects into a ObjectByGVK map func (o *desiredSet) collect(objList []runtime.Object) objectset.ObjectByGVK { result := objectset.ObjectByGVK{} for _, obj := range objList { @@ -189,6 +191,8 @@ func GetSelectorFromOwner(setID string, owner runtime.Object) (labels.Selector, return GetSelector(ownerLabel) } +// GetSelector returns the label selector for the owner object, which is useful +// to list the dependents func GetSelector(labelSet map[string]string) (labels.Selector, error) { req, err := labels.NewRequirement(LabelHash, selection.Equals, []string{labelSet[LabelHash]}) if err != nil { @@ -197,6 +201,10 @@ func GetSelector(labelSet map[string]string) (labels.Selector, error) { return labels.NewSelector().Add(*req), nil } +// GetLabelsAndAnnotations returns a new map of annotations with the setID +// and/or owner in it. It also returns a new map of labels, which contains a +// hash over the annotations. The hash is used to identify resources that +// belong to the same setID/owner. func GetLabelsAndAnnotations(setID string, owner runtime.Object) (map[string]string, map[string]string, error) { if setID == "" && owner == nil { return nil, nil, fmt.Errorf("set ID or owner must be set") @@ -227,6 +235,8 @@ func GetLabelsAndAnnotations(setID string, owner runtime.Object) (map[string]str return labels, annotations, nil } +// injectLabelsAndAnnotations returns the desiredSet's objects. The given +// labels and annotations extend and overwrite existing labels func (o *desiredSet) injectLabelsAndAnnotations(labels, annotations map[string]string) ([]runtime.Object, error) { var result []runtime.Object @@ -248,6 +258,7 @@ func (o *desiredSet) injectLabelsAndAnnotations(labels, annotations map[string]s return result, nil } +// setAnnotations overwrites the annotations in meta with the given annotations map func setAnnotations(meta metav1.Object, annotations map[string]string) { objAnn := meta.GetAnnotations() if objAnn == nil { @@ -260,6 +271,7 @@ func setAnnotations(meta metav1.Object, annotations map[string]string) { meta.SetAnnotations(objAnn) } +// setLabels overwrites the labels in meta with the given labels map func setLabels(meta metav1.Object, labels map[string]string) { objLabels := meta.GetLabels() if objLabels == nil { diff --git a/pkg/apply/desiredset_compare.go b/pkg/apply/desiredset_compare.go index 15abe6dc..80837490 100644 --- a/pkg/apply/desiredset_compare.go +++ b/pkg/apply/desiredset_compare.go @@ -23,7 +23,6 @@ import ( "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/jsonmergepatch" "k8s.io/apimachinery/pkg/util/strategicpatch" - "k8s.io/client-go/dynamic" ) const ( @@ -45,6 +44,10 @@ var ( } ) +// prepareObjectForCreate returns a copy of the object with the "applied" +// annotation set. The 'applied' annotation contains a serialized, pruned +// representation of obj, e.g. Strings are shortened to 64 characters and bytes +// are skipped. func prepareObjectForCreate(gvk schema.GroupVersionKind, obj runtime.Object) (runtime.Object, error) { serialized, err := serializeApplied(obj) if err != nil { @@ -76,6 +79,8 @@ func prepareObjectForCreate(gvk schema.GroupVersionKind, obj runtime.Object) (ru return obj, nil } +// originalAndModified returns the original bytes from the oldObject's +// "applied" annotation and the 'modified' bytes for the newObject func originalAndModified(gvk schema.GroupVersionKind, oldMetadata v1.Object, newObject runtime.Object) ([]byte, []byte, error) { original, err := getOriginalBytes(gvk, oldMetadata) if err != nil { @@ -169,6 +174,7 @@ func sanitizePatch(patch []byte, removeObjectSetAnnotation bool) ([]byte, error) return json.Marshal(data) } +// applyPatch applies the patch to the object and returns true if the object was changed func applyPatch(gvk schema.GroupVersionKind, reconciler Reconciler, patcher Patcher, debugID string, ignoreOriginal bool, oldObject, newObject runtime.Object, diffPatches [][]byte) (bool, error) { oldMetadata, err := meta.Accessor(oldObject) if err != nil { @@ -235,7 +241,8 @@ func applyPatch(gvk schema.GroupVersionKind, reconciler Reconciler, patcher Patc return true, err } -func (o *desiredSet) compareObjects(gvk schema.GroupVersionKind, reconciler Reconciler, patcher Patcher, client dynamic.NamespaceableResourceInterface, debugID string, oldObject, newObject runtime.Object, force bool) error { +// compareObjects updates the resource with the passed in patcher. It considers WithDiffPatch and WithReconciler. +func (o *desiredSet) compareObjects(gvk schema.GroupVersionKind, reconciler Reconciler, patcher Patcher, debugID string, oldObject, newObject runtime.Object) error { oldMetadata, err := meta.Accessor(oldObject) if err != nil { return err diff --git a/pkg/apply/desiredset_process.go b/pkg/apply/desiredset_process.go index e24c8d59..1077ff0b 100644 --- a/pkg/apply/desiredset_process.go +++ b/pkg/apply/desiredset_process.go @@ -207,6 +207,9 @@ func (o *desiredSet) filterCrossVersion(gvk schema.GroupVersionKind, keys []obje return result } +// process executes the apply by creating, updating, and deleting objects. It +// compares the desired state to the current state and builds necessary +// patchers. func (o *desiredSet) process(debugID string, set labels.Selector, gvk schema.GroupVersionKind, objs objectset.ObjectByKey) { controller, client, err := o.getControllerAndClient(debugID, gvk) if err != nil { @@ -316,7 +319,7 @@ func (o *desiredSet) process(debugID string, set labels.Selector, gvk schema.Gro } updateF := func(k objectset.ObjectKey) { - err := o.compareObjects(gvk, reconciler, patcher, client, debugID, existing[k], objs[k], len(toCreate) > 0 || len(toDelete) > 0) + err := o.compareObjects(gvk, reconciler, patcher, debugID, existing[k], objs[k]) if err == ErrReplace { deleteF(k, true) o.err(fmt.Errorf("DesiredSet - Replace Wait %s %s for %s", gvk, k, debugID)) From 7b8615c016b140799a8388eade97491d6e4115be Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Wed, 9 Oct 2024 12:03:01 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Kevin Joiner <10265309+KevinJoiner@users.noreply.github.com> --- pkg/apply/desiredset_apply.go | 12 ++++++------ pkg/apply/desiredset_compare.go | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/apply/desiredset_apply.go b/pkg/apply/desiredset_apply.go index f39b793a..e79ebc17 100644 --- a/pkg/apply/desiredset_apply.go +++ b/pkg/apply/desiredset_apply.go @@ -80,7 +80,7 @@ func (o *desiredSet) apply() error { return err } - // Create maps for labels and annotations, contains identifying hash used for rate limitting + // Create maps for labels and annotations containing the identifying hash used for rate limiting. labelSet, annotationSet, err := GetLabelsAndAnnotations(o.setID, o.owner) if err != nil { return o.err(err) @@ -142,7 +142,7 @@ func (o *desiredSet) debugID() string { }) } -// collect converts a list of runtime objects into a ObjectByGVK map +// collect converts a list of runtime objects into an ObjectByGVK map. func (o *desiredSet) collect(objList []runtime.Object) objectset.ObjectByGVK { result := objectset.ObjectByGVK{} for _, obj := range objList { @@ -192,7 +192,7 @@ func GetSelectorFromOwner(setID string, owner runtime.Object) (labels.Selector, } // GetSelector returns the label selector for the owner object, which is useful -// to list the dependents +// to list the dependents. func GetSelector(labelSet map[string]string) (labels.Selector, error) { req, err := labels.NewRequirement(LabelHash, selection.Equals, []string{labelSet[LabelHash]}) if err != nil { @@ -236,7 +236,7 @@ func GetLabelsAndAnnotations(setID string, owner runtime.Object) (map[string]str } // injectLabelsAndAnnotations returns the desiredSet's objects. The given -// labels and annotations extend and overwrite existing labels +// labels and annotations extend and overwrite existing labels. func (o *desiredSet) injectLabelsAndAnnotations(labels, annotations map[string]string) ([]runtime.Object, error) { var result []runtime.Object @@ -258,7 +258,7 @@ func (o *desiredSet) injectLabelsAndAnnotations(labels, annotations map[string]s return result, nil } -// setAnnotations overwrites the annotations in meta with the given annotations map +// setAnnotations overwrites the annotations in meta with the given annotations map. func setAnnotations(meta metav1.Object, annotations map[string]string) { objAnn := meta.GetAnnotations() if objAnn == nil { @@ -271,7 +271,7 @@ func setAnnotations(meta metav1.Object, annotations map[string]string) { meta.SetAnnotations(objAnn) } -// setLabels overwrites the labels in meta with the given labels map +// setLabels overwrites the labels in meta with the given labels map. func setLabels(meta metav1.Object, labels map[string]string) { objLabels := meta.GetLabels() if objLabels == nil { diff --git a/pkg/apply/desiredset_compare.go b/pkg/apply/desiredset_compare.go index 80837490..e46fa0ad 100644 --- a/pkg/apply/desiredset_compare.go +++ b/pkg/apply/desiredset_compare.go @@ -45,7 +45,7 @@ var ( ) // prepareObjectForCreate returns a copy of the object with the "applied" -// annotation set. The 'applied' annotation contains a serialized, pruned +// annotation set. The "applied" annotation contains a serialized and pruned // representation of obj, e.g. Strings are shortened to 64 characters and bytes // are skipped. func prepareObjectForCreate(gvk schema.GroupVersionKind, obj runtime.Object) (runtime.Object, error) { @@ -80,7 +80,7 @@ func prepareObjectForCreate(gvk schema.GroupVersionKind, obj runtime.Object) (ru } // originalAndModified returns the original bytes from the oldObject's -// "applied" annotation and the 'modified' bytes for the newObject +// "applied" annotation and the modified bytes for the newObject. func originalAndModified(gvk schema.GroupVersionKind, oldMetadata v1.Object, newObject runtime.Object) ([]byte, []byte, error) { original, err := getOriginalBytes(gvk, oldMetadata) if err != nil { @@ -174,7 +174,7 @@ func sanitizePatch(patch []byte, removeObjectSetAnnotation bool) ([]byte, error) return json.Marshal(data) } -// applyPatch applies the patch to the object and returns true if the object was changed +// applyPatch applies the patch to the object and returns true if the object was changed. func applyPatch(gvk schema.GroupVersionKind, reconciler Reconciler, patcher Patcher, debugID string, ignoreOriginal bool, oldObject, newObject runtime.Object, diffPatches [][]byte) (bool, error) { oldMetadata, err := meta.Accessor(oldObject) if err != nil {