charts icon indicating copy to clipboard operation
charts copied to clipboard

fix(lancache): fix port assignments

Open jonaswre opened this issue 3 years ago • 7 comments

Description I couldn't get lancache to work because it seems like the ports were not configured the right way.

for lancache-dns the UDP port 53 was not setup. for lancache-monolith nether port 443 nor 80 was setup correctly.

⚙️ Type of change

  • [ ] ⚙️ Feature/App addition
  • [x] 🪛 Bugfix
  • [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] 🔃 Refactor of current code

🧪 How Has This Been Tested? Used it at a lan party. It didn't work without these changes.

✔️ Checklist:

  • [x] ⚖️ My code follows the style guidelines of this project
  • [x] 👀 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
  • [x] ⚠️ My changes generate no new warnings
  • [ ] 🧪 I have added tests to this description that prove my fix is effective or that my feature works
  • [x] ⬆️ I increased versions for any altered app according to semantic versioning

Please don't blindly check all the boxes. Read them and only check those that apply. Those checkboxes are there for the reviewer to see what is this all about and the status of this PR with a quick glance.

jonaswre avatar Sep 17 '22 15:09 jonaswre

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 17 '22 15:09 CLAassistant

Is it now acceptable?

And I have one more question. Whats standards need to be meet for an app to go to stable?

jonaswre avatar Sep 17 '22 15:09 jonaswre

Is it now acceptable?

Tbh I'd prefer instead of the label, to go to a docs file. Having same layout everywhere helps to do bulk changes if needed. And also I think I've heard people have had success with it, without changing the ports. So not 100% sure that this is needed.

And I have one more question. Whats standards need to be meet for an app to go to stable?

It needs to be verified that it works. Usually by 2-3 people

Also if that doesn't work with non 80 or 443 ports. It's kinda bummer. As most of the people will have traefik at those ports.

stavros-k avatar Sep 17 '22 15:09 stavros-k

Tbh I'd prefer instead of the label, to go to a docs file.

I understand that. I think it would be good to have the warning somewhere very prominent in the ui because if you install the app it wont work out of the box. Which kind of takes away the smooth ux. But that should not be a stopper.

Having same layout everywhere helps to do bulk changes if needed.

What kind of bulk changes do you do?

And also I think I've heard people have had success with it, without changing the ports. So not 100% sure that this is needed.

The ports 80 and 443 needs to be accessible. Lancache intersects the dns request and forwards them to the monolith. That can't change the port just the ip. The client (Steam, Battelnet...) request port 80/443 so this will need to be opened.

jonaswre avatar Sep 17 '22 16:09 jonaswre

What kind of bulk changes do you do?

Currently none, but if there needs to be a change in the future, apps with changed parts that are meant to be standardised will be left out.

The ports 80 and 443 needs to be accessible. Lancache intersects the dns request and forwards them to the monolith. That can't change the port just the ip. The client (Steam, Battelnet...) request port 80/443 so this will need to be opened.

Might worth it then using traefik for that, but that's your choice.

Also, you can use the internal DNS name instead of your TrueNAS IP (for lancache-dns communication). Internal DNS name will resolve to the pods IP which listens to 443 (and 80)

stavros-k avatar Sep 17 '22 16:09 stavros-k

Also: Still waiting for CLA signing....

I've done it twice and it redirects me back successfully but it's not reflected in the PR

jonaswre avatar Sep 18 '22 19:09 jonaswre

Also: Still waiting for CLA signing....

I've done it twice and it redirects me back successfully but it's not reflected in the PR

I can confirm!

PrivatePuffin avatar Sep 19 '22 11:09 PrivatePuffin

Any updates?

jonaswre avatar Sep 25 '22 14:09 jonaswre

Any updates?

No, you have to sign the CLA

stavros-k avatar Sep 25 '22 14:09 stavros-k

Also: Still waiting for CLA signing....

I've done it twice and it redirects me back successfully but it's not reflected in the PR

I can confirm!

I've signed that so many times now.

jonaswre avatar Sep 25 '22 14:09 jonaswre

I've checked again and it was signed with a diffrent e-mail. I've added that and rechecked it.

jonaswre avatar Sep 25 '22 14:09 jonaswre

Any updates?

No, you have to sign the CLA

I;ve already manually confirmed that he signed. In the end the CLA tool is a tool, my word overrules the tool ;-)

PrivatePuffin avatar Sep 25 '22 15:09 PrivatePuffin

This PR is locked to prevent necro-posting on closed PRs. Please create a issue or contact staff on discord if you want to further discuss this

truecharts-admin avatar Apr 04 '23 00:04 truecharts-admin