From 5e2f984136797bb72e9cb033376906295692cfa9 Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Wed, 28 Feb 2018 05:28:05 +0800 Subject: [PATCH] Fix CLI not update function's secret/configmap correctly (#512) This PR addressed some issues introduced in PR399. Also, now env builders and function pods can mount shared secret/configmap volumes correctly. --- buildermgr/envwatcher.go | 34 ++++++- executor/newdeploy/newdeploy.go | 36 +++++++- executor/poolmgr/gp.go | 18 ++-- fission/function.go | 92 +++++++++++++------ fission/main.go | 2 +- .../test_secret_cfgmap/test_secret_cfgmap.sh | 85 +++++++++++------ types.go | 7 ++ 7 files changed, 199 insertions(+), 75 deletions(-) diff --git a/buildermgr/envwatcher.go b/buildermgr/envwatcher.go index 4a8df74a..67f71819 100644 --- a/buildermgr/envwatcher.go +++ b/buildermgr/envwatcher.go @@ -478,7 +478,19 @@ func (envw *environmentWatcher) createBuilderDeployment(env *crd.Environment) (* Spec: apiv1.PodSpec{ Volumes: []apiv1.Volume{ { - Name: "package", + Name: fission.SharedVolumePackages, + VolumeSource: apiv1.VolumeSource{ + EmptyDir: &apiv1.EmptyDirVolumeSource{}, + }, + }, + { + Name: fission.SharedVolumeSecrets, + VolumeSource: apiv1.VolumeSource{ + EmptyDir: &apiv1.EmptyDirVolumeSource{}, + }, + }, + { + Name: fission.SharedVolumeConfigmaps, VolumeSource: apiv1.VolumeSource{ EmptyDir: &apiv1.EmptyDirVolumeSource{}, }, @@ -492,9 +504,17 @@ func (envw *environmentWatcher) createBuilderDeployment(env *crd.Environment) (* TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: "package", + Name: fission.SharedVolumePackages, MountPath: sharedMountPath, }, + { + Name: fission.SharedVolumeSecrets, + MountPath: sharedSecretPath, + }, + { + Name: fission.SharedVolumeConfigmaps, + MountPath: sharedCfgMapPath, + }, }, Command: []string{"/builder", sharedMountPath}, ReadinessProbe: &apiv1.Probe{ @@ -518,9 +538,17 @@ func (envw *environmentWatcher) createBuilderDeployment(env *crd.Environment) (* TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: "package", + Name: fission.SharedVolumePackages, MountPath: sharedMountPath, }, + { + Name: fission.SharedVolumeSecrets, + MountPath: sharedSecretPath, + }, + { + Name: fission.SharedVolumeConfigmaps, + MountPath: sharedCfgMapPath, + }, }, Command: []string{"/fetcher", "-secret-dir", sharedSecretPath, diff --git a/executor/newdeploy/newdeploy.go b/executor/newdeploy/newdeploy.go index bb794358..bf91c20b 100644 --- a/executor/newdeploy/newdeploy.go +++ b/executor/newdeploy/newdeploy.go @@ -55,7 +55,7 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro replicas = 1 } targetFilename := "user" - userfunc := "userfunc" + var gracePeriodSeconds int64 = 6 * 60 existingDepl, err := deploy.kubernetesClient.ExtensionsV1beta1().Deployments(deploy.namespace).Get(deployName, metav1.GetOptions{}) @@ -120,7 +120,19 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro Spec: apiv1.PodSpec{ Volumes: []apiv1.Volume{ { - Name: userfunc, + Name: fission.SharedVolumeUserfunc, + VolumeSource: apiv1.VolumeSource{ + EmptyDir: &apiv1.EmptyDirVolumeSource{}, + }, + }, + { + Name: fission.SharedVolumeSecrets, + VolumeSource: apiv1.VolumeSource{ + EmptyDir: &apiv1.EmptyDirVolumeSource{}, + }, + }, + { + Name: fission.SharedVolumeConfigmaps, VolumeSource: apiv1.VolumeSource{ EmptyDir: &apiv1.EmptyDirVolumeSource{}, }, @@ -134,9 +146,17 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: userfunc, + Name: fission.SharedVolumeUserfunc, MountPath: deploy.sharedMountPath, }, + { + Name: fission.SharedVolumeSecrets, + MountPath: deploy.sharedSecretPath, + }, + { + Name: fission.SharedVolumeConfigmaps, + MountPath: deploy.sharedCfgMapPath, + }, }, Resources: env.Spec.Resources, Lifecycle: &apiv1.Lifecycle{ @@ -157,9 +177,17 @@ func (deploy *NewDeploy) createOrGetDeployment(fn *crd.Function, env *crd.Enviro TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: userfunc, + Name: fission.SharedVolumeUserfunc, MountPath: deploy.sharedMountPath, }, + { + Name: fission.SharedVolumeSecrets, + MountPath: deploy.sharedSecretPath, + }, + { + Name: fission.SharedVolumeConfigmaps, + MountPath: deploy.sharedCfgMapPath, + }, }, Command: []string{"/fetcher", "-specialize-on-startup", "-fetch-request", string(fetchPayload), diff --git a/executor/poolmgr/gp.go b/executor/poolmgr/gp.go index 93e68512..7fe4df9d 100644 --- a/executor/poolmgr/gp.go +++ b/executor/poolmgr/gp.go @@ -504,19 +504,19 @@ func (gp *GenericPool) createPool() error { Spec: apiv1.PodSpec{ Volumes: []apiv1.Volume{ { - Name: "userfunc", + Name: fission.SharedVolumeUserfunc, VolumeSource: apiv1.VolumeSource{ EmptyDir: &apiv1.EmptyDirVolumeSource{}, }, }, { - Name: "secrets", + Name: fission.SharedVolumeSecrets, VolumeSource: apiv1.VolumeSource{ EmptyDir: &apiv1.EmptyDirVolumeSource{}, }, }, { - Name: "config", + Name: fission.SharedVolumeConfigmaps, VolumeSource: apiv1.VolumeSource{ EmptyDir: &apiv1.EmptyDirVolumeSource{}, }, @@ -530,15 +530,15 @@ func (gp *GenericPool) createPool() error { TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: "userfunc", + Name: fission.SharedVolumeUserfunc, MountPath: gp.sharedMountPath, }, { - Name: "secrets", + Name: fission.SharedVolumeSecrets, MountPath: gp.sharedSecretPath, }, { - Name: "config", + Name: fission.SharedVolumeConfigmaps, MountPath: gp.sharedCfgMapPath, }, }, @@ -567,15 +567,15 @@ func (gp *GenericPool) createPool() error { TerminationMessagePath: "/dev/termination-log", VolumeMounts: []apiv1.VolumeMount{ { - Name: "userfunc", + Name: fission.SharedVolumeUserfunc, MountPath: gp.sharedMountPath, }, { - Name: "secrets", + Name: fission.SharedVolumeSecrets, MountPath: gp.sharedSecretPath, }, { - Name: "config", + Name: fission.SharedVolumeConfigmaps, MountPath: gp.sharedCfgMapPath, }, }, diff --git a/fission/function.go b/fission/function.go index 0240d719..44e4fd1e 100644 --- a/fission/function.go +++ b/fission/function.go @@ -141,14 +141,6 @@ func fnCreate(c *cli.Context) error { secretNameSpace := c.String("secretNamespace") cfgMapNameSpace := c.String("configmapNamespace") - if len(secretNameSpace) == 0 && len(secretName) > 0 { - secretNameSpace = metav1.NamespaceDefault - } - - if len(cfgMapNameSpace) == 0 && len(cfgMapName) > 0 { - cfgMapNameSpace = metav1.NamespaceDefault - } - if len(pkgName) > 0 { // use existing package pkg, err := client.PackageGet(&metav1.ObjectMeta{ @@ -209,6 +201,31 @@ func fnCreate(c *cli.Context) error { invokeStrategy := getInvokeStrategy(c.Int("minscale"), c.Int("maxscale"), c.String("executortype"), targetCPU) + var secrets []fission.SecretReference + var cfgmaps []fission.ConfigMapReference + + if len(secretName) > 0 { + if len(secretNameSpace) == 0 { + secretNameSpace = metav1.NamespaceDefault + } + newSecret := fission.SecretReference{ + Name: secretName, + Namespace: secretNameSpace, + } + secrets = []fission.SecretReference{newSecret} + } + + if len(cfgMapName) > 0 { + if len(cfgMapNameSpace) == 0 { + cfgMapNameSpace = metav1.NamespaceDefault + } + newCfgMap := fission.ConfigMapReference{ + Name: cfgMapName, + Namespace: cfgMapNameSpace, + } + cfgmaps = []fission.ConfigMapReference{newCfgMap} + } + function := &crd.Function{ Metadata: metav1.ObjectMeta{ Name: fnName, @@ -227,29 +244,13 @@ func fnCreate(c *cli.Context) error { ResourceVersion: pkgMetadata.ResourceVersion, }, }, - Secrets: []fission.SecretReference{}, - ConfigMaps: []fission.ConfigMapReference{}, + Secrets: secrets, + ConfigMaps: cfgmaps, Resources: resourceReq, InvokeStrategy: invokeStrategy, }, } - if len(secretName) > 0 { - newSecret := fission.SecretReference{ - Name: secretName, - Namespace: secretNameSpace, - } - function.Spec.Secrets = append(function.Spec.Secrets, newSecret) - } - - if len(cfgMapName) > 0 { - newCfgMap := fission.ConfigMapReference{ - Name: cfgMapName, - Namespace: cfgMapNameSpace, - } - function.Spec.ConfigMaps = append(function.Spec.ConfigMaps, newCfgMap) - } - // if we're writing a spec, don't create the function if spec { err = specSave(*function, specFile) @@ -375,9 +376,44 @@ func fnUpdate(c *cli.Context) error { buildcmd := c.String("buildcmd") force := c.Bool("force") + secretName := c.String("secret") + cfgMapName := c.String("configmap") + + secretNameSpace := c.String("secretNamespace") + cfgMapNameSpace := c.String("configmapNamespace") + if len(envName) == 0 && len(deployArchiveName) == 0 && len(srcArchiveName) == 0 && len(pkgName) == 0 && - len(entrypoint) == 0 && len(buildcmd) == 0 { - fatal("Need --env or --deploy or --src or --pkg or --entrypoint or --buildcmd argument.") + len(entrypoint) == 0 && len(buildcmd) == 0 && len(secretName) == 0 && len(secretNameSpace) == 0 && + len(cfgMapName) == 0 && len(cfgMapNameSpace) == 0 { + fatal("Need --env or --deploy or --src or --pkg or --entrypoint or --buildcmd or --secret or --secretNamespace or --configmap or --configmapNamespace argument.") + } + + if len(secretName) > 0 { + if len(function.Spec.Secrets) > 1 { + fatal("Please use 'fission spec apply' to update list of secrets") + } + if len(secretNameSpace) == 0 { + secretNameSpace = metav1.NamespaceDefault + } + newSecret := fission.SecretReference{ + Name: secretName, + Namespace: secretNameSpace, + } + function.Spec.Secrets = []fission.SecretReference{newSecret} + } + + if len(cfgMapName) > 0 { + if len(function.Spec.ConfigMaps) > 1 { + fatal("Please use 'fission spec apply' to update list of configmaps") + } + if len(cfgMapNameSpace) == 0 { + cfgMapNameSpace = metav1.NamespaceDefault + } + newCfgMap := fission.ConfigMapReference{ + Name: cfgMapName, + Namespace: cfgMapNameSpace, + } + function.Spec.ConfigMaps = []fission.ConfigMapReference{newCfgMap} } if len(envName) > 0 { diff --git a/fission/main.go b/fission/main.go index 8a716faf..77176739 100644 --- a/fission/main.go +++ b/fission/main.go @@ -90,7 +90,7 @@ func main() { {Name: "create", Usage: "Create new function (and optionally, an HTTP route to it)", Flags: []cli.Flag{fnNameFlag, fnEnvNameFlag, fnSpecSaveFlag, fnCodeFlag, fnPackageFlag, fnSrcArchiveFlag, fnDeployArchiveFlag, fnEntryPointFlag, fnBuildCmdFlag, fnPkgNameFlag, htUrlFlag, htMethodFlag, minCpu, maxCpu, minMem, maxMem, minScale, maxScale, fnExecutorTypeFlag, targetcpu, fnCfgMapFlag, fnSecretFlag, fnSecretnsFlag, fnCfgMapnsFlag}, Action: fnCreate}, {Name: "get", Usage: "Get function source code", Flags: []cli.Flag{fnNameFlag}, Action: fnGet}, {Name: "getmeta", Usage: "Get function metadata", Flags: []cli.Flag{fnNameFlag}, Action: fnGetMeta}, - {Name: "update", Usage: "Update function source code", Flags: []cli.Flag{fnNameFlag, fnEnvNameFlag, fnCodeFlag, fnPackageFlag, fnSrcArchiveFlag, fnDeployArchiveFlag, fnEntryPointFlag, fnPkgNameFlag, fnBuildCmdFlag, fnForceFlag, minCpu, maxCpu, minMem, maxMem, minScale, maxScale, fnExecutorTypeFlag, targetcpu}, Action: fnUpdate}, + {Name: "update", Usage: "Update function source code", Flags: []cli.Flag{fnNameFlag, fnEnvNameFlag, fnCodeFlag, fnPackageFlag, fnSrcArchiveFlag, fnDeployArchiveFlag, fnEntryPointFlag, fnPkgNameFlag, fnBuildCmdFlag, fnForceFlag, minCpu, maxCpu, minMem, maxMem, minScale, maxScale, fnExecutorTypeFlag, targetcpu, fnCfgMapFlag, fnSecretFlag, fnSecretnsFlag, fnCfgMapnsFlag}, Action: fnUpdate}, {Name: "delete", Usage: "Delete function", Flags: []cli.Flag{fnNameFlag}, Action: fnDelete}, {Name: "list", Usage: "List all functions", Flags: []cli.Flag{}, Action: fnList}, {Name: "logs", Usage: "Display function logs", Flags: []cli.Flag{fnNameFlag, fnPodFlag, fnFollowFlag, fnDetailFlag, fnLogDBTypeFlag, fnLogCountFlag}, Action: fnLogs}, diff --git a/test/tests/test_secret_cfgmap/test_secret_cfgmap.sh b/test/tests/test_secret_cfgmap/test_secret_cfgmap.sh index 4b5dc34f..7a3b308a 100755 --- a/test/tests/test_secret_cfgmap/test_secret_cfgmap.sh +++ b/test/tests/test_secret_cfgmap/test_secret_cfgmap.sh @@ -18,15 +18,34 @@ function cleanup { log "Cleanup everything" kubectl delete secret -n default ${fn_secret} kubectl delete configmap -n default ${fn_cfgmap} - fission function delete --name ${fn_secret} - fission function delete --name ${fn_cfgmap} - fission function delete --name ${fn} - var=$(fission route list | grep ${fn_secret} | awk '{print $1;}') - var2=$(fission route list | grep ${fn_cfgmap} | awk '{print $1;}') - var3=$(fission route list | grep ${fn} | awk '{print $1;}') - fission route delete --name ${var} - fission route delete --name ${var2} - fission route delete --name ${var3} + # delete functions + for f in ${fn_secret} ${fn_cfgmap} ${fn} + do + fission fn list | grep ${f} | awk '{print $1;}' | xargs -I@ bash -c "fission function delete --name @" + done + # delete routes + for r in ${fn_secret} ${fn_cfgmap} ${fn} + do + fission route list | grep ${r} | awk '{print $1;}' | xargs -I@ bash -c "fission route delete --name @" + done +} + +checkFunctionResponse() { + log "Doing an HTTP GET on the function's route" + response=$(curl http://$FISSION_ROUTER/${1}) + val=${2} + type=${3} + + log "Checking for valid response" + log ${response} + + if [[ ${response} != ${val} ]] + then + log "test ${type} failed" + cleanup + exit 1 + fi + log "test ${type} passed" } # Create a hello world function in nodejs, test it with an http trigger @@ -41,7 +60,6 @@ log "Creating secret" kubectl create secret generic ${fn_secret} --from-literal=TEST_KEY="TESTVALUE" -n default trap "kubectl delete secret ${fn_secret} -n default" EXIT - log "Creating function with secret" fission fn create --name ${fn_secret} --env python --code secret.py --secret ${fn_secret} trap "fission fn delete --name ${fn_secret}" EXIT @@ -52,20 +70,23 @@ fission route create --function ${fn_secret} --url /${fn_secret} --method GET log "Waiting for router to catch up" sleep 5 -log "HTTP GET on the function's route" -res=$(curl http://${FISSION_ROUTER}/${fn_secret}) -val='TESTVALUE' +checkFunctionResponse ${fn_secret} 'TESTVALUE' 'secret' -if [[ ${res} != ${val} ]] -then - log "test secret failed" - cleanup - exit 1 -fi -log "test secret passed" +log "Creating function with newdeploy executorType and new secret value" +kubectl patch secrets ${fn_secret} -p '{"data":{"TEST_KEY":"TkVXVkFMCg=="}}' -n default +fission fn create --name ${fn_secret}-1 --env python --code secret.py --secret ${fn_secret} --executortype newdeploy +trap "fission fn delete --name ${fn_secret}-1" EXIT + +log "Creating route" +fission route create --function ${fn_secret}-1 --url /${fn_secret}-1 --method GET + +log "Waiting for router catch up" +sleep 5 + +checkFunctionResponse ${fn_secret}-1 'NEWVAL' 'secret' log "Creating configmap" -kubectl create configmap ${fn_cfgmap} --from-literal=TEST_KEY=TESTVALUE -n default +kubectl create configmap ${fn_cfgmap} --from-literal=TEST_KEY="TESTVALUE" -n default trap "kubectl delete configmap ${fn_cfgmap} -n default" EXIT log "creating function with configmap" @@ -78,16 +99,20 @@ fission route create --function ${fn_cfgmap} --url /${fn_cfgmap} --method GET log "Waiting for router to catch up" sleep 5 -log "HTTP GET on the function's route" -rescfg=$(curl http://${FISSION_ROUTER}/${fn_cfgmap}) +checkFunctionResponse ${fn_cfgmap} 'TESTVALUE' 'configmap' -if [ ${rescfg} != ${val} ] -then - log "test cfgmap failed" - cleanup - exit 1 -fi -log "test configmap passed" +log "Creating function with newdeploy executorType and new configmap value" +kubectl patch configmap ${fn_cfgmap} -p '{"data":{"TEST_KEY":"NEWVAL"}}' -n default +fission fn create --name ${fn_cfgmap}-1 --env python --code cfgmap.py --configmap ${fn_cfgmap} --executortype newdeploy +trap "fission fn delete --name ${fn_cfgmap}-1" EXIT + +log "Creating route" +fission route create --function ${fn_cfgmap}-1 --url /${fn_cfgmap}-1 --method GET + +log "Waiting for router catch up" +sleep 5 + +checkFunctionResponse ${fn_cfgmap}-1 'NEWVAL' 'configmap' log "testing creating a function without a secret or configmap" fission function create --name ${fn} --env python --code empty.py diff --git a/types.go b/types.go index cb520834..df06bdb0 100644 --- a/types.go +++ b/types.go @@ -354,6 +354,13 @@ const ( StrategyTypeExecution = "execution" ) +const ( + SharedVolumeUserfunc = "userfunc" + SharedVolumePackages = "packages" + SharedVolumeSecrets = "secrets" + SharedVolumeConfigmaps = "configmaps" +) + const ( // FunctionReferenceFunctionName means that the function // reference is simply by function name.