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

intereceptor service isn't created #755

Closed
trondhindenes opened this issue Aug 20, 2023 · 14 comments
Closed

intereceptor service isn't created #755

trondhindenes opened this issue Aug 20, 2023 · 14 comments
Labels
bug Something isn't working stale All issues that are marked as stale due to inactivity

Comments

@trondhindenes
Copy link

trondhindenes commented Aug 20, 2023

Report

According to the docs:

Each one will be named consistently like so, in the same namespace as the HTTPScaledObject and your application
I understand that an "interceptor service" will be created in the namespace where I create the HTTPScaledObject, and I should point my ingress to this service.

However, it also says:

"This is installed by the Helm chart as a ClusterIP Service by default"

This does not make sense to me - why would the Helm chart for the http-scaler have any knowledge about which namespace(s) I plan to install my applications?
It would be good if the documentation could clarify this

Further, when installing the Helm chart I can see a keda-add-ons-http-interceptor-proxy created in the keda namespace, but not in any "application" namespaces.

It would be good if the docs went into more detail of explaining the flow to make it easier to troubleshoot this.

Expected Behavior

I would expect a service called keda-add-ons-http-interceptor-proxy to be created in the namespace where the HttpScaledObject resource was created

Actual Behavior

the service in question is only available in the "keda" namespace

Steps to Reproduce the Problem

I just installed the '0.5.2' chart with defaults, and tried creating a HttpScaledObject resource in the default namespace.

Logs from KEDA HTTP operator

example

HTTP Add-on Version

0.5.0

Kubernetes Version

1.25

Platform

Amazon Web Services

Anything else?

No response

@JorTurFer
Copy link
Member

Hello,
The error was in the HTTP add-on code and chart. We have released today a new version (0.6.0) with this and other fixes, please give it a try and let me know if this has solved

@trondhindenes
Copy link
Author

great, thanks! will do!

@trondhindenes
Copy link
Author

trondhindenes commented Oct 6, 2023

WIth 0.6.0 I'm still not seeing an interceptor service created in the namespace where my service lives. Is that how this is supposed to work? Its really hard to troubleshoot this since I don't know what's supposed to happen.

@trondhindenes
Copy link
Author

Looking at the IngressRoute in this issue: #586 it looks like I should point to the interceptor service in the keda namespace. As far as I can see, that doesn't match the documentation

@JorTurFer
Copy link
Member

Hey,
Maybe it was a miscommunication, sorry :/
Currently, the interceptor server will be in the namespace where you deploy the add-on, there is another issue to spin up interceptors on demand per HTTPScaledObject, but currently there is a single interceptor workload

@JorTurFer
Copy link
Member

As far as I can see, that doesn't match the documentation

Which documentation (because that's not correct and we have to fix it)?

@trondhindenes
Copy link
Author

trondhindenes commented Oct 6, 2023

In the walkthru (https://github.com/kedacore/http-add-on/blob/main/docs/walkthrough.md#routing-to-the-right-service) it gives an impression that an interceptor service will be created in each application namespace:

you'll need to anticipate the name of these created Services. Each one will be named consistently like so, in the same namespace as the HTTPScaledObject and your application (i.e. $NAMESPACE)

As far as I can see, that's incorrect

It's also very hard to deduce how this is supposed to work from the example app, since https://github.com/kedacore/http-add-on/blob/main/examples/xkcd/values.yaml takes an ingressNamespace parameter (mentioned in the walk-thru) but that parameter just explicitly stamps the namespace onto the generated ingress object, which is something helm does implicitly by default, so there's really no need for it.

It would be good of the install/walkthru had snippets of "assertions" like "you should now see xyz in namespace xyz" - stuff like that makes it a lot easier to figure out where stuff went wrong, and pinpoint exactly when reality and documentation differs.

But: You're saiying that

there is another issue to spin up interceptors on demand

Which I interpret as "that is the planned behavior, but it's not ready yet" is that correct? If so I can make a PR to clean up the confusing helm charts and unclear/incorrect documentation. I'm guessing this means that you reccomend creating an ExternalName to represent the interceptor service in the app namespace, so that the actual routing becomes

ingress (app namespace) --> interceptor externalName service (app namespace) --> interceptor service (keda namespace)

Is that correct?

@github-project-automation github-project-automation bot moved this from To Triage to Done in Roadmap - KEDA HTTP Add-On Oct 6, 2023
@JorTurFer JorTurFer reopened this Oct 6, 2023
@JorTurFer
Copy link
Member

Ignore the closing, I did it by mistake when I was answering 🤦

@JorTurFer
Copy link
Member

Hi!

Which I interpret as "that is the planned behavior, but it's not ready yet" is that correct?

No no, it's a feature request, not a bug: #241. It's a nice to have in the future but also it's something that should be configurable, because some use cases can require that approach and other can require running all together for reducing costs.
The issue was there and I guess that's something we can try to push, but for the moment I think that current approach is good as starting point.

ingress (app namespace) --> interceptor externalName service (app namespace) --> interceptor service (keda namespace)

That's correct, you can do this (I recommend it) or deploy the ingress manifests in KEDA namespace, which is possible, but sounds weird. Both cases are covered by e2e tests: https://github.com/kedacore/http-add-on/tree/main/tests/checks

We have to improve the documentation to be more concise and clear, but if you are willing to open a PR clarifying these points, it'd be awesome

@trondhindenes
Copy link
Author

first and foremost: thanks for replying here, I really appreciate you taking the time to help me get a handle on how it's supposed to work. I think I understand a bit better now. And I would say that the documentation is vastly out of sync with reality here. I'd be happy to try and help address that with a doc PR or two at least.

@JorTurFer
Copy link
Member

it'd be nice. This project was staled when the principal contributor left it and we are trying to restart it. I think that HTTP scaling is a missing feature that we should to provide, but as you see, there are a lot of areas to improve, and definitively documentation is one of them.
Any help you provide is more than welcomed, so if you are willing to do it, go ahead ❤️

@trondhindenes
Copy link
Author

I agree, there's a lot of potential here. I'll try and come up with a doc improvement PR during the coming days!

Copy link

stale bot commented Dec 8, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Dec 8, 2023
Copy link

stale bot commented Dec 15, 2023

This issue has been automatically closed due to inactivity.

@stale stale bot closed this as completed Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale All issues that are marked as stale due to inactivity
Projects
Status: Done
Development

No branches or pull requests

2 participants