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

Set a maximum amount of time to wait for an image to be pulled #550

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

bormanp
Copy link
Collaborator

@bormanp bormanp commented Jul 18, 2024

after seeing ErrImagePull.

@bormanp bormanp requested a review from alexmasi July 18, 2024 14:28
@bormanp
Copy link
Collaborator Author

bormanp commented Jul 18, 2024

/gcbrun

Copy link

Pull Request Test Coverage Report for Build 9993408083

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 65.3%

Totals Coverage Status
Change from base Build 9864302534: 0.06%
Covered Lines: 4665
Relevant Lines: 7144

💛 - Coveralls

@@ -14,13 +14,18 @@ import (
log "k8s.io/klog/v2"
)

// MaxPullTime is how long we will give a pod to start after seeing
// ErrImagePull.
var MaxPullTime = time.Minute * 15
Copy link

Choose a reason for hiding this comment

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

15 minutes seems generous, but I suppose it's better than the current situation where it waits indefinitely!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made it public so it can be turned into a flag :-)

We have tests that fail if they take more than an hour. I decide 15 minutes was sufficient to a) it really probably will never succeed and b) short enough that it won't accidentally timeout a test.

@bormanp
Copy link
Collaborator Author

bormanp commented Jul 18, 2024

UPDATE 2: I had opened the "Details" link along side the check. It was of very little help. Looking at the diffs I see where the blank like was (not at the end of a file). Pushed the commit to remove that one newline.

UPDATE: It appears lint decided to start passing.

I am not sure why lint is complaining about an empty line. The only files whose last line is empty or consists solely of whitespace are:

./topo/node/cisco/testdata/push_config_success
./cloudbuild/vendors/topology.textproto
./cloudbuild/vendors/testbed.textproto
./cloudbuild/vendors/juniper.cfg
./examples/arista/ceos-150/arista.cfg
./examples/cisco/xrd/r2.config
./examples/cisco/xrd/r1.config
./examples/cisco/8000e/8000e.pb.txt
./examples/cisco/xrd-traffic/r2.config
./examples/cisco/xrd-traffic/r1.config
./examples/nokia/srlinux-services/2node-srl-ixr6-with-oc-services.pbtxt
./examples/nokia/srlinux-cert/2node-srl-with-cert.pbtxt
./examples/juniper/cptx-ixia/cptx-ixia.pb.txt
./manifests/base/kustomization.yaml
./.mk/kind.mk

None of those are Go files.

@bormanp
Copy link
Collaborator Author

bormanp commented Jul 18, 2024

The presubmit seems to be failing in TestMetalLBSpec which is unrelated to this change.

Looking at the "details" report of very little help.
@bormanp bormanp merged commit 1eb8cd5 into main Jul 18, 2024
9 checks passed
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.

3 participants