feat: Add missing fields to the describe output #915

LucasRoesler · 2022-02-26T10:45:03Z

Add the function

constraints
environment variables
secrets
requests and limits

to the faas-cli describe output.

Motivation and Context

I have raised an issue to propose this change (required)
Resolves Could we list secret names for functions on faas-cli describe? #913

How Has This Been Tested?

Create a test cluster using KinD

kind create cluster --config=cluster.yaml
arkade install openfaas -a=false --function-pull-policy=IfNotPresent --wait

faas-cli store deploy nodeinfo --annotation sample=true --label status=418

Now deploy a sample function

$ faas-cli describe nodeinfo
Name:                nodeinfo
Status:              Ready
Replicas:            1
Available replicas:  1
Invocations:         0
Image:               ghcr.io/openfaas/nodeinfo:latest
Function process:    <default>
URL:                 http://127.0.0.1:8080/function/nodeinfo
Async URL:           http://127.0.0.1:8080/async-function/nodeinfo
Labels               faas_function : nodeinfo
                     status : 418
Annotations          sample : true
                     prometheus.io.scrape : false
Constraints          <none>
Environment          <none>
Secrets              <none>
Requests             <none>
Limits               <none>

Now we add some more interesting values, like a secret and env variables.

$ faas-cli secret create db-password --from-literal=password
Creating secret: db-password.
Created: 202 Accepted

$ faas-cli store deploy nodeinfo --annotation sample=true --label status=418 --secret db-password --env db-user=postgres --env db-host=rds.aws.example.com

Deployed. 202 Accepted.
URL: http://127.0.0.1:8080/function/nodeinfo

$ faas-cli describe nodeinfo
Name:                nodeinfo
Status:              Ready
Replicas:            1
Available replicas:  1
Invocations:         0
Image:               ghcr.io/openfaas/nodeinfo:latest
Function process:    <default>
URL:                 http://127.0.0.1:8080/function/nodeinfo
Async URL:           http://127.0.0.1:8080/async-function/nodeinfo
Labels:              faas_function: nodeinfo
                     status: 418
                     uid: 736815901
Annotations:         prometheus.io.scrape: false
                     sample: true
Constraints:         <none>
Environment:         <none>
Secrets:              - db-password
Requests:            <none>
Limits:              <none>

To see how multiple Secrets and the Requests and Limits are rendered, use

version: 1.0
provider:
  name: openfaas
  gateway: http://127.0.0.1:8080
functions:
  nodeinfo:
    lang: Dockerfile
    image: ghcr.io/openfaas/nodeinfo:latest
    secrets:
      - cache-password
      - db-password
    environment:
      db-host: rds.aws.example.com
      cache-host: redis.aws.example.com
    labels:
      status: 481
    annotations:
      sample: "true"
    requests:
      cpu: 100m
      memory: 128Mi
    limits:
      cpu: 200m
      memory: 256Mi

then

$ faas-cli secret create cache-password --from-literal=password
Creating secret: cache-password.
Created: 202 Accepted
$ faas-cli deploy
Deploying: nodeinfo.

Deployed. 202 Accepted.
URL: http://127.0.0.1:8080/function/nodeinfo

$ faas-cli describe nodeinfo
Name:                nodeinfo
Status:              Ready
Replicas:            1
Available replicas:  1
Invocations:         0
Image:               ghcr.io/openfaas/nodeinfo:latest
Function process:    <default>
URL:                 http://127.0.0.1:8080/function/nodeinfo
Async URL:           http://127.0.0.1:8080/async-function/nodeinfo
Labels:              faas_function: nodeinfo
                     status: 481
                     uid: 679245186
Annotations:         prometheus.io.scrape: false
                     sample: true
Constraints:         <none>
Environment:         <none>
Secrets:             - cache-password
                     - db-password
Requests:            CPU: 100m
                     Memory: 128Mi
Limits:              CPU: 200m
                     Memory: 256Mi

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.

Add the function * constraints * environment variables * secrets * requests and limits This can be tested as follows: Create a test cluster using KinD ```sh kind create cluster --config=cluster.yaml arkade install openfaas -a=false --function-pull-policy=IfNotPresent --wait faas-cli store deploy nodeinfo --annotation sample=true --label status=418 ``` Now deploy a sample function ```sh $ faas-cli describe nodeinfo Name: nodeinfo Status: Ready Replicas: 1 Available replicas: 1 Invocations: 0 Image: ghcr.io/openfaas/nodeinfo:latest Function process: <default> URL: http://127.0.0.1:8080/function/nodeinfo Async URL: http://127.0.0.1:8080/async-function/nodeinfo Labels faas_function : nodeinfo status : 418 Annotations sample : true prometheus.io.scrape : false Constraints <none> Environment <none> Secrets <none> Requests <none> Limits <none> ``` Now we add some more interesting values, like a secret and env variables. ```sh $ faas-cli secret create db-password --from-literal=password Creating secret: db-password. Created: 202 Accepted $ faas-cli store deploy nodeinfo --annotation sample=true --label status=418 --secret db-password --env db-user=postgres --env db-host=rds.aws.example.com Deployed. 202 Accepted. URL: http://127.0.0.1:8080/function/nodeinfo $ faas-cli describe nodeinfo Name: nodeinfo Status: Ready Replicas: 1 Available replicas: 1 Invocations: 0 Image: ghcr.io/openfaas/nodeinfo:latest Function process: <default> URL: http://127.0.0.1:8080/function/nodeinfo Async URL: http://127.0.0.1:8080/async-function/nodeinfo Labels: faas_function: nodeinfo status: 418 uid: 736815901 Annotations: prometheus.io.scrape: false sample: true Constraints: <none> Environment: <none> Secrets: - db-password Requests: <none> Limits: <none> ``` To see how multiple Secrets and the Requests and Limits are rendered, use ```yaml version: 1.0 provider: name: openfaas gateway: http://127.0.0.1:8080 functions: nodeinfo: lang: Dockerfile image: ghcr.io/openfaas/nodeinfo:latest secrets: - cache-password - db-password environment: db-host: rds.aws.example.com cache-host: redis.aws.example.com labels: status: 481 annotations: sample: "true" requests: cpu: 100m memory: 128Mi limits: cpu: 200m memory: 256Mi ``` then ```sh $ faas-cli secret create cache-password --from-literal=password Creating secret: cache-password. Created: 202 Accepted $ faas-cli deploy Deploying: nodeinfo. Deployed. 202 Accepted. URL: http://127.0.0.1:8080/function/nodeinfo $ faas-cli describe nodeinfo Name: nodeinfo Status: Ready Replicas: 1 Available replicas: 1 Invocations: 0 Image: ghcr.io/openfaas/nodeinfo:latest Function process: <default> URL: http://127.0.0.1:8080/function/nodeinfo Async URL: http://127.0.0.1:8080/async-function/nodeinfo Labels: faas_function: nodeinfo status: 481 uid: 679245186 Annotations: prometheus.io.scrape: false sample: true Constraints: <none> Environment: <none> Secrets: - cache-password - db-password Requests: CPU: 100m Memory: 128Mi Limits: CPU: 200m Memory: 256Mi ``` Signed-off-by: Lucas Roesler <[email protected]>

alexellis · 2022-03-02T11:20:18Z

commands/describe.go

@@ -103,20 +104,11 @@ func runDescribe(cmd *cobra.Command, args []string) error {
 	url, asyncURL := getFunctionURLs(gatewayAddress, functionName, functionNamespace)

 	funcDesc := schema.FunctionDescription{


Why were these fields deleted?

Because i just embed the FunctionStatus now instead of copying each field one-at-a-time. This will ensure that the FunctionDescription always has access to the FunctionStatus fields

see https://github.com/openfaas/faas-cli/pull/915/files/20f8af79081c4b3e7d175804609fc4ebd541ca54#diff-cc6e60246691f8b3534c70829a1eaa3a123849240f9c046c2b46f5abf917be2cR10

Thanks for explaining.

alexellis · 2022-03-02T11:20:35Z

commands/describe.go

 	fmt.Fprintln(w, "URL:\t "+funcDesc.URL)
 	fmt.Fprintln(w, "Async URL:\t "+funcDesc.AsyncURL)
 	printMap(w, "Labels", *funcDesc.Labels)
 	printMap(w, "Annotations", *funcDesc.Annotations)
+	printList(w, "Constraints", funcDesc.Constraints)
+	printMap(w, "Environment", funcDesc.EnvVars)


How about "Environment Variables"?

The extra length is going to force the content even wider and add a lot of empty space to the other rows, which will make it harder to read.

It is always env or environment in yaml specs, so I think it is pretty clear what it means here.

Environment also aligns with the output from kubectl describe

alexellis · 2022-03-02T11:21:58Z

Looks good, but we now have quite a long output.

Could you update to make these only print text if there are values?

Constraints          <none>
Environment Variables          <none>
Secrets              <none>
Requests             <none>
Limits               <none>

alexellis

Overall looks good. Just want to iterate on the experience when most of the fields are empty.

LucasRoesler · 2022-03-02T11:52:11Z

Looks good, but we now have quite a long output.

Could you update to make these only print text if there are values?
Constraints          <none>
Environment Variables          <none>
Secrets              <none>
Requests             <none>
Limits               <none>

This is certainly possible, but I copied the behavior from the existing implementation. There must have been a good reason for including the fields using <none> instead of omitting it. I assumed that including it makes the output more deterministic and explicit, so it, for example, if someone is copy/pasting this output and a field is missing it is then clear that the copy was incorrect.

alexellis · 2022-03-16T17:30:43Z

I think you're right, can we add a --verbose flag, and hide some of the other fields like limits / requests / constraints when that's set to false?

The printer object accepts a parameter to control how verbose it is. When verbose is false, empty values will be omitted from the describe output. Signed-off-by: Lucas Roesler <[email protected]>

LucasRoesler · 2022-03-16T21:12:11Z

commands/describe_test.go

+				Status: "Ready",
+			},
+			verbose:        false,
+			expectedOutput: "Name:\tfiglet\nStatus:\tReady\nReplicas:\t0\nAvailable Replicas: 0\nInvocations:\t0\nImage:\topenfaas/figlet:latest\nFunction Process:\t<default>\n",


@alexellis what do you think of this? these two test cases demonstrate the kind of minimal and maximal cases

Nicely done. The only thing I might suggest is making this a block quote to make it easier to read, but there are pros and cons to that too. We can merge as is, thanks for adding the tests.

alexellis · 2022-03-17T08:50:27Z

commands/describe.go

+		return
+	}
+
+	if usage == nil {


I would probably not print this message, since for anyone who's not on openfaas pro, they will see this all the time, so we could just omit printing it.

alexellis · 2022-03-17T08:52:35Z

Trying this locally, I am seeing a whitespace difference on the first few fields then on the next few after that?

Ensure that the values for maps and slices are aligned with the string values. Signed-off-by: Lucas Roesler <[email protected]>

alexellis · 2022-03-17T09:03:19Z

Function Process:   ./derek
URL:                https://derek.openfaas.com/function/derek-0114
Async URL:          https://derek.openfaas.com/async-function/derek-0114
Environment:         debug: true
 read_timeout: 20s
 validate_customers: true
 combine_output: true

With the latest change, I am seeing a bit of an inconsistency with the environment variable printing

I'd expect it to look more like:

Environment:
 debug: true
 read_timeout: 20s
 validate_customers: true

I'm seeing the same on secrets:

Secrets:             - derek-secret-key
 - derek-private-key

Add the header `Usage:` for the usage data to help it fit with the other fields better Signed-off-by: Lucas Roesler <[email protected]>

alexellis

LGTM

LucasRoesler requested a review from alexellis February 26, 2022 10:45

LucasRoesler force-pushed the feat-913-add-secrets-to-describe-output branch from 65bc24c to 20f8af7 Compare February 26, 2022 10:45

This was referenced Feb 26, 2022

bug: function info endpoint does not return the env variables openfaas/faas-netes#922

Open

fix: return full status object from the info endpoint openfaas/faas-netes#923

Open

alexellis reviewed Mar 2, 2022

View reviewed changes

alexellis requested changes Mar 2, 2022

View reviewed changes

feat: create dynamic printer for the function describe output

879b853

The printer object accepts a parameter to control how verbose it is. When verbose is false, empty values will be omitted from the describe output. Signed-off-by: Lucas Roesler <[email protected]>

LucasRoesler commented Mar 16, 2022

View reviewed changes

alexellis reviewed Mar 17, 2022

View reviewed changes

fix: remove extra spaces from the describe output

d7d4a95

Ensure that the values for maps and slices are aligned with the string values. Signed-off-by: Lucas Roesler <[email protected]>

fix: add section header for usage data in describe output

02d8fc8

Add the header `Usage:` for the usage data to help it fit with the other fields better Signed-off-by: Lucas Roesler <[email protected]>

LucasRoesler requested a review from alexellis March 17, 2022 13:32

alexellis approved these changes Mar 17, 2022

View reviewed changes

alexellis merged commit 17baca4 into openfaas:master Mar 17, 2022

		@@ -103,20 +104,11 @@ func runDescribe(cmd *cobra.Command, args []string) error {
		url, asyncURL := getFunctionURLs(gatewayAddress, functionName, functionNamespace)

		funcDesc := schema.FunctionDescription{

feat: Add missing fields to the describe output #915

feat: Add missing fields to the describe output #915

Uh oh!

Conversation

LucasRoesler commented Feb 26, 2022

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexellis commented Mar 2, 2022

Uh oh!

alexellis left a comment

Choose a reason for hiding this comment

Uh oh!

LucasRoesler commented Mar 2, 2022

Uh oh!

alexellis commented Mar 16, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexellis commented Mar 17, 2022

Uh oh!

alexellis commented Mar 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexellis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexellis commented Mar 17, 2022 •

edited

Loading