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

fix: return full status object from the info endpoint #923

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions pkg/handlers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,19 @@ func updateDeploymentSpec(
}

if len(deployment.Spec.Template.Spec.Containers) > 0 {
deployment.Spec.Template.Spec.Containers[0].Image = request.Image
idx, _ := k8s.FunctionContainer(*deployment)
if idx < 0 {
return fmt.Errorf("deployment is not a valid function spec"), http.StatusBadRequest
}

deployment.Spec.Template.Spec.Containers[idx].Image = request.Image

// Disabling update support to prevent unexpected mutations of deployed functions,
// since imagePullPolicy is now configurable. This could be reconsidered later depending
// on desired behavior, but will need to be updated to take config.
//deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy = v1.PullAlways
//deployment.Spec.Template.Spec.Containers[idx].ImagePullPolicy = v1.PullAlways

deployment.Spec.Template.Spec.Containers[0].Env = buildEnvVars(&request)
deployment.Spec.Template.Spec.Containers[idx].Env = buildEnvVars(&request)

factory.ConfigureReadOnlyRootFilesystem(request, deployment)
factory.ConfigureContainerUserID(deployment)
Expand Down Expand Up @@ -135,7 +140,7 @@ func updateDeploymentSpec(
return resourceErr, http.StatusBadRequest
}

deployment.Spec.Template.Spec.Containers[0].Resources = *resources
deployment.Spec.Template.Spec.Containers[idx].Resources = *resources

secrets := k8s.NewSecretsClient(factory.Client)
existingSecrets, err := secrets.GetSecrets(functionNamespace, request.Secrets)
Expand All @@ -154,8 +159,8 @@ func updateDeploymentSpec(
return err, http.StatusBadRequest
}

deployment.Spec.Template.Spec.Containers[0].LivenessProbe = probes.Liveness
deployment.Spec.Template.Spec.Containers[0].ReadinessProbe = probes.Readiness
deployment.Spec.Template.Spec.Containers[idx].LivenessProbe = probes.Liveness
deployment.Spec.Template.Spec.Containers[idx].ReadinessProbe = probes.Readiness

// compare the annotations from args to the cache copy of the deployment annotations
// at this point we have already updated the annotations to the new value, if we
Expand Down
66 changes: 55 additions & 11 deletions pkg/k8s/function_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package k8s
import (
types "github.com/openfaas/faas-provider/types"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
)

// EnvProcessName is the name of the env variable containing the function process
Expand All @@ -19,20 +20,23 @@ func AsFunctionStatus(item appsv1.Deployment) *types.FunctionStatus {
replicas = uint64(*item.Spec.Replicas)
}

functionContainer := item.Spec.Template.Spec.Containers[0]
_, functionContainer := FunctionContainer(item)

labels := item.Spec.Template.Labels
function := types.FunctionStatus{
Name: item.Name,
Replicas: replicas,
Image: functionContainer.Image,
AvailableReplicas: uint64(item.Status.AvailableReplicas),
InvocationCount: 0,
Labels: &labels,
Annotations: &item.Spec.Template.Annotations,
Namespace: item.Namespace,
Secrets: ReadFunctionSecretsSpec(item),
CreatedAt: item.CreationTimestamp.Time,
Name: item.Name,
Replicas: replicas,
Image: functionContainer.Image,
AvailableReplicas: uint64(item.Status.AvailableReplicas),
InvocationCount: 0,
Labels: &labels,
Annotations: &item.Spec.Template.Annotations,
Namespace: item.Namespace,
Secrets: ReadFunctionSecretsSpec(item),
CreatedAt: item.CreationTimestamp.Time,
Constraints: nodeSelectorToConstraint(item),
ReadOnlyRootFilesystem: hasReadOnlyRootFilesystem(item),
EnvVars: map[string]string{},
}

req := &types.FunctionResources{Memory: functionContainer.Resources.Requests.Memory().String(), CPU: functionContainer.Resources.Requests.Cpu().String()}
Expand All @@ -48,8 +52,48 @@ func AsFunctionStatus(item appsv1.Deployment) *types.FunctionStatus {
for _, v := range functionContainer.Env {
if EnvProcessName == v.Name {
function.EnvProcess = v.Value
continue
}
function.EnvVars[v.Name] = v.Value
}

return &function
}

func nodeSelectorToConstraint(deploy appsv1.Deployment) []string {
nodeSelector := deploy.Spec.Template.Spec.NodeSelector
alexellis marked this conversation as resolved.
Show resolved Hide resolved
constraints := []string{}
for k, v := range nodeSelector {
constraints = append(constraints, k+"="+v)
}
return constraints
}

func hasReadOnlyRootFilesystem(function appsv1.Deployment) bool {
_, c := FunctionContainer(function)
securityContext := c.SecurityContext
if securityContext == nil {
return false
}

if securityContext.ReadOnlyRootFilesystem == nil {
return false
}

return *securityContext.ReadOnlyRootFilesystem
}

// FunctionContainer returns the index and spec of the OpenFaaS function container
// in the deployment. Use this method to safely retrieve the container spec, it protects
// the controller from potential changes in the deployment spec by other Operators and
// Admission webhooks in the cluster.
//
// idx will be -1 if the function container is not found.
func FunctionContainer(function appsv1.Deployment) (idx int, c corev1.Container) {
for idx, container := range function.Spec.Template.Spec.Containers {
if container.Name == function.Name {
return idx, container
}
}
return -1, c
}
79 changes: 78 additions & 1 deletion pkg/k8s/function_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -23,7 +24,10 @@ func Test_AsFunctionStatus(t *testing.T) {
annotations := map[string]string{"data": "datavalue"}
namespace := "func-namespace"
envProcess := "process string here"
envVars := map[string]string{"env1": "env1value", "env2": "env2value"}
secrets := []string{"0-imagepullsecret", "1-genericsecret", "2-genericsecret"}
readOnlyRootFilesystem := false
constraints := []string{"node-label=node-value"}

deploy := appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -39,6 +43,9 @@ func Test_AsFunctionStatus(t *testing.T) {
Labels: labels,
},
Spec: corev1.PodSpec{
NodeSelector: map[string]string{
"node-label": "node-value",
},
ImagePullSecrets: []corev1.LocalObjectReference{
{Name: "0-imagepullsecret"},
},
Expand All @@ -48,7 +55,8 @@ func Test_AsFunctionStatus(t *testing.T) {
Image: image,
Env: []corev1.EnvVar{
{Name: "fprocess", Value: envProcess},
{Name: "customEnv", Value: "customValue"},
{Name: "env1", Value: "env1value"},
{Name: "env2", Value: "env2value"},
},
},
},
Expand Down Expand Up @@ -122,4 +130,73 @@ func Test_AsFunctionStatus(t *testing.T) {
t.Errorf("incorrect Secrets: expected %v, got : %v", secrets, status.Secrets)
}

if !reflect.DeepEqual(status.EnvVars, envVars) {
t.Errorf("incorrect EnvVars: expected %+v, got %+v", envVars, status.EnvVars)
}

if !reflect.DeepEqual(status.Constraints, constraints) {
t.Errorf("incorrect Constraints: expected %+v, got %+v", constraints, status.Constraints)
}

if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem {
t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem)
}

t.Run("can parse readonly root filesystem when nil", func(t *testing.T) {
readOnlyRootFilesystem := false
deployment := deploy
deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{}

status := AsFunctionStatus(deployment)
if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem {
t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem)
}
})

t.Run("can parse readonly root filesystem enabled", func(t *testing.T) {
readOnlyRootFilesystem := true
deployment := deploy
deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
ReadOnlyRootFilesystem: &readOnlyRootFilesystem,
}

status := AsFunctionStatus(deployment)
if status.ReadOnlyRootFilesystem != readOnlyRootFilesystem {
t.Errorf("incorrect ReadOnlyRootFilesystem: expected %v, got : %v", readOnlyRootFilesystem, status.ReadOnlyRootFilesystem)
}
})

t.Run("returns non-empty resource requests", func(t *testing.T) {
deployment := deploy
deployment.Spec.Template.Spec.Containers[0].Resources.Requests = corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
}

status := AsFunctionStatus(deployment)
if status.Requests.CPU != "100m" {
t.Errorf("incorrect Requests.CPU: expected %s, got %s", "100m", status.Requests.CPU)
}

if status.Requests.Memory != "100Mi" {
t.Errorf("incorrect Requests.Memory: expected %s, got %s", "100Mi", status.Requests.Memory)
}
})

t.Run("returns non-empty resource limits", func(t *testing.T) {
deployment := deploy
deployment.Spec.Template.Spec.Containers[0].Resources.Limits = corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
}

status := AsFunctionStatus(deployment)
if status.Limits.CPU != "100m" {
t.Errorf("incorrect Limits.CPU: expected %s, got %s", "100m", status.Limits.CPU)
}

if status.Limits.Memory != "100Mi" {
t.Errorf("incorrect Limits.Memory: expected %s, got %s", "100Mi", status.Limits.Memory)
}
})
}
40 changes: 31 additions & 9 deletions pkg/k8s/securityContext.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,22 @@ func (f *FunctionFactory) ConfigureContainerUserID(deployment *appsv1.Deployment
functionUser = &userID
}

if deployment.Spec.Template.Spec.Containers[0].SecurityContext == nil {
deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{}
if deployment == nil {
return
}

deployment.Spec.Template.Spec.Containers[0].SecurityContext.RunAsUser = functionUser
idx, container := FunctionContainer(*deployment)
if idx < 0 {
// function container not found
// and there is nothing we can do at this point
return
}

if container.SecurityContext == nil {
deployment.Spec.Template.Spec.Containers[idx].SecurityContext = &corev1.SecurityContext{}
}

deployment.Spec.Template.Spec.Containers[idx].SecurityContext.RunAsUser = functionUser
}

// ConfigureReadOnlyRootFilesystem will create or update the required settings and mounts to ensure
Expand All @@ -39,19 +50,30 @@ func (f *FunctionFactory) ConfigureContainerUserID(deployment *appsv1.Deployment
//
// This method is safe for both create and update operations.
func (f *FunctionFactory) ConfigureReadOnlyRootFilesystem(request types.FunctionDeployment, deployment *appsv1.Deployment) {
if deployment.Spec.Template.Spec.Containers[0].SecurityContext != nil {
deployment.Spec.Template.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem = &request.ReadOnlyRootFilesystem
if deployment == nil {
return
}

idx, container := FunctionContainer(*deployment)
if idx < 0 {
// function container not found
// and there is nothing we can do at this point
return
}

if container.SecurityContext != nil {
deployment.Spec.Template.Spec.Containers[idx].SecurityContext.ReadOnlyRootFilesystem = &request.ReadOnlyRootFilesystem
} else {
deployment.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{
deployment.Spec.Template.Spec.Containers[idx].SecurityContext = &corev1.SecurityContext{
ReadOnlyRootFilesystem: &request.ReadOnlyRootFilesystem,
}
}

existingVolumes := removeVolume("temp", deployment.Spec.Template.Spec.Volumes)
deployment.Spec.Template.Spec.Volumes = existingVolumes

existingMounts := removeVolumeMount("temp", deployment.Spec.Template.Spec.Containers[0].VolumeMounts)
deployment.Spec.Template.Spec.Containers[0].VolumeMounts = existingMounts
existingMounts := removeVolumeMount("temp", container.VolumeMounts)
deployment.Spec.Template.Spec.Containers[idx].VolumeMounts = existingMounts

if request.ReadOnlyRootFilesystem {
deployment.Spec.Template.Spec.Volumes = append(
Expand All @@ -64,7 +86,7 @@ func (f *FunctionFactory) ConfigureReadOnlyRootFilesystem(request types.Function
},
)

deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(
deployment.Spec.Template.Spec.Containers[idx].VolumeMounts = append(
existingMounts,
corev1.VolumeMount{
Name: "temp",
Expand Down
13 changes: 13 additions & 0 deletions pkg/k8s/securityContext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

appsv1 "k8s.io/api/apps/v1"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func readOnlyRootDisabled(t *testing.T, deployment *appsv1.Deployment) {
Expand Down Expand Up @@ -68,6 +69,9 @@ func readOnlyRootEnabled(t *testing.T, deployment *appsv1.Deployment) {
func Test_configureReadOnlyRootFilesystem_Disabled_To_Disabled(t *testing.T) {
f := mockFactory()
deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "testfunc",
},
Spec: appsv1.DeploymentSpec{
Template: apiv1.PodTemplateSpec{
Spec: apiv1.PodSpec{
Expand All @@ -91,6 +95,9 @@ func Test_configureReadOnlyRootFilesystem_Disabled_To_Disabled(t *testing.T) {
func Test_configureReadOnlyRootFilesystem_Disabled_To_Enabled(t *testing.T) {
f := mockFactory()
deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "testfunc",
},
Spec: appsv1.DeploymentSpec{
Template: apiv1.PodTemplateSpec{
Spec: apiv1.PodSpec{
Expand All @@ -115,6 +122,9 @@ func Test_configureReadOnlyRootFilesystem_Enabled_To_Disabled(t *testing.T) {
f := mockFactory()
trueValue := true
deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "testfunc",
},
Spec: appsv1.DeploymentSpec{
Template: apiv1.PodTemplateSpec{
Spec: apiv1.PodSpec{
Expand Down Expand Up @@ -155,6 +165,9 @@ func Test_configureReadOnlyRootFilesystem_Enabled_To_Enabled(t *testing.T) {
f := mockFactory()
trueValue := true
deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "testfunc",
},
Spec: appsv1.DeploymentSpec{
Template: apiv1.PodTemplateSpec{
Spec: apiv1.PodSpec{
Expand Down