Filter System tags before patching #172
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change ensures that neither ACK tags nor AWS tags will be patched. Sometimes if there's a tag sync issues, these tags get patched to the controller.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: michaelhtm The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
This change introduces some changes to the mocks directory.
|
@michaelhtm: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
michaelhtm
added a commit
to michaelhtm/ack-code-generator
that referenced
this pull request
Mar 6, 2025
When converting between ACK tags (map[string]string) and resource tags(which could be either a list of Tag struct or map[string]*string) can be tricky. One issue that we found was order preservation, especially when converting from `map[string]string` to list of `Tag`. The changed order, when patched to the resource in the cluster, would trigger a reconciliation, for an unpredictable amount of times (at least until we luckily convert to the list with a preserved order.) This change introduces two new functions, similar to the existing ones. The `toACKTagsWithKeyOrder` has the same function as `ToACKTags`, but besides returning the ACK tags, it also returns a list of keys in the same order it is in the resource. The `fromACKTagsWithKeyOrder`, also similar to `FromACKTags`, accepts the keyOrder returned by `toACKTagsWithKeyOrder`, and rebuilds the resource tags, if it's a list, in the same order as it was before. Any extra tags will be added in random order. These two new functions will be called before and after `mirrowAWSTags` and `FilterSystemTags`. The extra tags mentioned earlier are the AWS tags that are mirrored from the latest resource (more explanation on what they are [here](aws-controllers-k8s/runtime#170)) and will be filtered before getting patched with [this](aws-controllers-k8s/runtime#172) change. This will ensure that the tags in desired will maintain their order and avoid triggering unnecessary reconciliations.
michaelhtm
added a commit
to michaelhtm/ack-code-generator
that referenced
this pull request
Mar 6, 2025
When converting between ACK tags (map[string]string) and resource tags(which could be either a list of Tag struct or map[string]*string) can be tricky. One issue that we found was order preservation, especially when converting from `map[string]string` to list of `Tag`. The changed order, when patched to the resource in the cluster, would trigger a reconciliation, for an unpredictable amount of times (at least until we luckily convert to the list with a preserved order.) This change introduces two new functions, similar to the existing ones. The `toACKTagsWithKeyOrder` has the same function as `ToACKTags`, but besides returning the ACK tags, it also returns a list of keys in the same order it is in the resource. The `fromACKTagsWithKeyOrder`, also similar to `FromACKTags`, accepts the keyOrder returned by `toACKTagsWithKeyOrder`, and rebuilds the resource tags, if it's a list, in the same order as it was before. Any extra tags will be added in random order. These two new functions will be called before and after `mirrowAWSTags` and `FilterSystemTags`. The extra tags mentioned earlier are the AWS tags that are mirrored from the latest resource (more explanation on what they are [here](aws-controllers-k8s/runtime#170)) and will be filtered before getting patched with [this](aws-controllers-k8s/runtime#172) change. This will ensure that the tags in desired will maintain their order and avoid triggering unnecessary reconciliations.
michaelhtm
added a commit
to michaelhtm/ack-code-generator
that referenced
this pull request
Mar 7, 2025
When converting between ACK tags (map[string]string) and resource tags(which could be either a list of Tag struct or map[string]*string) can be tricky. One issue that we found was order preservation, especially when converting from `map[string]string` to list of `Tag`. The changed order, when patched to the resource in the cluster, would trigger a reconciliation, for an unpredictable amount of times (at least until we luckily convert to the list with a preserved order.) This change introduces two new functions, similar to the existing ones. The `toACKTagsWithKeyOrder` has the same function as `ToACKTags`, but besides returning the ACK tags, it also returns a list of keys in the same order it is in the resource. The `fromACKTagsWithKeyOrder`, also similar to `FromACKTags`, accepts the keyOrder returned by `toACKTagsWithKeyOrder`, and rebuilds the resource tags, if it's a list, in the same order as it was before. Any extra tags will be added in random order. These two new functions will be called before and after `mirrowAWSTags` and `FilterSystemTags`. The extra tags mentioned earlier are the AWS tags that are mirrored from the latest resource (more explanation on what they are [here](aws-controllers-k8s/runtime#170)) and will be filtered before getting patched with [this](aws-controllers-k8s/runtime#172) change. This will ensure that the tags in desired will maintain their order and avoid triggering unnecessary reconciliations.
ack-prow bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this pull request
Mar 10, 2025
…#572) Description of changes: When converting between ACK tags (map[string]string) and resource tags(which could be either a list of Tag struct or map[string]*string) can be tricky. One issue that we found was order preservation, especially when converting from `map[string]string` to list of `Tag`. The changed order, when patched to the resource in the cluster, would trigger a reconciliation, for an unpredictable amount of times (at least until we luckily convert to the list with a preserved order.) This change introduces two new functions, similar to the existing ones. The `toACKTagsWithKeyOrder` has the same function as `ToACKTags`, but besides returning the ACK tags, it also returns a list of keys in the same order it is in the resource. The `fromACKTagsWithKeyOrder`, also similar to `FromACKTags`, accepts the keyOrder returned by `toACKTagsWithKeyOrder`, and rebuilds the resource tags, if it's a list, in the same order as it was before. Any extra tags will be added in random order. These two new functions will be called before and after `mirrowAWSTags` and `FilterSystemTags`. The extra tags mentioned earlier are the AWS tags that are mirrored from the latest resource (more explanation on what they are [here](aws-controllers-k8s/runtime#170)) and will be filtered before getting patched with [this](aws-controllers-k8s/runtime#172) change. This will ensure that the tags in desired will maintain their order and avoid triggering unnecessary reconciliations. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
This change ensures that neither ACK tags nor AWS tags will be patched.
Sometimes if there's a tag sync issues, these tags get patched to the controller.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.