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

talk - fix eternal relay-ip #3217

Merged
merged 2 commits into from
Aug 21, 2023
Merged

talk - fix eternal relay-ip #3217

merged 2 commits into from
Aug 21, 2023

Conversation

szaimen
Copy link
Collaborator

@szaimen szaimen commented Aug 21, 2023

Follow-up to #2855

cc @sando38

Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen szaimen added 3. to review Waiting for reviews bug Something isn't working labels Aug 21, 2023
@szaimen szaimen added this to the next milestone Aug 21, 2023
@szaimen szaimen requested a review from Zoey2936 August 21, 2023 16:52
Comment on lines 22 to 23
IPv4_ADDRESS_TALK="$(dig nextcloud-aio-talk IN A +short | grep '^[0-9.]\+$' | sort | head -n1)"
IPv6_ADDRESS_TALK="$(dig nextcloud-aio-talk AAAA +short | grep '^[0-9a-f:]\+$' | sort | head -n1)"
Copy link

Choose a reason for hiding this comment

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

Are these two always the container IP addresses?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

Copy link

Choose a reason for hiding this comment

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

Gotcha. In that case, I propose additional changes in the upcoming comment.

@sando38
Copy link

sando38 commented Aug 21, 2023

Follow up (#3217 (comment))

I now understand your setup better, it wasn't totally clear for me. As you relay internally between eturnal and your talk component, you do not need a publicly facing IP address as a relay address. I run a similar setup in a kubernetes setup ;)

Therefore, I suggest to eliminate the STUN lookup as well from your Dockerfile, use the 2nd option from here #2855 (comment)

The STUN query would return an external IP address, which will let your setup fail I guess in 99,99% of the cases, because your container is not reachable publicly on the relay port range 49152-65535.

P.S. And you can leave away the ENV STUN_SERVICE=stun.nextcloud.com 443 within your Dockerfile as well.

Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen
Copy link
Collaborator Author

szaimen commented Aug 21, 2023

Thank you @sando38! :)
Please have a look at 2868d45

Copy link
Collaborator

@Zoey2936 Zoey2936 left a comment

Choose a reason for hiding this comment

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

From my side it looks good, but let's wait what @sando38 says

@sando38
Copy link

sando38 commented Aug 21, 2023

Thank you @sando38! :) Please have a look at 2868d45

Looks fine from my side.

@szaimen
Copy link
Collaborator Author

szaimen commented Aug 21, 2023

Thank you both! :)

@szaimen szaimen merged commit 9200cb3 into main Aug 21, 2023
4 checks passed
@delete-merged-branch delete-merged-branch bot deleted the enh/noid/fix-talk branch August 21, 2023 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants