Add config for fetcher resource requests & limits (fission#1489)

This PR adds fetcher resource requests&limits chart setting and remove unreasonable limits value from the charts.
blackfly19 · Jan 15, 2020 · b96ca07 · b96ca07
1 parent c421e65
commit b96ca07
Show file tree

Hide file tree

Showing 11 changed files with 88 additions and 72 deletions.
diff --git a/charts/README.md b/charts/README.md
@@ -45,8 +45,8 @@ Parameter | Description | Default
 `image` | Fission image repository | `fission/fission-bundle`
 `imageTag` | Fission image tag | `1.7.1`
 `pullPolicy` | Image pull policy | `IfNotPresent`
-`fetcherImage` | Fission fetcher repository | `fission/fetcher`
-`fetcherImageTag` | Fission fetcher image tag | `1.7.1`
+`fetcher.image` | Fission fetcher repository | `fission/fetcher`
+`fetcher.imageTag` | Fission fetcher image tag | `1.7.1`
 `controllerPort` | Fission Controller service port | `31313`
 `routerPort` | Fission Router service port | ` 31314`
 `functionNamespace` | Namespace in which to run fission functions (this is different from the release namespace) | `fission-function`

diff --git a/charts/fission-all/templates/deployment.yaml b/charts/fission-all/templates/deployment.yaml
@@ -210,7 +210,7 @@ spec:
         args: ["--executorPort", "8888", "--namespace", "{{ .Values.functionNamespace }}"]
         env:
         - name: FETCHER_IMAGE
-          value: "{{ .Values.fetcherImage }}:{{ .Values.fetcherImageTag }}"
+          value: "{{ .Values.fetcher.image }}:{{ .Values.fetcher.imageTag }}"
         - name: FETCHER_IMAGE_PULL_POLICY
           value: "{{ .Values.pullPolicy }}"
         - name: RUNTIME_IMAGE_PULL_POLICY
@@ -224,13 +224,13 @@ spec:
         - name: TRACING_SAMPLING_RATE
           value: {{ .Values.traceSamplingRate | default "0.5" | quote }}
         - name: FETCHER_MINCPU
-          value: {{ .Values.fetcherMinCpu | default "10m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.requests | quote }}
         - name: FETCHER_MINMEM
-          value: {{ .Values.fetcherMinMem | default "16Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.requests | quote }}
         - name: FETCHER_MAXCPU
-          value: {{ .Values.fetcherMaxCpu | default "1000m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.limits | quote }}
         - name: FETCHER_MAXMEM
-          value: {{ .Values.fetcherMaxMem | default "128Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.limits | quote }}
         - name: DEBUG_ENV
           value: {{ .Values.debugEnv | quote }}
         readinessProbe:
@@ -282,7 +282,7 @@ spec:
         args: ["--builderMgr", "--storageSvcUrl", "http://storagesvc.{{ .Release.Namespace }}", "--envbuilder-namespace", "{{ .Values.builderNamespace }}"]
         env:
         - name: FETCHER_IMAGE
-          value: "{{ .Values.fetcherImage }}:{{ .Values.fetcherImageTag }}"
+          value: "{{ .Values.fetcher.image }}:{{ .Values.fetcher.imageTag }}"
         - name: FETCHER_IMAGE_PULL_POLICY
           value: "{{ .Values.pullPolicy }}"
         - name: BUILDER_IMAGE_PULL_POLICY
@@ -294,13 +294,13 @@ spec:
         - name: TRACING_SAMPLING_RATE
           value: {{ .Values.traceSamplingRate | default "0.5" | quote }}
         - name: FETCHER_MINCPU
-          value: {{ .Values.fetcherMinCpu | default "10m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.requests | quote }}
         - name: FETCHER_MINMEM
-          value: {{ .Values.fetcherMinMem | default "16Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.requests | quote }}
         - name: FETCHER_MAXCPU
-          value: {{ .Values.fetcherMaxCpu | default "1000m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.limits | quote }}
         - name: FETCHER_MAXMEM
-          value: {{ .Values.fetcherMaxMem | default "128Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.limits | quote }}
         - name: DEBUG_ENV
           value: {{ .Values.debugEnv | quote }}
       serviceAccountName: fission-svc

diff --git a/charts/fission-all/values.yaml b/charts/fission-all/values.yaml
@@ -22,12 +22,6 @@ pullPolicy: IfNotPresent
 ## Fission image version
 imageTag: 1.7.1
 
-## Fission fetcher repository
-fetcherImage: fission/fetcher
-
-## Fission fetcher image version
-fetcherImageTag: 1.7.1
-
 ## Port at which Fission controller service should be exposed
 controllerPort: 31313
 
@@ -51,6 +45,23 @@ builderNamespace: fission-builder
 ## Enable istio integration
 enableIstio: false
 
+fetcher:
+  ## Fetcher repository
+  image: fission/fetcher
+  ## Fetcher image version
+  imageTag: 1.7.1
+
+  ## Fetcher is only for to downloading or uploading archive.
+  ## Normally, you don't need to change the value here, unless necessary.
+  resource:
+    cpu:
+      requests: "10m"
+      ## Low CPU limits will increases the function specialization time.
+      limits: ""
+    mem:
+      requests: "16Mi"
+      limits: ""
+
 ## Logger config
 logger:
   influxdbAdmin: "admin"

diff --git a/charts/fission-core/templates/deployment.yaml b/charts/fission-core/templates/deployment.yaml
@@ -210,7 +210,9 @@ spec:
         args: ["--executorPort", "8888", "--namespace", "{{ .Values.functionNamespace }}"]
         env:
         - name: FETCHER_IMAGE
-          value: "{{ .Values.fetcherImage }}:{{ .Values.fetcherImageTag }}"
+          value: "{{ .Values.fetcher.image }}:{{ .Values.fetcher.imageTag }}"
+        - name: RUNTIME_IMAGE_PULL_POLICY
+          value: "{{ .Values.pullPolicy }}"
         - name: FETCHER_IMAGE_PULL_POLICY
           value: "{{ .Values.pullPolicy }}"
         - name: TRACE_JAEGER_COLLECTOR_ENDPOINT
@@ -222,13 +224,13 @@ spec:
         - name: ENABLE_ISTIO
           value: "{{ .Values.enableIstio }}"
         - name: FETCHER_MINCPU
-          value: {{ .Values.fetcherMinCpu | default "10m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.requests | quote }}
         - name: FETCHER_MINMEM
-          value: {{ .Values.fetcherMinMem | default "16Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.requests | quote }}
         - name: FETCHER_MAXCPU
-          value: {{ .Values.fetcherMaxCpu | default "1000m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.limits | quote }}
         - name: FETCHER_MAXMEM
-          value: {{ .Values.fetcherMaxMem | default "128Mi" | quote }}          
+          value: {{ .Values.fetcher.resource.mem.limits | quote }}
         readinessProbe:
           httpGet:
             path: "/healthz"
@@ -278,7 +280,7 @@ spec:
         args: ["--builderMgr", "--storageSvcUrl", "http://storagesvc.{{ .Release.Namespace }}", "--envbuilder-namespace", "{{ .Values.builderNamespace }}"]
         env:
         - name: FETCHER_IMAGE
-          value: "{{ .Values.fetcherImage }}:{{ .Values.fetcherImageTag }}"
+          value: "{{ .Values.fetcher.image }}:{{ .Values.fetcher.imageTag }}"
         - name: FETCHER_IMAGE_PULL_POLICY
           value: "{{ .Values.pullPolicy }}"
         - name: BUILDER_IMAGE_PULL_POLICY
@@ -290,13 +292,13 @@ spec:
         - name: ENABLE_ISTIO
           value: "{{ .Values.enableIstio }}"
         - name: FETCHER_MINCPU
-          value: {{ .Values.fetcherMinCpu | default "10m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.requests | quote }}
         - name: FETCHER_MINMEM
-          value: {{ .Values.fetcherMinMem | default "16Mi" | quote }}
+          value: {{ .Values.fetcher.resource.mem.requests | quote }}
         - name: FETCHER_MAXCPU
-          value: {{ .Values.fetcherMaxCpu | default "1000m" | quote }}
+          value: {{ .Values.fetcher.resource.cpu.limits | quote }}
         - name: FETCHER_MAXMEM
-          value: {{ .Values.fetcherMaxMem | default "128Mi" | quote }}          
+          value: {{ .Values.fetcher.resource.mem.limits | quote }}
       serviceAccountName: fission-svc
 {{- if .Values.extraCoreComponentPodConfig }}
 {{ toYaml .Values.extraCoreComponentPodConfig | indent 6 -}}

diff --git a/charts/fission-core/values.yaml b/charts/fission-core/values.yaml
@@ -22,12 +22,6 @@ imageTag: 1.7.1
 ## Image pull policy
 pullPolicy: IfNotPresent
 
-## Fission fetcher repository
-fetcherImage: fission/fetcher
-
-## Fission fetcher image version
-fetcherImageTag: 1.7.1
-
 ## Port at which Fission controller service should be exposed
 controllerPort: 31313
 
@@ -45,6 +39,23 @@ builderNamespace: fission-builder
 ## Enable istio integration
 enableIstio: false
 
+fetcher:
+  ## Fetcher repository
+  image: fission/fetcher
+  ## Fetcher image version
+  imageTag: 1.7.1
+
+  ## Fetcher is only for to downloading or uploading archive.
+  ## Normally, you don't need to change the value here, unless necessary.
+  resource:
+    cpu:
+      requests: "10m"
+      ## Low CPU limits will increases the function specialization time.
+      limits: ""
+    mem:
+      requests: "16Mi"
+      limits: ""
+
 executor:
   adoptExistingResources: false
 

diff --git a/pkg/executor/executortype/poolmgr/packagewatcher.go b/pkg/executor/executortype/poolmgr/packagewatcher.go
@@ -43,7 +43,7 @@ func (gpm *GenericPoolManager) makePkgController(fissionClient *crd.FissionClien
 				pkg := obj.(*fv1.Package)
 				gpm.logger.Debug("list watch for package reported a new package addition",
 					zap.String("package_name", pkg.Metadata.Name),
-					zap.String("package_namepsace", pkg.Metadata.Namespace))
+					zap.String("package_namespace", pkg.Metadata.Namespace))
 
 				// create or update role-binding for fetcher sa in env ns to be able to get the pkg contents from pkg namespace
 				envNs := fissionfnNamespace

diff --git a/pkg/executor/reaper/reaper.go b/pkg/executor/reaper/reaper.go
@@ -207,7 +207,7 @@ func CleanupRoleBindings(logger *zap.Logger, client *kubernetes.Clientset, fissi
 			// we can list the functions once per role-binding.
 			funcList, err := fissionClient.Functions(roleBinding.Namespace).List(meta_v1.ListOptions{})
 			if err != nil {
-				logger.Error("error fetching environment list in namespace", zap.Error(err), zap.String("namespace", roleBinding.Namespace))
+				logger.Error("error fetching function list in namespace", zap.Error(err), zap.String("namespace", roleBinding.Namespace))
 				continue
 			}
 
@@ -248,7 +248,7 @@ func CleanupRoleBindings(logger *zap.Logger, client *kubernetes.Clientset, fissi
 					}
 				}
 
-				// if its a package-getterr-rb, we have 2 kinds of SAs and each of them is handled differently
+				// if its a package-getter-rb, we have 2 kinds of SAs and each of them is handled differently
 				// else if its a secret-configmap-rb, we have only one SA which is fission-fetcher
 				if roleBinding.Name == types.PackageGetterRB {
 					// check if there is an env obj in saNs

diff --git a/pkg/fetcher/config/config.go b/pkg/fetcher/config/config.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -31,46 +32,36 @@ type Config struct {
 	sharedSecretPath string
 	sharedCfgMapPath string
 
-	// dockerRegistryAuthDomain string
-	// dockerRegistryUsername   string
-	// dockerRegistryPassword   string
-
 	serviceAccount string
 
 	jaegerCollectorEndpoint string
 }
 
 func getFetcherResources() (apiv1.ResourceRequirements, error) {
-	mincpu, err := resource.ParseQuantity(os.Getenv("FETCHER_MINCPU"))
-	if err != nil {
-		return apiv1.ResourceRequirements{}, err
-	}
-
-	minmem, err := resource.ParseQuantity(os.Getenv("FETCHER_MINMEM"))
-	if err != nil {
-		return apiv1.ResourceRequirements{}, err
-	}
-
-	maxcpu, err := resource.ParseQuantity(os.Getenv("FETCHER_MAXCPU"))
-	if err != nil {
-		return apiv1.ResourceRequirements{}, err
+	resourceReqs := apiv1.ResourceRequirements{
+		Requests: map[apiv1.ResourceName]resource.Quantity{},
+		Limits:   map[apiv1.ResourceName]resource.Quantity{},
 	}
+	errs := utils.MultiErrorWithFormat()
+	errs = multierror.Append(errs,
+		parseResources("FETCHER_MINCPU", resourceReqs.Requests, apiv1.ResourceCPU),
+		parseResources("FETCHER_MINMEM", resourceReqs.Requests, apiv1.ResourceMemory),
+		parseResources("FETCHER_MAXCPU", resourceReqs.Limits, apiv1.ResourceCPU),
+		parseResources("FETCHER_MAXMEM", resourceReqs.Limits, apiv1.ResourceMemory),
+	)
+	return resourceReqs, errs.ErrorOrNil()
+}
 
-	maxmem, err := resource.ParseQuantity(os.Getenv("FETCHER_MAXMEM"))
-	if err != nil {
-		return apiv1.ResourceRequirements{}, err
+func parseResources(env string, resourceReqs map[apiv1.ResourceName]resource.Quantity, resName apiv1.ResourceName) error {
+	val := os.Getenv(env)
+	if len(val) > 0 {
+		quantity, err := resource.ParseQuantity(val)
+		if err != nil {
+			return err
+		}
+		resourceReqs[resName] = quantity
 	}
-
-	return apiv1.ResourceRequirements{
-		Requests: map[apiv1.ResourceName]resource.Quantity{
-			apiv1.ResourceCPU:    mincpu,
-			apiv1.ResourceMemory: minmem,
-		},
-		Limits: map[apiv1.ResourceName]resource.Quantity{
-			apiv1.ResourceCPU:    maxcpu,
-			apiv1.ResourceMemory: maxmem,
-		},
-	}, nil
+	return nil
 }
 
 func MakeFetcherConfig(sharedMountPath string) (*Config, error) {

diff --git a/skaffold.yaml b/skaffold.yaml
@@ -42,8 +42,9 @@ deploy:
         setValues:
           image: <DOCKERHUB_REPO>/fission
           imageTag: skaffold-test
-          fetcherImage: <DOCKERHUB_REPO>/fetcher
-          fetcherImageTag: skaffold-test
+          fetcher:
+            image: <DOCKERHUB_REPO>/fetcher
+            imageTag: skaffold-test
           namespace: fission
           preUpgradeChecksImage: <DOCKERHUB_REPO>/preupgradechecks
           repository: index.docker.io

diff --git a/test/test_utils.sh b/test/test_utils.sh
@@ -240,7 +240,7 @@ helm_install_fission() {
     ns=f-$id
     fns=f-func-$id
 
-    helmVars=repository=$repo,image=$image,imageTag=$imageTag,fetcherImage=$fetcherImage,fetcherImageTag=$fetcherImageTag,functionNamespace=$fns,controllerPort=$controllerNodeport,routerPort=$routerNodeport,pullPolicy=Always,analytics=false,debugEnv=true,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType,serviceType=$serviceType,preUpgradeChecksImage=$preUpgradeCheckImage,prometheus.server.persistentVolume.enabled=false,prometheus.alertmanager.enabled=false,prometheus.kubeStateMetrics.enabled=false,prometheus.nodeExporter.enabled=false
+    helmVars=repository=$repo,image=$image,imageTag=$imageTag,fetcher.image=$fetcherImage,fetcher.imageTag=$fetcherImageTag,functionNamespace=$fns,controllerPort=$controllerNodeport,routerPort=$routerNodeport,pullPolicy=Always,analytics=false,debugEnv=true,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType,serviceType=$serviceType,preUpgradeChecksImage=$preUpgradeCheckImage,prometheus.server.persistentVolume.enabled=false,prometheus.alertmanager.enabled=false,prometheus.kubeStateMetrics.enabled=false,prometheus.nodeExporter.enabled=false
 
     timeout 30 bash -c "helm_setup"
 

diff --git a/test/upgrade/fission_upgrade_test.sh b/test/upgrade/fission_upgrade_test.sh
@@ -90,7 +90,7 @@ sudo mv $ROOT/fission/fission /usr/local/bin/
 
 ## Upgrade 
 
-helmVars=repository=$repo,image=$IMAGE,imageTag=$TAG,fetcherImage=$FETCHER_IMAGE,fetcherImageTag=$TAG,functionNamespace=$fns,controllerPort=$controllerNodeport,pullPolicy=Always,analytics=false,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType
+helmVars=repository=$repo,image=$IMAGE,imageTag=$TAG,fetcher.image=$FETCHER_IMAGE,fetcher.imageTag=$TAG,functionNamespace=$fns,controllerPort=$controllerNodeport,pullPolicy=Always,analytics=false,pruneInterval=$pruneInterval,routerServiceType=$routerServiceType
 
 echo "Upgrading fission"
 helm upgrade	\