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

Orchestrator: add flag to skip the availability check on startup #2928

Merged
merged 4 commits into from Dec 14, 2023

Conversation

stronk-dev
Copy link
Contributor

What does this pull request do? Explain your changes. (required)
When booting an Orchestrator, an availability check is performed by sending a Ping GRPC request to the configured service address
On certain setups local loopback might not be available, causing the test to fail even though the Orchestrator is available from remote connections

This change adds a testOrchAvail flag to explicitly disable the startup check

How did you test each of these updates (required)
Going to use the binaries built by this PR to test this on my Singapore node, which exhibits this issue

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Merging #2928 (588bdbd) into master (37447ea) will decrease coverage by 0.01354%.
The diff coverage is 17.64706%.

❗ Current head 588bdbd differs from pull request most recent head bde3974. Consider uploading reports for the commit bde3974 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #2928         +/-   ##
===================================================
- Coverage   56.35345%   56.33991%   -0.01354%     
===================================================
  Files             89          89                 
  Lines          19454       19464         +10     
===================================================
+ Hits           10963       10966          +3     
- Misses          7882        7889          +7     
  Partials         609         609                 
Files Coverage Δ
cmd/livepeer/livepeer.go 50.74627% <100.00000%> (+0.74627%) ⬆️
cmd/livepeer/starter/starter.go 4.70685% <0.00000%> (-0.02737%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37447ea...bde3974. Read the comment docs.

Files Coverage Δ
cmd/livepeer/livepeer.go 50.74627% <100.00000%> (+0.74627%) ⬆️
cmd/livepeer/starter/starter.go 4.70685% <0.00000%> (-0.02737%) ⬇️

@stronk-dev
Copy link
Contributor Author

Currently testing on Singapore. Flag seems to have worked, let's see if work still comes in...

@thomshutt
Copy link
Contributor

Awesome, thanks @stronk-dev! Let me know once you're happy with the testing and we can merge

@stronk-dev
Copy link
Contributor Author

Seems fine so far, any issues with the naming or comments? Wasn't sure what to group the new flag as

@thomshutt
Copy link
Contributor

Naming looks good, the only bit I'd maybe change would be the flag itself to be the full version - startupAvailabilityCheck or testOrchAvailability? Internal variables etc. can stay as they are though

Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @stronk-dev Looks good to me.

Agree with @thomshutt that we could rename the flag. Other than that LGTM. @thomshutt Feel free to merge this PR.

@thomshutt
Copy link
Contributor

LGTM @stronk-dev, could you update the pending changelog please and then I'll merge

@thomshutt thomshutt merged commit 002e602 into master Dec 14, 2023
9 of 15 checks passed
@thomshutt thomshutt deleted the md/feature/skipPing branch December 14, 2023 15:28
@thomshutt thomshutt mentioned this pull request Jan 29, 2024
eliteprox pushed a commit to eliteprox/go-livepeer that referenced this pull request Feb 21, 2024
…epeer#2928)

* Orchestrator: add flag to skip the availability check on startup

* Edit name of startup test param

* Mod pending changelog

---------

Co-authored-by: Thom Shutt <thomshutt@users.noreply.github.com>
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.

None yet

3 participants