charts icon indicating copy to clipboard operation
charts copied to clipboard

fix(mineos): add docs and update chart.yaml

Open xstar97 opened this issue 3 years ago • 10 comments

Description updated the chart.yaml with a actual category value, added docs, and bumped app version.

⚒️ Fixes #

⚙️ 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?

📃 Notes:

✔️ 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
  • [X] 📄 I have made corresponding changes to the documentation
  • [ ] ⚠️ My changes generate no new warnings
  • [ ] 🧪 I have added tests to this description that prove my fix is effective or that my feature works
  • [ ] ⬆️ I increased versions for any altered app according to semantic versioning

➕ App addition

If this PR is an app addition please make sure you have done the following.

  • [ ] 🪞 I have opened a PR on truecharts/containers adding the container to TrueCharts mirror repo.
  • [ ] 🖼️ I have added an icon in the Chart's root directory called icon.png

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.

xstar97 avatar Jul 29 '22 00:07 xstar97

You also sneaked in actual minecraft ports while we got told this was just a GUI before?!

PrivatePuffin avatar Jul 29 '22 13:07 PrivatePuffin

You also sneaked in actual minecraft ports while we got told this was just a GUI before?!

Mineos is a gui that creates its own non docker minecraft servers and runs them...it needs all the same ports as any other mc server.

The ports for 25565 can be removed, but those ports are still default, i honestly don't see an issue using the same ports... no one will run both mineos and the non gui minecraft servers at the same time.

I understand port conflicting is a pain to deal with but some apps will share the same port and that's especially high for minecraft servers and gui counterparts like mineos.

xstar97 avatar Jul 29 '22 15:07 xstar97

I removed the conflicting ports and added docs instead to explain how to add additional ports.

xstar97 avatar Jul 31 '22 03:07 xstar97

What do you mean with "non-docker"? I sincerely hope it's not run on the host?

If it runs minecraft containers those ports need to be there. (its not about the conflicts at all, we don't prevent conflicts anymore)

PrivatePuffin avatar Jul 31 '22 06:07 PrivatePuffin

What do you mean with "non-docker"? I sincerely hope it's not run on the host?

If it runs minecraft containers those ports need to be there. (its not about the conflicts at all, we don't prevent conflicts anymore)

What do you mean with "non-docker"? I sincerely hope it's not run on the host?

If it runs minecraft containers those ports need to be there. (its not about the conflicts at all, we don't prevent conflicts anymore)

let me rephrase this. it doesnt create docker inside docker servers, it just runs the jar inside the single pod. no additional containers, docker, or pods are created even if multiple servers are used. yes obv, to make the chart work, some MC ports are needed, my assumption was that port conflict was still a thing from your comment that i sneaked it in and looking at all the minecraft charts all have different ports, only minecraft-java uses 25565 as default.

xstar97 avatar Jul 31 '22 07:07 xstar97

Tested mineos last few days and it doesn't run on the host, another user had tested it also and works as intended.

I had no issues with mineos even from truenas core.

Can i receive some instructions if i should add the ports back or the docs suffice? I have other PRs i would like to update.

xstar97 avatar Aug 01 '22 20:08 xstar97

I am one such user who has used this app, and I can confirm it is currently working as intended - I have a minecraft server running on it right now. As Xstar97 says, all the logic is contained within the pod and is not run on the host system, as with any other scale app I've used.

I would also say that when Ornias wrote "If it runs minecraft containers those ports need to be there", that means you @Xstar97 should indeed re-add the ports back into the PR. If you want more concrete affirmation on that before going ahead then fair enough, but from what I can see you're good to go with re-adding the ports.

hugocowan avatar Aug 01 '22 22:08 hugocowan

You can't mix UDP and TCP ports in the same service on TN Scale. iX has enabled the flag but it's only available in nightlies

thanks for the heads up, is this the rcon or just the tcp and udp ports for MC?

xstar97 avatar Aug 02 '22 23:08 xstar97

You can't mix UDP and TCP ports in the same service on TN Scale. iX has enabled the flag but it's only available in nightlies

thanks for the heads up, is this the rcon or just the tcp and udp ports for MC?

Only the places where you have TCP and UDP ports mixed in the same service. rcon has only tcp ports defined

stavros-k avatar Aug 02 '22 23:08 stavros-k

You can't mix UDP and TCP ports in the same service on TN Scale. iX has enabled the flag but it's only available in nightlies

thanks for the heads up, is this the rcon or just the tcp and udp ports for MC?

Only the places where you have TCP and UDP ports mixed in the same service. rcon has only tcp ports defined

ok, minecraft java only needs TCP ports, should i make a separate service like:

minecraft-java, minecraft-bedrock, minecraft-rcon

xstar97 avatar Aug 02 '22 23:08 xstar97

Please rebase. And review again please @stavros-k

PrivatePuffin avatar Aug 21 '22 08:08 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 09 '23 00:04 truecharts-admin