-
Notifications
You must be signed in to change notification settings - Fork 217
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
Populate the memory limits of functions in the REST API #206
Conversation
…n to k8s resource value and return it through the REST API. Signed-off-by: Shikachuu <[email protected]>
Signed-off-by: Shikachuu <[email protected]>
In the testing instructions, can you give us a YAML file or equivalent that will deploy a function with memory limits? |
I've left some comments for you and would like both the input for the scenario and the output in step 8 to be added to show what came back. Here's a potential input you could use: functions:
figlet:
skip_build: true
image: ghcr.io/openfaas/figlet:latest
limits:
memory: 20Mi |
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.
I've left a few comments, but this is shaping up nicely.
Signed-off-by: Shikachuu <[email protected]>
…d error message. Signed-off-by: Shikachuu <[email protected]>
Signed-off-by: Shikachuu <[email protected]>
Duly noted, PR updated with example function + example responses! |
If you still have the environment running, can you test with My example with figlet uses |
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.
👏 Great Pull Request and testing. Thank you.
I've left one comment for an end to end test scenario I'd like you to run before I merge this.
Signed-off-by: Shikachuu <[email protected]>
@Shikachuu would you like to comment on or resolve the conversations above? |
No, thanks, I think I am a-okay with this implementation. |
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.
LGTM
I'm sorry that I was vague. That was me asking you to review and action the feedback. I'll try to be more direct in the future. |
@nitishkumar71 @LucasRoesler can we work this functionality into the tests of the certifier? |
Sure, I will work on it. |
Signed-off-by: Shikachuu [email protected]
Description
Add memory limits to the response json file, if you query functions through the API.
Motivation and Context
How Has This Been Tested?
sudo systemctl stop faasd faasd-provider
make dist
/usr/local/bin/faasd
with the one that we compiled in the previous stepsudo mv bin/faasd /usr/local/bin/faasd
sudo chown root /usr/local/bin/faasd
sudo systemctl start faasd faasd-provider
FAASDPW=$(sudo cat /var/lib/faasd/secrets/basic-auth-password ; echo)
faas-cli deploy -f ./fn.yml
jq
to see the resultcurl -s http://admin:$FAASDPW@localhost:8080/system/functions | jq
Example output:
Or you can query just for the limits object:
curl -s http://admin:$FAASDPW@localhost:8080/system/functions | jq .[0].limits
Example output:
Types of changes
Checklist:
Commits:
git commit -s
for the Developer Certificate of Origin (DCO)Code:
Docs: