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

Timeouts when attempting to use vendir with a registry with a significant amount of tags #329

Closed
alexbarbato opened this issue Dec 12, 2023 · 3 comments · Fixed by #344
Closed
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed

Comments

@alexbarbato
Copy link
Contributor

alexbarbato commented Dec 12, 2023

What steps did you take:
When attempting to use a vendir sync from a repository with a significant number of files there will be timeouts with no way to handle them.
With imgpkg natively you can use --registry-response-header-timeout, but that API isn't exposed here in vendir.

What happened:
The sync will timeout.

What did you expect:
That as a user I could extend the timeout as long as I need to be able to make it so the request won't time out.

Environment:

  • vendir version (execute vendir --version): 0.35.0
  • OS (e.g. from /etc/os-release): MacOS (ARM)

Example config

apiVersion: vendir.k14s.io/v1alpha1
kind: Config
minimumRequiredVersion: 0.12.0
directories:
  # Use this if you're downloading manifests directly from a location.
  - path: bundle/packages.temp
    contents:
      - path: .
        imgpkgBundle:
          image: REPO.COM/IMAGE-REPO
          tagSelection:
            semver:
              constraints: ">1.1.0 <1.7.1"
        newRootPath: packages
  - path: bundle/.imgpkg
    contents:
      - path: .
        imgpkgBundle: *tap
        newRootPath: .imgpkg

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@alexbarbato alexbarbato added bug This issue describes a defect or unexpected behavior carvel-triage This issue has not yet been reviewed for validity labels Dec 12, 2023
@alexbarbato
Copy link
Contributor Author

@alexbarbato
Copy link
Contributor Author

I started poking at this a bit today and noticed that when I change the reference above to 120 seconds from 30 seconds and then recompile vendir things do work for a vendir sync

But I couldn't come to a confident place of where the issue should be rectified:

  1. Maybe the offending repository needs to clear out the massive amount of tags they are building?
    • This is somewhat problematic as we do want to encourage folks to release frequently and often. I see the internal builds as well as release candidates.
  2. Maybe we just want to up the timeout limit and cut retries in vendir?
    • This is very brute force, not ideal feeling - but would solve the issue.
  3. Maybe we want to expose a timeout to the tag selection block in a Config file because this is a unique issue tied to tags?
    • This is debatably the most "right" approach if this is an issue that should be solved within vendir. Masking the underlying imgpkg configuration is not a foreign practice as you can skip TLS verification right now.

@praveenrewar
Copy link
Member

This is debatably the most "right" approach if this is an issue that should be solved within vendir. Masking the underlying imgpkg configuration is not a foreign practice as you can skip TLS verification right now.

+1, I think this is the most reasonable approach.
cc @kumaritanushree @joaopapereira

@renuy renuy added carvel-accepted This issue should be considered for future work and that the triage process has been completed discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel-triage This issue has not yet been reviewed for validity carvel-accepted This issue should be considered for future work and that the triage process has been completed labels Jan 2, 2024
@praveenrewar praveenrewar added carvel-accepted This issue should be considered for future work and that the triage process has been completed and removed discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution labels Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants