fix(azure): enhance retry logic using azure SDK #5361

Reddyshruti26 · 2025-05-06T09:18:59Z

Description
This PR enhances the retry logic of the Azure SDK to allow customers to configure it using an environment variable specified in the manifest file, depending on their workload. Currently, this serves as a short-term solution for our users for both public and private DNS. A long-term fix is planned and will be implemented in the near future. The relevant documentation has been updated accordingly to reflect this change.

Fixes #4839

Checklist

Unit tests updated
End user documentation updated

linux-foundation-easycla · 2025-05-06T09:19:04Z

The committers listed above are authorized under a signed CLA.

✅ login: Reddyshruti26 / name: Shruti Panapana (0869c4a, cbe4015, 3c6ab12, 2859743, 69e7da3, e80b2bb, 3c05d8c)

k8s-ci-robot · 2025-05-06T09:19:08Z

Welcome @Reddyshruti26!

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. 😃

k8s-ci-robot · 2025-05-06T09:19:09Z

Hi @Reddyshruti26. 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. Regular contributors should join the org to skip this step.

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.

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.

provider/azure/config.go

mloiseleur · 2025-05-07T20:18:11Z

@Reddyshruti26 You need to sign the CLA before we can review this PR.

docs/tutorials/azure.md

mloiseleur · 2025-05-08T08:20:49Z

/ok-to-test

provider/azure/config.go

mloiseleur

All the parameters in external-dns are configured with CLI args, starting with providers name.

Would you please follow this convention on this new parameter ?

Reddyshruti26 · 2025-05-08T09:07:05Z

All the parameters in external-dns are configured with CLI args, starting with providers name.

Would you please follow this convention on this new parameter ?

Hi @mloiseleur, Could you please clarify if the AZURE_SDK_MAX_RETRIES should be configured as a CLI argument instead of an environment variable?

mloiseleur · 2025-05-08T09:45:53Z

Hi @mloiseleur, Could you please clarify if the AZURE_SDK_MAX_RETRIES should be configured as a CLI argument instead of an environment variable?

Yes, that's my point.

mloiseleur · 2025-05-08T10:09:33Z

To be more precise, all CLI flags can be used as env variable.

See --help on azure:

  --azure-config-file="/etc/kubernetes/azure.json"  
                                 When using the Azure provider,
                                 specify the Azure configuration file
                                 (required when --provider=azure)
                                 ($EXTERNAL_DNS_AZURE_CONFIG_FILE)
  --azure-resource-group=""      When using the Azure provider, override
                                 the Azure resource group to use (optional)
                                 ($EXTERNAL_DNS_AZURE_RESOURCE_GROUP)
  --azure-subscription-id=""     When using the Azure provider, override
                                 the Azure subscription to use (optional)
                                 ($EXTERNAL_DNS_AZURE_SUBSCRIPTION_ID)
  --azure-user-assigned-identity-client-id=""  
                                 When using the Azure provider,
                                 override the client id of user assigned
                                 identity in config file (optional)
                                 ($EXTERNAL_DNS_AZURE_USER_ASSIGNED_IDENTITY_CLIENT_ID)
  --azure-zones-cache-duration=0s  
                                 When using the Azure provider, set the
                                 zones list cache TTL (0s to disable).
                                 ($EXTERNAL_DNS_AZURE_ZONES_CACHE_DURATION)

Reddyshruti26 · 2025-05-09T03:39:54Z

To be more precise, all CLI flags can be used as env variable.

See --help on azure:

  --azure-config-file="/etc/kubernetes/azure.json"  
                                 When using the Azure provider,
                                 specify the Azure configuration file
                                 (required when --provider=azure)
                                 ($EXTERNAL_DNS_AZURE_CONFIG_FILE)
  --azure-resource-group=""      When using the Azure provider, override
                                 the Azure resource group to use (optional)
                                 ($EXTERNAL_DNS_AZURE_RESOURCE_GROUP)
  --azure-subscription-id=""     When using the Azure provider, override
                                 the Azure subscription to use (optional)
                                 ($EXTERNAL_DNS_AZURE_SUBSCRIPTION_ID)
  --azure-user-assigned-identity-client-id=""  
                                 When using the Azure provider,
                                 override the client id of user assigned
                                 identity in config file (optional)
                                 ($EXTERNAL_DNS_AZURE_USER_ASSIGNED_IDENTITY_CLIENT_ID)
  --azure-zones-cache-duration=0s  
                                 When using the Azure provider, set the
                                 zones list cache TTL (0s to disable).
                                 ($EXTERNAL_DNS_AZURE_ZONES_CACHE_DURATION)

Hi @mloiseleur, I am assuming the implementation of this is already in place and I just have to implement the new flag as requested earlier. Or, do I have to implement the new flag and corresponding env variable?

provider/azure/azure_private_dns.go

pkg/apis/externaldns/types.go

mloiseleur · 2025-05-15T06:26:00Z

@sfiguemsft Any comment ?

mloiseleur · 2025-05-15T08:58:16Z

LGTM. It needs a 2nd review.
/approve

sfiguemsft · 2025-05-15T12:50:35Z

@sfiguemsft Any comment ?

Approved - please let me know if you need any further input.

provider/azure/azure.go

provider/azure/azure_private_dns.go

provider/azure/config.go

ivankatliarchuk · 2025-05-16T10:39:59Z

/label tide/merge-method-squash

… value from cli/env

ivankatliarchuk

/lgtm

k8s-ci-robot · 2025-05-17T08:47:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivankatliarchuk, mloiseleur

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mloiseleur]

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

vflaux · 2025-06-02T22:34:23Z

provider/azure/config_test.go

+}
+
+// Helper function to parse max retries value
+func parseMaxRetries(value string, defaultValue int) (int, error) {


defaultValue is not used in this function.
Is this expected ?

This is a redundant parameter that might have been missed during code clean-up.

k8s-ci-robot requested review from ivankatliarchuk and mloiseleur May 6, 2025 09:19

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 6, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 6, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 6, 2025

IyerBhuvi reviewed May 7, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

IyerBhuvi reviewed May 7, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

IyerBhuvi reviewed May 7, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

IyerBhuvi reviewed May 7, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

IyerBhuvi reviewed May 7, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

mloiseleur changed the title ~~fix(azure): Enhance retry logic using azure SDK~~ fix(azure): enhance retry logic using azure SDK May 7, 2025

fix(azure): Enhance retry logic using azure SDK

0869c4a

Reddyshruti26 force-pushed the azure-retry-fix branch from ce7c1e3 to 0869c4a Compare May 8, 2025 06:58

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 8, 2025

mloiseleur reviewed May 8, 2025

View reviewed changes

docs/tutorials/azure.md Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 8, 2025

mloiseleur reviewed May 8, 2025

View reviewed changes

provider/azure/config.go Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems May 8, 2025

View reviewed changes

provider/azure/config.go Fixed Show fixed Hide fixed

mloiseleur requested changes May 8, 2025

View reviewed changes

k8s-ci-robot assigned mloiseleur May 8, 2025

Reddyshruti26 added 3 commits May 13, 2025 13:09

Added the changes for flag based maxretries configuration

3c05d8c

Fixed types.go, flags.md and delected unneccesary comments

3c6ab12

Added the correct image for the Azure Private DNS tutorial

cbe4015

mloiseleur reviewed May 14, 2025

View reviewed changes

provider/azure/azure_private_dns.go Outdated Show resolved Hide resolved

Following the go naming convention for maxRetriesCount

e80b2bb

mloiseleur reviewed May 15, 2025

View reviewed changes

pkg/apis/externaldns/types.go Outdated Show resolved Hide resolved

Added the correct flag information to the --azure-maxretries-count

2859743

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2025

ivankatliarchuk suggested changes May 15, 2025

View reviewed changes

k8s-ci-robot assigned ivankatliarchuk May 15, 2025

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label May 16, 2025

Made the required changes to accept the --azure-maxretries-count flag…

69e7da3

… value from cli/env

ivankatliarchuk approved these changes May 17, 2025

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 17, 2025

k8s-ci-robot merged commit 7b9d8d9 into kubernetes-sigs:master May 17, 2025
13 checks passed

vflaux reviewed Jun 2, 2025

View reviewed changes

fix(azure): enhance retry logic using azure SDK #5361

fix(azure): enhance retry logic using azure SDK #5361

Uh oh!

Conversation

Reddyshruti26 commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented May 6, 2025

Uh oh!

k8s-ci-robot commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mloiseleur commented May 7, 2025

Uh oh!

Uh oh!

mloiseleur commented May 8, 2025

Uh oh!

Uh oh!

Uh oh!

mloiseleur left a comment

Choose a reason for hiding this comment

Uh oh!

Reddyshruti26 commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mloiseleur commented May 8, 2025

Uh oh!

mloiseleur commented May 8, 2025

Uh oh!

Reddyshruti26 commented May 9, 2025

Uh oh!

Uh oh!

Uh oh!

mloiseleur commented May 15, 2025

Uh oh!

mloiseleur commented May 15, 2025

Uh oh!

sfiguemsft commented May 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivankatliarchuk commented May 16, 2025

Uh oh!

ivankatliarchuk left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 17, 2025

Uh oh!

Uh oh!

vflaux Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reddyshruti26 Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Reddyshruti26 commented May 6, 2025 •

edited

Loading

linux-foundation-easycla bot commented May 6, 2025 •

edited

Loading

Reddyshruti26 commented May 8, 2025 •

edited

Loading