Skip to content

refactor(exoscale): reduce ApplyChanges cyclomatic complexity - #6709

Draft
somaz94 wants to merge 1 commit into
kubernetes-sigs:masterfrom
somaz94:refactor/exoscale-applychanges-complexity
Draft

somaz94 wants to merge 1 commit into
kubernetes-sigs:masterfrom
somaz94:refactor/exoscale-applychanges-complexity

Conversation

@somaz94

@somaz94 somaz94 commented Sep 8, 2026

Copy link
Copy Markdown

What does it do ?

Splits ExoscaleProvider.ApplyChanges into per-operation helpers (createRecords,
updateRecords, deleteRecords) plus two shared helpers: resolveZone for the
domain-filter and zone lookup that all three repeated, and findRecordID for the
name and type record match that update and delete duplicated verbatim.

This lowers the function's cyclomatic complexity from 26 to 6, with no new helper
above 7, leaving merge (9) as the package maximum. Output, ordering, error
propagation and the number of API calls are unchanged.

Part of #5419.

Motivation

ApplyChanges was the most complex function in the provider, and the update and
delete paths had drifted into near-identical copies of the same record lookup.
Separating them makes each operation reviewable on its own as the project lowers
its cyclop threshold.

Testing

  • go test -race -count=1 ./provider/exoscale/ (20 tests)
  • make test (full suite, all packages)
  • golangci-lint run --timeout=10m ./provider/exoscale/... (0 issues)
  • make licensecheck
  • cyclop with max-complexity: 1 to read the exact numbers before and after
  • The two tests added here cover the ListDNSDomainRecords error path and the
    no-matching-record path, neither of which was covered before. Both pass
    unmodified against the pre-refactor code, so they check that the behavior is
    the same rather than only that the new structure works.

AI assistance

Claude Code helped with the refactor and the two added tests. I reviewed the
result and checked the behavior myself with the test suite and linters.

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly

Signed-off-by: somaz <genius5711@gmail.com>
@kubernetes-prow kubernetes-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign szuecs for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the provider Issues or PRs related to a provider label Sep 8, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

Welcome @somaz94!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 8, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

Hi @somaz94. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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-sigs/prow repository.

@kubernetes-prow kubernetes-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 8, 2026
somaz94 added a commit to somaz94/somaz94 that referenced this pull request Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. provider Issues or PRs related to a provider size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant