Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v1.1] Allow GeneratingHandlers to skip Apply if resource version didn't change #351

Merged

Conversation

aruiz14
Copy link
Contributor

@aruiz14 aruiz14 commented Jan 3, 2024

Backport of #341. Note that this pull request was originally forward-ported from v1, as that is what Fleet 0.7 and Fleet 0.8 uses. This makes this patch the closest to what was already tested by some users on their testing and production setups (using a debug build). In other words, this backport (and subsequent new v1 release) is necessary in order to use this change on Fleet, since the current release branches still use wrangler/v1, and upgrading to v2 is not feasible.

Besides updating the import paths, the only change from the original PR is in the test code:
Since the pkg/generic/fake does not exist in v1 (used in tests), I had to replace it by using a mock

@aruiz14 aruiz14 requested review from KevinJoiner, rmweir and moio January 3, 2024 10:51
@aruiz14
Copy link
Contributor Author

aruiz14 commented Jan 3, 2024

I needed this change in v1 in order to be able to use it in Fleet (master branch already upgraded to v2, but existing release branches still use v1).

@aruiz14
Copy link
Contributor Author

aruiz14 commented Jan 3, 2024

CI is consistently failing but it's unrelated to my changes. I've opened #352 to address it

Copy link
Contributor

@moio moio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional changes seem identical to the original PR - I will let others comment on test code if necessary. LGTM

aruiz14 and others added 2 commits January 9, 2024 18:36
…nge (rancher#341)

* Allow GeneratingHandlers to skip Apply if resource version didn't change

* Run go generate

* Add call count to FakeApply

* Add test for UniqueApplyForResourceVersion

* Simplify error handling

Co-authored-by: Kevin Joiner <[email protected]>

* Make generated code more readable

Co-authored-by: Kevin Joiner <[email protected]>

* Rename constant

* Rename seenResourceVersion to storeResourceVersion

* go generate

* Add comments to code generator

* Run go generate

* Upgrade golangci-lint GitHub action, since it always fails now

---------

Co-authored-by: Kevin Joiner <[email protected]>
@aruiz14 aruiz14 force-pushed the v1-generating-handlers-unique-apply branch from 71dd4a6 to 5e12992 Compare January 9, 2024 17:37
@mattfarina mattfarina merged commit b5a5a52 into rancher:release-1.1 Jan 10, 2024
@aruiz14 aruiz14 deleted the v1-generating-handlers-unique-apply branch January 10, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants