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

Allow use of headless hostname #69

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Conversation

angelini
Copy link
Contributor

@angelini angelini commented Dec 5, 2023

One issue with using dateilager.gadget-services.net to resolve a DL instance is that the K8S service will resolve this to a single instance IP.

If for wtv reason you fail to connect to that IP, for example during a DL deployment, it cannot recover by trying another instance because it has already been resolved to that fixed IP.

If instead we use a headless service, we can learn about all instances and then retry among different ones.

We already make use of this for our GRPC JS client, this just adds it to the binary client.

@angelini angelini force-pushed the allow_use_of_headless_hostname branch 2 times, most recently from 54eccb7 to 2317280 Compare December 5, 2023 15:26
@angelini angelini force-pushed the allow_use_of_headless_hostname branch from 2317280 to 2b15877 Compare December 5, 2023 15:31
@angelini
Copy link
Contributor Author

angelini commented Dec 5, 2023

I have 🎩 this from evil-ssh pod and I was able to connect to DL using the headless hostname and omitting it.

@angelini angelini force-pushed the allow_use_of_headless_hostname branch 2 times, most recently from dde4aa6 to 531b08a Compare December 5, 2023 15:45
Copy link

@thegedge thegedge left a comment

Choose a reason for hiding this comment

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

🚀

@@ -55,7 +55,8 @@ func NewClientConn(conn *grpc.ClientConn) *Client {
}

type options struct {
token string
headlessAdress string
Copy link

Choose a reason for hiding this comment

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

nit: typo on address

Copy link
Contributor

@MikePresman MikePresman left a comment

Choose a reason for hiding this comment

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

LGTM

@angelini angelini force-pushed the allow_use_of_headless_hostname branch from 531b08a to 3d56ef7 Compare December 5, 2023 16:17
@angelini angelini marked this pull request as ready for review December 5, 2023 16:17
@angelini angelini force-pushed the allow_use_of_headless_hostname branch from 3d56ef7 to 6445881 Compare December 5, 2023 17:25
@angelini angelini force-pushed the allow_use_of_headless_hostname branch from 6445881 to 959a047 Compare December 5, 2023 17:34
@angelini angelini merged commit 40889ab into main Dec 5, 2023
4 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