metakgp / metakgp-wiki

Dockerized source for the metakgp wiki.
https://wiki.metakgp.org
GNU General Public License v3.0
23 stars 20 forks source link

Added Ingress specific changes (backwards compatible) #96

Closed grapheo12 closed 9 months ago

grapheo12 commented 3 years ago

After @thealphadollar 's changes to ingress, there were some problems which I corrected.

After testing, I came up with this simple config:

Now we do not need to specify ports for wiki. It can listen to port 80. However, we need a fixed container name (which this PR fixes) and a fixed network name (also ensured by this PR and partly ensured by the naming convention of docker-compose).

See the current ingress docker-compose for the use of metakgp-wiki_nginx-network.

This makes the changes backwards compatible, meaning, even without Ingress, wiki would run without any change to the docker-compose file. Just then port forwarding has to be done manually.

If this is merged, #95 would become stale.

grapheo12 commented 3 years ago

@icyflame @amrav @thealphadollar Please review.

grapheo12 commented 3 years ago

Can you elaborate what you are trying to say here? We should avoid any manual step (even one-time steps) to ensure that we can bring the wiki up from scratch with just docker-compose.

I meant just as you restart the containers now, after this PR, there would be almost no change to how that happens. Only change you need to do, if Ingress isn't being used, is to specify which port to forward to in the command line with -p8000:80 etc. flags.

grapheo12 commented 3 years ago

In our current plan, there is only one port exposed to the host, and that is the port 80 of Ingress. Instead of leaving different ports of Wiki and IQPS open in the host environment, we are using Docker networks for the containers to connect with each other.

Please check out: https://github.com/grapheo12/ingress/commit/a069047b4a900f47dc1734c6e29e91e2c2e78a8b for the recent changes in the working of Ingress.

If you think this might not be a good idea, do tell us what to change.

icyflame commented 3 years ago

@grapheo12 Yes, sorry, I don't think it's a good idea to make the addition of a command line option required. This requires quite a bit of explanation and I would like to avoid those kind of gotchas if possible.

This is my proposal:

  1. Host port exposed to the world is only port 80 for HTTPs, 443 for SSH
  2. All other host ports are blocked to world using UFW
  3. The nginx container inside metakgp-wiki exposes it's port 80 to host port 8001
  4. The nginx container inside iqps exposes it's port 80 to host port 8002 https://github.com/grapheo12/iqps/blob/master/docker-compose.yml.template#L8-L17
  5. Ingress's nginx container connects to port 80, 3001, 3002 on the host
  6. Incoming traffic on port 80 is proxy passed to the localhost's 3001 port in case it's wiki.metakgp.org request https://github.com/grapheo12/ingress/blob/a069047b4a900f47dc1734c6e29e91e2c2e78a8b/wiki.conf#L12
  7. Incoming traffic on port 80 is proxy passed to the localhost's 8002 port in case it's iqps.metakgp.org request https://github.com/grapheo12/ingress/blob/master/iqps.conf#L12

Here's a list of pros and cons I could think of. I can't think of any other cons, please add any that of 🙇‍♂️

Pros Cons
Host is safe as the only outward facing port is 80 UFW has to be set-up properly
Adding a new service can be done by adding a new conf to the nginx conf inside ingress
We don't depend on host names on the docker network being the same
We don't have to keep docker network names in sync across wiki, iqps, ingress to ensure routing. This is ensured by port numbers alone which will be kept in sync between wiki and ingress OR iqps and ingress
All configuration is inside each repo's docker-compose files; no command line options are required
Local testing is easy, simply connect to localhost:8001 etc on local PC

What do you think about this approach?

grapheo12 commented 3 years ago

This was what Ingress initially did. And then we moved to docker networks. We then have to rollback ingress. :sweat_smile:

thealphadollar commented 3 years ago

@icyflame Do you think it is pro for the network that they ingress cannot be started without wiki and iqps containers running? I think this loose coupling will help avoid issues.

One con, a trivial one for our case, for networks is that bridge networks are slower compared to host network.

Anyways, it was my suggestion to use docker networks since I preferred to not expose ports to the system for this loose coupling and in general to avoid usage of ports that could be used by some other application. My apologies @grapheo12 for increasing the work for you.

I especially like the tests part and then these containers can be run independently as well. Can we please roll the docker-networks back? I'm really sorry for the extra work you had to do :cry:

icyflame commented 3 years ago

Ah, I see, I didn't follow the conversation closely about moving to docker networks closely. Sorry about the back and forth on this, Shubham! 🙇‍♂️


Do you think it is pro for the network that they ingress cannot be started without wiki and iqps containers running?

The ingress can be started, it won't work because the underlying service is not up but the ingress will be up and do it's job. Essentially, the ingress, wiki and iqps are completely independent.

  1. Without the ingress, no traffic from world can get in ✅
    1. This is very useful; during maintenance, we can simply bring the network down and forward host port 3001 to local port 3001 for testing 😌
  2. With ingress, world traffic will be proxy_passed to the appropriate port
    1. If the destination service (wiki, iqps, etc) is down, ingress doesn't really care about this and will not be affected by it (Decoupling)

I think this loose coupling will help avoid issues.

Yes, exactly 👍


Yes, I agree with your assessment that this will not affect us significantly.

image

I can't tell what unit the latency is in here, but either way, it doesn't matter because the difference is about 10-20% 🙆‍♂️

harshkhandeparkar commented 9 months ago

No longer relevant.