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

Add a new type of ingress for CQRS #53

Open
vaslabs opened this issue Jan 21, 2020 · 7 comments
Open

Add a new type of ingress for CQRS #53

vaslabs opened this issue Jan 21, 2020 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@vaslabs
Copy link
Owner

vaslabs commented Jan 21, 2020

Gives value to the current community nginx ingress controller by taking responsibility of the configuration complexity to achieve splitting service traffic per http method call

See possible implementation example: kubernetes/ingress-nginx#187 (comment)

@vaslabs vaslabs added the enhancement New feature or request label Jan 21, 2020
@vaslabs vaslabs modified the milestone: 0.3.0 Jan 21, 2020
@vaslabs vaslabs added this to To do in Kubernetes utilities via automation Jan 21, 2020
@vaslabs
Copy link
Owner Author

vaslabs commented Jan 22, 2020

This should only support the case where the codebase is the same (e.g. the same module) as if the project is already split in two, 2 different ingresss configurations can be given explicitly by the user and support different paths.

The case to solve is:

  1. We have one module
  2. The module splits in two deployments and two services but with the same docker image
  3. There are two sets of environment variables one for each deployment.
  4. There is 1 ingress definition that on enabling requestMethodBasedCQRS splits the single deployment configuration in two (one read, one write) and the same for the service adding the configuration to point with the read/write paths respectively plus the nginx configuration.
  5. The environment variables are over-written or appended by user input for each deployment.
  6. The end result is 5 files two deployment yaml with 2 yaml definitions (a yaml stream), 2 for service that follows the same rules and one for the single ingress rule.

Optionally give the user the ability to have 3 files in case they want to keep the deployment in one single kubectl command but this is not necessary for this ticket

@vaslabs vaslabs added this to the v0.4 milestone Feb 25, 2020
@VGerris
Copy link

VGerris commented Apr 27, 2021

Although that is a great idea, why not simply add add either a CRD like VirtualServer in nginx-ingress :
https://docs.nginx.com/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/#
or something that just extends that path matching with HTTP verbs like in the above, so the ingress can just have:

path: /api/clients
  • matches:
    • action:
      pass: api-read-service
      conditions:
      • value: GET
        variable: $request_method

and the same for write service for POST.
with an implementation like that, it's generic and can be extended with PUT for example for updates.
Regardless of implementation, I think this kind of functionality is much asked for and should really be part of it.

@vaslabs
Copy link
Owner Author

vaslabs commented Apr 27, 2021

The problem for me at the moment is that I'm not using this in production for the time being.

I need to look more into your suggestion but if you'd like you could submit a PR especially if u have the means to test it in a production environment.

@VGerris
Copy link

VGerris commented Apr 28, 2021

I was able to make the snippet work in the referenced post and on a tight time schedule so I went for that on the short term.
The challenges I see there are that of maintenace of the location when multiple URLs are added.
In load balancers that support verb based filtering, this is trivial, because it's simply done per application.
But if documentation/remarks are correct regarding it only applying to one host, we would have one file growing and needing to increasingly more parse added URLS.
If I find some time I will look into how nginxinc did it, I hope another dev that has more clue about this can pick it before that because I have never looked at the source as of now :).

Just to stress it agian, I think this kind of functionality is very important to have because it's a serious pain to script it and maintain it as such. We want the application to contain all information for itself to be routed to.
I would be happy with a 'clone' of the https://docs.nginx.com/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/# .
Any takers :)?

@vaslabs
Copy link
Owner Author

vaslabs commented Apr 28, 2021

I agree that is important to have something out of the box here, but this is just a plugin for sbt. It would give additional value to scala projects. Unfortunately as I said I don't have access to an environment to test this directly but if people volunteer to at least test it I can add the feature.

My plan was to have some flag to mark a module as CQRS and it would split into 2 service definitions and 2 pods adding the ingress to divert traffic.

@VGerris
Copy link

VGerris commented Apr 29, 2021

hi, you mean test your implementation? Easy to spin up a microk8s or minikube. I can test it. Your plan sounds good for this specific case, although I believe that just have a verb parameter to filter on will make it more generic and likely to be accepted?
I'm considering the PUT verbs for updates, so if you split you would have to be opiniated in the code about that, while it may be more useful to let users define their own preferred implementation on that level. Just my 2 cents

@vaslabs
Copy link
Owner Author

vaslabs commented Apr 29, 2021

this plugin is a bit opinionated, and it is driven by my and team's experiences but in most cases it does allow the user to alter configuration. Segregating reads from writes can be done purely on verb given that the user is using the verbs properly (might be a bit tricky for OPTION) .

For testing I can do it, but I'm not gonna use it in production. I guess I can get away by marking the feature as experimental.

In any case I don't see me working on this soon, maybe will start end of May, but thanks for your ideas, I will think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants