faasd icon indicating copy to clipboard operation
faasd copied to clipboard

faasd does not provide memory limits of functions in Canonicalize format

Open nitishkumar71 opened this issue 4 years ago • 7 comments

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
  • [x] 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

nitishkumar71 avatar Dec 16 '21 14:12 nitishkumar71

Could you submit a PR for initial review and testing?

Alex

alexellis avatar Dec 16 '21 18:12 alexellis

I will work on this.

nitishkumar71 avatar Dec 17 '21 02:12 nitishkumar71

Sorry, I think this issue is my bad :sweat: 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: https://github.com/openfaas/faasd/pull/206#discussion_r713779617

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

Shikachuu avatar Dec 18 '21 17:12 Shikachuu

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)

alexellis avatar Dec 22 '21 16:12 alexellis

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

nitishkumar71 avatar Dec 22 '21 17:12 nitishkumar71

Is that ordering correct in the output?

alexellis avatar Dec 22 '21 21:12 alexellis

Yes.

Including playground link.

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

nitishkumar71 avatar Dec 23 '21 05:12 nitishkumar71