Skip to content

Forward porting Gloo service lookup by labels to 1.19.x - #10669

Merged
soloio-bulldozer[bot] merged 5 commits into
mainfrom
rolds/forwardport_gloo_service_lookup
Mar 4, 2025
Merged

Forward porting Gloo service lookup by labels to 1.19.x#10669
soloio-bulldozer[bot] merged 5 commits into
mainfrom
rolds/forwardport_gloo_service_lookup

Conversation

@ryanrolds

@ryanrolds ryanrolds commented Feb 27, 2025

Copy link
Copy Markdown

Description

Forward porting the Gloo service discovery changes made to 1.17.x. Added namespace argument to service lookup to make it easier to test.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@solo-changelog-bot

Copy link
Copy Markdown

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/7646

@ryanrolds

Copy link
Copy Markdown
Author

/kick

@ryanrolds
ryanrolds marked this pull request as ready for review February 27, 2025 22:26

@sam-heilbron sam-heilbron left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It may be worth it to check-in on #10667. The work you and @jjamroga doesn't step on eachother, but it may be related

@github-actions

github-actions Bot commented Feb 27, 2025

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 1033c97):

https://gloo-edge--pr10669-rolds-forwardport-gl-8edeuqjz.web.app

(expires Tue, 11 Mar 2025 16:33:53 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@jjamroga

Copy link
Copy Markdown
Member

It may be worth it to check-in on #10667. The work you and @jjamroga doesn't step on eachother, but it may be related

Had a chat with Ryan offline, we won't step on eachothers feet. Seems like the issues we're both solving for are only tangentially related.

@ryanrolds
ryanrolds requested a review from sam-heilbron March 4, 2025 18:17
WithContext(ctx).
WithTimeout(time.Second * 10).
WithPolling(time.Millisecond * 200).
WithTimeout(time.Second * 30).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm good with this change. I'm always a little nervous when we have timeouts that take 30 seconds (or some time frame beyond what I would expect for a change to propagate in a small environment) since those can be a signal of some other issue going on that we're masking.

@soloio-bulldozer
soloio-bulldozer Bot merged commit 3b19d23 into main Mar 4, 2025
@soloio-bulldozer
soloio-bulldozer Bot deleted the rolds/forwardport_gloo_service_lookup branch March 4, 2025 18:23
ymesika pushed a commit that referenced this pull request Apr 23, 2025
Signed-off-by: timflannagan <timflannagan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants