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

Support function resolution in multiple namespaces #683

Conversation

LucasRoesler
Copy link
Member

Description

What

  • Ensure that the pass only the function name to the Endpoint lister,
    because the namespace is already set when constructing the lister.
    The existing code would always return a 404 because it sent the
    "function_name.namespace" as the function name. This can never be
    found since it is not a valid service name
  • Update the error messages to make the component parts of the message
    easier to read. It will now be easier to see which part is the error from the
    k8s api.

Motivation and Context

  • I have raised an issue to propose this change (required)

Resolves openfaas/faas#1565

How Has This Been Tested?

The unit test for the proxy resolver has been updated to catch this case and it now passes.

It has also been tested locally using

kind create cluster

# doing this will cause extras and openfaas-fn to show in the UI
kubectl create ns extras
kubectl annotate ns openfaas-fn  openfaas=1
kubectl annotate ns extras  openfaas=1

arkade install openfaas --basic-auth=false --direct-functions=false --operator --clusterrole --set operator.image=theaxer/faas-netes:6802054

# in a separate terminal
kubectl port-forward svc/gateway 8080

open http://localhost:8080 and use the UI to deploy and invoke the function

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

**What**
- Ensure that the pass only the function name to the Endpoint lister,
  because the namespace is already set when constructing the lister.
  The existing code would always return a 404 because it sent the
  "function_name.namespace" as the function name. This can never be
  found since it is not a valid service name
- Update the error messages to make the component parts of the message
  easier to read.  It will now be easier to see which part is the error
  from the k8s api.

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
@amalkh5
Copy link

amalkh5 commented Aug 31, 2020

@LucasRoesler I've tested these changes and it working as expected.

Screenshot 2020-08-31 at 11 17 39 pm

@LucasRoesler
Copy link
Member Author

Thanks 💪

@alexellis
Copy link
Member

Thank you @amalkh5

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@alexellis alexellis merged commit 4cbd76b into openfaas:master Sep 17, 2020
@LucasRoesler LucasRoesler deleted the fix-provider-function-proxy-with-multi-namespace branch September 4, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invoke function in the UI is not working when direct_function is set to false
3 participants