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

faasd does not provide memory limits of functions in Canonicalize format #228

Open
1 of 2 tasks
nitishkumar71 opened this issue Dec 16, 2021 · 7 comments
Open
1 of 2 tasks

Comments

@nitishkumar71
Copy link
Member

nitishkumar71 commented Dec 16, 2021

All the openfaas implementation should return the memory info in the Canonicalize format.

Expected Behaviour

faasd should return memory limits in Canonicalize i.e. 5000000 should be returned as 5M. We follow the same into faas-netes.

Current Behaviour

faasd Provides function memory limit in bytes.

Are you a GitHub Sponsor (Yes/No?)

Check at: https://github.com/sponsors/openfaas

  • Yes
  • No

List all Possible Solutions

We can follow the implementation for k8s

https://github.com/openfaas/faas-netes/blob/master/pkg/k8s/function_status.go#L38

https://github.com/openfaas/faas-netes/blob/master/vendor/k8s.io/api/core/v1/resource.go#L34

List the one solution that you would recommend

Steps to Reproduce (for bugs)

  1. We can use certifier with the current release of faasd as done in the pipeline https://github.com/openfaas/certifier/runs/4542572124?check_suite_focus=true#step:5:138
@alexellis
Copy link
Member

Could you submit a PR for initial review and testing?

Alex

@nitishkumar71
Copy link
Member Author

I will work on this.

@Shikachuu
Copy link
Contributor

Shikachuu commented Dec 18, 2021

Sorry, I think this issue is my bad 😓 I should have written a test case for the SI conversion aswell.

Anyways @nitishkumar71 this is the problematic line that you looking for:
https://github.com/openfaas/faasd/blob/master/pkg/provider/handlers/read.go#L41

There was a conversation about this on the PR aswell, you might find something useful there:
#206 (comment)

Since the memory limit itself is working, only the format breaks.

@alexellis
Copy link
Member

So we put in: 1Mi
And get out: 1048576

For this import "k8s.io/apimachinery/pkg/api/resource"

What do these calls generate?

limitBytes:=1048576
resource.NewQuantity(limitBytes, resource.BinarySI)
resource.NewQuantity(limitBytes, resource.DecimalSI)

@nitishkumar71
Copy link
Member Author

So just checked below lines in playground

	limitBytes1 := int64(1048576)
	fmt.Println(resource.NewQuantity(limitBytes1, resource.BinarySI))
	fmt.Println(resource.NewQuantity(limitBytes1, resource.DecimalSI))

	limitBytes2 := int64(5000000)
	fmt.Println(resource.NewQuantity(limitBytes2, resource.BinarySI))
	fmt.Println(resource.NewQuantity(limitBytes2, resource.DecimalSI))

and the output we received is


1Mi
1048576

5000000
5M

@alexellis
Copy link
Member

Is that ordering correct in the output?

@nitishkumar71
Copy link
Member Author

Yes.

Including playground link.

https://go.dev/play/p/r7J2X3pOT7g

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

No branches or pull requests

3 participants