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

feat(internet): add retries for internet dependent states #55

Merged
merged 1 commit into from
Jul 1, 2019

Conversation

noelmcloughlin
Copy link
Member

This PR adds retry for internet dependent states.

For example workaround "Temporary failure in name resolution" issue.

          ID: packages-archive-wanted-download-kubectl
    Function: file.managed
        Name: /usr/local/bin/kubectl
      Result: False
     Comment: Failed to cache 
https://storage.googleapis.com/kubernetes-release/release/v1.12.3/bin/linux/amd64/kubectl: 
Error: [Errno -3] Temporary failure in name resolution reading 
https://storage.googleapis.com/kubernetes-release/release/v1.12.3/bin/linux/amd64/kubectl
     Started: 03:26:55.100108
    Duration: 2635.434 ms

@myii
Copy link
Member

myii commented Jun 25, 2019

@noelmcloughlin @javierbertoli Just thinking out loud but if we're going forward with this, wouldn't it be worth removing the duplication, perhaps using a little macro?

@javierbertoli
Copy link
Member

I was thinking about the duplication, and was more on the line of configurable values in the YAMLs instead of a macro, to keep it simple, like

  • defaults.yaml
packages:
  resources_retries:
    attempts: 3
    until: True
    interval: 60
    splay: 10

and then, use like

packages-archive-wanted-download-{{ package }}:
  file.managed:
    - name: {{ archive.dest }}/{{ archivename }}
...
    - retry: {{ packages.resources_retries }}

or similar approach? wdyt?

@noelmcloughlin
Copy link
Member Author

Thank you both for reviewing. @javierbertoli @myii
Yes lets remove the duplication.
I called the dict retry_option because that is what its called in saltstack documentation.
I updated pillar.example too.

@noelmcloughlin
Copy link
Member Author

Are you okay to approve @javierbertoli

@javierbertoli
Copy link
Member

Is too pedantic to ask you to set it to retry_options (plural)? I see the logs failures are not related to this PR so, if we agree in the naming, I'll be happy to merge it

@noelmcloughlin
Copy link
Member Author

Done. I intended singular over plural but it's a matter of taste in language usage.
Thanks, would be great to merge this.

pillar.example Outdated Show resolved Hide resolved
@javierbertoli javierbertoli merged commit 8941b20 into saltstack-formulas:master Jul 1, 2019
@javierbertoli
Copy link
Member

Thanks, @noelmcloughlin !

@noelmcloughlin noelmcloughlin deleted the retries branch July 1, 2019 23:34
@saltstack-formulas-travis

🎉 This PR is included in version 0.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants