Skip to content

Implement retry logic on downloader#606

Open
Shunpoco wants to merge 6 commits intospinframework:mainfrom
Shunpoco:implement-downloader-retry
Open

Implement retry logic on downloader#606
Shunpoco wants to merge 6 commits intospinframework:mainfrom
Shunpoco:implement-downloader-retry

Conversation

@Shunpoco
Copy link
Contributor

@Shunpoco Shunpoco commented Feb 21, 2026

Describe your changes

A downloader needs a retry logic for curl, because sometimes network trouble happens.

This PR does:

  • Add a retry logic in the downloader shell script. The number of retry and sleep duration between iteration are configurable
  • Modify helm chart to add a ConfigMap for setting those variables on downloader container
  • Modify Reconciler code to add the ConfigMap if it is specified
  • Add a verification step for a ConfigMap from helm chart to actual installer pod

Issue ticket number and link

#591

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • I tested the changes with the following distributions:
    • Kind
    • MiniKube
    • MicroK8s
    • Rancher RKE2
    • Azure AKS
    • GCP GKE (Ubuntu nodes)
    • AWS EKS (AmazonLinux2 nodes)
    • AWS EKS (Ubuntu nodes)
    • Digital Ocean Kubernetes

Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
@Shunpoco Shunpoco force-pushed the implement-downloader-retry branch from 6607064 to 0e6dca9 Compare February 21, 2026 10:47
Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
@vdice vdice self-requested a review March 9, 2026 16:46
Copy link
Collaborator

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Hi @Shunpoco, sorry for the delay in reviewing. This looks really good. I had one thought around simplifying the configuration such that the explicit enabled field is no longer needed. What do you think?

ttl: 0
leaderElectEnabled: false
# shimDownloaderConfig generates a ConfigMap which sets a downloader container's variables. Since those variables have defaults, you don't need to enable it unless you want to customize them.
shimDownloaderConfig:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about removing the enabled field and commenting this sample configuration out? Then, in the deployment template (and any others), we can just use: {{- if .Values.rcm.shimDownloaderConfig }}

Maybe something like:

# shimDownloaderConfig generates a ConfigMap which sets a downloader container's variables. Since those variables have defaults, you don't need to enable it unless you want to customize them.
# shimDownloaderConfig:
#   configMapName: shim-downloader-config
#   content:
#     numRetry: 3
#     sleepDuration: 2

When the user/operator would like to supply different values, they can uncomment and adjust as needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, e71016b addressed it.

@Shunpoco Shunpoco force-pushed the implement-downloader-retry branch from 49086b8 to ceb5320 Compare March 9, 2026 21:58
Signed-off-by: Shunpoco <tkngsnsk313320@gmail.com>
@Shunpoco Shunpoco force-pushed the implement-downloader-retry branch from ceb5320 to e71016b Compare March 9, 2026 22:18
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.

2 participants