-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Fix: Add kubernetes namespace to function instance url #4701
Fix: Add kubernetes namespace to function instance url #4701
Conversation
@cckellogg please also take look |
@@ -993,6 +993,10 @@ private static String createJobName(String tenant, String namespace, String func | |||
return "pf-" + tenant + "-" + namespace + "-" + functionName; | |||
} | |||
|
|||
private static String getServiceUrl(String jobName, String jobNamespace, int instanceId) { | |||
return String.format("%s-%d.%s.%s", jobName, instanceId, jobName, jobNamespace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does there need to be svc.cluster.local
after the namespace? https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/#namespaces-and-dns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That format worked when I tested in minikube
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe by default "svc.cluster.local" is added on if not specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add it since that is what is in the documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think either should work.
retest this please |
run cpp tests |
### Motivation Currently, if the kubernetes namespace set to deploy functions in is different than the one in which brokers/workers reside, get status and stats doesn't work because the url for instances does not specify the namespace.
### Motivation Currently, if the kubernetes namespace set to deploy functions in is different than the one in which brokers/workers reside, get status and stats doesn't work because the url for instances does not specify the namespace. (cherry picked from commit 8c3445a)
Motivation
Currently, if the kubernetes namespace set to deploy functions in is different than the one in which brokers/workers reside, get status and stats doesn't work because the url for instances does not specify the namespace.