fix: return full status object from the info endpoint#923
fix: return full status object from the info endpoint#923LucasRoesler wants to merge 2 commits intoopenfaas:masterfrom
Conversation
Ensure that all fields of the `FunctionStatus` are populated based on the function `Deployment`. In particular, this change ensures that the env variables, constrains, and the read-only root filesystem values are populuated correctly. Signed-off-by: Lucas Roesler <[email protected]>
pkg/k8s/function_status.go
Outdated
| } | ||
|
|
||
| func hasReadOnlyRootFilesystem(function appsv1.Deployment) bool { | ||
| securityContext := function.Spec.Template.Spec.Containers[0].SecurityContext |
There was a problem hiding this comment.
Is there a chance that the Istio sidecar could take position 0?
There was a problem hiding this comment.
Could it be indexed upon some kind of property or name in a range loop?
There was a problem hiding this comment.
Could SecurityContext ever be nil?
There was a problem hiding this comment.
Yes, it can be nil, but this is checked immediately on the next line.
There was a problem hiding this comment.
regarding the position of the container:
Both the controller and the handler put the labels on the Pod, but not the container. We can use the container Name though, it will equal the deployment.Name.
As a side note, it would be really nice if we could refactor the an unify the generation of the Deployment, the function Factory doesn't actually generate it, both the controller and the handler have different code for the construction of the Deployment
There was a problem hiding this comment.
@alexellis i have updated the code and replaced all instances of Containers[0] with a proper FunctionContainer() implementation that finds the correct index and container spec.
aafc518 to
5795ba5
Compare
Add a utility method for correctly finding the index of the function Container inside a Deployment spec. Signed-off-by: Lucas Roesler <[email protected]>
5795ba5 to
591ec14
Compare
|
Can you remind me which endpoint this is for by showing an example of before/after with |
|
@alexellis I have updated the PR description with the curl examples. As stated in the PR title this impacts the function info endpoint: |
Description
Ensure that all fields of the
FunctionStatusare populated based onthe function
Deployment. In particular, this change ensures that theenv variables, constrains, and the read-only root filesystem values are
populated correctly.
Found while updating the
faas-cli, see openfaas/faas-cli#915Motivation and Context
Resolves #922
How Has This Been Tested?
The extended unit tests fully cover the new code.
As stated in the PR title, this adds missing fields to the function info endpoint:
/system/function/{function_name}Without this change, the endpoint misses values like
readOnlyFilesystemWith this change
Types of changes
Checklist:
git commit -s