From 2542f55d15d84a3992e3010fb3692f0b86c61320 Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Mon, 18 Nov 2019 10:05:49 +0800 Subject: [PATCH] Always embed URL provided by user in the archive (#1413) To keep archive creation implementation simple and prevent any confusion, we decided to remove `--keepurl` flag and embed URL directly without downloading the file from it . In this way, we can ensure consistent behavior in either package creation or spec file creation. Also, it increases the portability of spec file. --- pkg/fission-cli/cmd/function/command.go | 4 +-- pkg/fission-cli/cmd/function/create.go | 6 ++--- pkg/fission-cli/cmd/function/update.go | 4 +-- pkg/fission-cli/cmd/package/command.go | 4 +-- pkg/fission-cli/cmd/package/create.go | 9 +++---- pkg/fission-cli/cmd/package/package.go | 33 ++++++++----------------- pkg/fission-cli/cmd/package/update.go | 10 +++----- pkg/fission-cli/flag/flag.go | 2 -- pkg/fission-cli/flag/key/key.go | 1 - test/tests/test_create_fn_with_url.sh | 17 ------------- 10 files changed, 26 insertions(+), 64 deletions(-) diff --git a/pkg/fission-cli/cmd/function/command.go b/pkg/fission-cli/cmd/function/command.go index ecfc66a38..3f8fb923b 100644 --- a/pkg/fission-cli/cmd/function/command.go +++ b/pkg/fission-cli/cmd/function/command.go @@ -38,7 +38,7 @@ func Commands() *cobra.Command { // TODO retired pkg related flag from function cmd flag.FnCode, flag.PkgSrcArchive, flag.PkgDeployArchive, - flag.HtUrl, flag.HtMethod, flag.FnKeepURL, + flag.HtUrl, flag.HtMethod, // flag for newdeploy to use. flag.RunTimeMinCPU, flag.RunTimeMaxCPU, flag.RunTimeMinMemory, @@ -84,7 +84,7 @@ func Commands() *cobra.Command { flag.FnSpecializationTimeout, flag.FnExecutionTimeout, flag.FnCode, flag.PkgSrcArchive, flag.PkgDeployArchive, - flag.FnKeepURL, flag.FnBuildCmd, flag.PkgForce, + flag.FnBuildCmd, flag.PkgForce, flag.RunTimeMinCPU, flag.RunTimeMaxCPU, flag.RunTimeMinMemory, flag.RunTimeMaxMemory, flag.ReplicasMin, flag.ReplicasMax, diff --git a/pkg/fission-cli/cmd/function/create.go b/pkg/fission-cli/cmd/function/create.go index db2808cc0..34628dc2f 100644 --- a/pkg/fission-cli/cmd/function/create.go +++ b/pkg/fission-cli/cmd/function/create.go @@ -169,11 +169,11 @@ func (opts *CreateSubCommand) complete(input cli.Input) error { } buildcmd := input.String(flagkey.PkgBuildCmd) - keepURL := input.Bool(flagkey.PkgKeepURL) + pkgName := fmt.Sprintf("%v-%v", fnName, uuid.NewV4().String()) // create new package in the same namespace as the function. - pkgMetadata, err = _package.CreatePackage(input, opts.client, fnName, fnNamespace, envName, envNamespace, - srcArchiveFiles, deployArchiveFiles, buildcmd, specDir, opts.specFile, noZip, keepURL) + pkgMetadata, err = _package.CreatePackage(input, opts.client, pkgName, fnNamespace, envName, envNamespace, + srcArchiveFiles, deployArchiveFiles, buildcmd, specDir, opts.specFile, noZip) if err != nil { return errors.Wrap(err, "error creating package") } diff --git a/pkg/fission-cli/cmd/function/update.go b/pkg/fission-cli/cmd/function/update.go index d57b180e3..f6d2e7969 100644 --- a/pkg/fission-cli/cmd/function/update.go +++ b/pkg/fission-cli/cmd/function/update.go @@ -226,9 +226,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error { return errors.New("package is used by multiple functions, use --force to force update") } - keepURL := input.Bool(flagkey.PkgKeepURL) - - pkgMetadata, err = _package.UpdatePackage(opts.client, pkg, envName, envNamespace, srcArchiveFiles, deployArchiveFiles, buildcmd, false, codeFlag, keepURL) + pkgMetadata, err = _package.UpdatePackage(opts.client, pkg, envName, envNamespace, srcArchiveFiles, deployArchiveFiles, buildcmd, false, codeFlag) if err != nil { return errors.Wrap(err, fmt.Sprintf("error updating package '%v'", pkgName)) } diff --git a/pkg/fission-cli/cmd/package/command.go b/pkg/fission-cli/cmd/package/command.go index ac4133d7d..a737035af 100644 --- a/pkg/fission-cli/cmd/package/command.go +++ b/pkg/fission-cli/cmd/package/command.go @@ -32,7 +32,7 @@ func Commands() *cobra.Command { wrapper.SetFlags(createCmd, flag.FlagSet{ Required: []flag.Flag{flag.PkgEnvironment}, Optional: []flag.Flag{flag.PkgName, flag.PkgSrcArchive, flag.PkgDeployArchive, - flag.PkgKeepURL, flag.PkgBuildCmd, flag.NamespacePackage, flag.NamespaceEnvironment}, + flag.PkgBuildCmd, flag.NamespacePackage, flag.NamespaceEnvironment}, }) getSrcCmd := &cobra.Command{ @@ -63,7 +63,7 @@ func Commands() *cobra.Command { wrapper.SetFlags(updateCmd, flag.FlagSet{ Required: []flag.Flag{flag.PkgName}, Optional: []flag.Flag{flag.PkgEnvironment, flag.PkgSrcArchive, flag.PkgDeployArchive, - flag.PkgKeepURL, flag.PkgBuildCmd, flag.PkgForce, flag.NamespacePackage, flag.NamespaceEnvironment}, + flag.PkgBuildCmd, flag.PkgForce, flag.NamespacePackage, flag.NamespaceEnvironment}, }) deleteCmd := &cobra.Command{ diff --git a/pkg/fission-cli/cmd/package/create.go b/pkg/fission-cli/cmd/package/create.go index 9ccf174c7..56bebdc32 100644 --- a/pkg/fission-cli/cmd/package/create.go +++ b/pkg/fission-cli/cmd/package/create.go @@ -70,21 +70,20 @@ func (opts *CreateSubCommand) run(input cli.Input) error { srcArchiveFiles := input.StringSlice(flagkey.PkgSrcArchive) deployArchiveFiles := input.StringSlice(flagkey.PkgDeployArchive) buildcmd := input.String(flagkey.PkgBuildCmd) - keepURL := input.Bool(flagkey.PkgKeepURL) if len(srcArchiveFiles) == 0 && len(deployArchiveFiles) == 0 { return errors.Errorf("need --%v or --%v flag", flagkey.PkgSrcArchive, flagkey.PkgDeployArchive) } _, err := CreatePackage(input, opts.client, pkgName, pkgNamespace, envName, envNamespace, - srcArchiveFiles, deployArchiveFiles, buildcmd, "", "", false, keepURL) + srcArchiveFiles, deployArchiveFiles, buildcmd, "", "", false) return err } // TODO: get all necessary value from CLI input directly func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNamespace string, envName string, envNamespace string, - srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, specDir string, specFile string, noZip bool, keepURL bool) (*metav1.ObjectMeta, error) { + srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, specDir string, specFile string, noZip bool) (*metav1.ObjectMeta, error) { pkgSpec := fv1.PackageSpec{ Environment: fv1.EnvironmentReference{ @@ -98,7 +97,7 @@ func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNa if len(specFile) > 0 { // we should do this in all cases, i think pkgStatus = fv1.BuildStatusNone } - deployment, err := CreateArchive(client, deployArchiveFiles, noZip, keepURL, specDir, specFile) + deployment, err := CreateArchive(client, deployArchiveFiles, noZip, specDir, specFile) if err != nil { return nil, err } @@ -108,7 +107,7 @@ func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNa } } if len(srcArchiveFiles) > 0 { - source, err := CreateArchive(client, srcArchiveFiles, false, keepURL, specDir, specFile) + source, err := CreateArchive(client, srcArchiveFiles, false, specDir, specFile) if err != nil { return nil, err } diff --git a/pkg/fission-cli/cmd/package/package.go b/pkg/fission-cli/cmd/package/package.go index e0c9ca247..d2871dcf3 100644 --- a/pkg/fission-cli/cmd/package/package.go +++ b/pkg/fission-cli/cmd/package/package.go @@ -40,7 +40,7 @@ import ( // create an archive upload spec in the specs directory; otherwise // upload the archive using client. noZip avoids zipping the // includeFiles, but is ignored if there's more than one includeFile. -func CreateArchive(client *client.Client, includeFiles []string, noZip bool, keepURL bool, specDir string, specFile string) (*fv1.Archive, error) { +func CreateArchive(client *client.Client, includeFiles []string, noZip bool, specDir string, specFile string) (*fv1.Archive, error) { errs := utils.MultiErrorWithFormat() fileURL := "" @@ -49,8 +49,7 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee // ignore http files if utils.IsURL(path) { if len(includeFiles) > 1 { - // It's intentional to disallow the user to provide file - // and URL at the same time even the keepurl is false. + // It's intentional to disallow the user to provide file and URL at the same time. return nil, errors.New("unable to create an archive that contains both file and URL") } fileURL = path @@ -73,13 +72,11 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee } if len(specFile) > 0 { - var archive fv1.Archive - if len(fileURL) > 0 { - archive = fv1.Archive{ + return &fv1.Archive{ Type: fv1.ArchiveTypeUrl, URL: fileURL, - } + }, nil } else { // create an ArchiveUploadSpec and reference it from the archive aus := &spectypes.ArchiveUploadSpec{ @@ -102,30 +99,20 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee return nil, errors.Wrapf(err, "write spec file %v", specFile) } } - // create the archive object - archive = fv1.Archive{ + archive := fv1.Archive{ Type: fv1.ArchiveTypeUrl, URL: fmt.Sprintf("%v%v", spec.ARCHIVE_URL_PREFIX, aus.Name), } + return &archive, nil } - - return &archive, nil } if len(fileURL) > 0 { - if keepURL { - return &fv1.Archive{ - Type: fv1.ArchiveTypeUrl, - URL: fileURL, - }, nil - } - // download the file before we archive it - dst, err := pkgutil.DownloadToTempFile(fileURL) - if err != nil { - return nil, err - } - includeFiles = []string{dst} + return &fv1.Archive{ + Type: fv1.ArchiveTypeUrl, + URL: fileURL, + }, nil } archivePath, err := makeArchiveFile("", includeFiles, noZip) diff --git a/pkg/fission-cli/cmd/package/update.go b/pkg/fission-cli/cmd/package/update.go index a7393989f..8c6df24e0 100644 --- a/pkg/fission-cli/cmd/package/update.go +++ b/pkg/fission-cli/cmd/package/update.go @@ -40,7 +40,6 @@ type UpdateSubCommand struct { srcArchiveFiles []string deployArchiveFiles []string buildcmd string - keepURL bool } func Update(input cli.Input) error { @@ -71,7 +70,6 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error { opts.srcArchiveFiles = input.StringSlice(flagkey.PkgSrcArchive) opts.deployArchiveFiles = input.StringSlice(flagkey.PkgDeployArchive) opts.buildcmd = input.String(flagkey.PkgBuildCmd) - opts.keepURL = input.Bool(flagkey.PkgKeepURL) return nil } @@ -106,7 +104,7 @@ func (opts *UpdateSubCommand) run(input cli.Input) error { newPkgMeta, err := UpdatePackage(opts.client, pkg, opts.envName, opts.envNamespace, opts.srcArchiveFiles, - opts.deployArchiveFiles, opts.buildcmd, false, false, opts.keepURL) + opts.deployArchiveFiles, opts.buildcmd, false, false) if err != nil { return errors.Wrap(err, "update package") } @@ -125,7 +123,7 @@ func (opts *UpdateSubCommand) run(input cli.Input) error { } func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespace string, - srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, forceRebuild bool, noZip bool, keepURL bool) (*metav1.ObjectMeta, error) { + srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, forceRebuild bool, noZip bool) (*metav1.ObjectMeta, error) { needToBuild := false @@ -145,7 +143,7 @@ func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespac } if len(srcArchiveFiles) > 0 { - srcArchive, err := CreateArchive(client, srcArchiveFiles, false, keepURL, "", "") + srcArchive, err := CreateArchive(client, srcArchiveFiles, false, "", "") if err != nil { return nil, err } @@ -154,7 +152,7 @@ func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespac } if len(deployArchiveFiles) > 0 { - deployArchive, err := CreateArchive(client, deployArchiveFiles, noZip, keepURL, "", "") + deployArchive, err := CreateArchive(client, deployArchiveFiles, noZip, "", "") if err != nil { return nil, err } diff --git a/pkg/fission-cli/flag/flag.go b/pkg/fission-cli/flag/flag.go index 59d3414d0..5166bff30 100644 --- a/pkg/fission-cli/flag/flag.go +++ b/pkg/fission-cli/flag/flag.go @@ -89,7 +89,6 @@ var ( FnSpecializationTimeout = Flag{Type: Int, Name: flagkey.FnSpecializationTimeout, Aliases: []string{"st"}, Usage: "Timeout for executor to wait for function pod creation", DefaultValue: fv1.DefaultSpecializationTimeOut} FnEnvName = Flag{Type: String, Name: flagkey.FnEnvironmentName, Usage: "Environment name for function"} FnCode = Flag{Type: String, Name: flagkey.FnCode, Usage: "Local path or URL for single file source code"} - FnKeepURL = Flag{Type: Bool, Name: flagkey.PkgKeepURL, Aliases: []string{"keepurl"}, Usage: "Keep the providing URL in archive instead of downloading file from it. (If set, no checksum will be generated for file integrity check. You must ensure the file won't be changed.)"} FnPkgName = Flag{Type: String, Name: flagkey.FnPackageName, Aliases: []string{"pkg"}, Usage: "Name of the existing package (--deploy and --src and --env will be ignored), should be in the same namespace as the function"} FnEntryPoint = Flag{Type: String, Name: flagkey.FnEntrypoint, Aliases: []string{"entry"}, Usage: "Entry point for environment v2 to load with"} FnBuildCmd = Flag{Type: String, Name: flagkey.FnBuildCmd, Usage: "Package build command for builder to run with"} @@ -153,7 +152,6 @@ var ( PkgName = Flag{Type: String, Name: flagkey.PkgName, Usage: "Package name"} PkgForce = Flag{Type: Bool, Name: flagkey.PkgForce, Short: "f", Usage: "Force update a package even if it is used by one or more functions"} PkgEnvironment = Flag{Type: String, Name: flagkey.PkgEnvironment, Usage: "Environment name"} - PkgKeepURL = Flag{Type: Bool, Name: flagkey.PkgKeepURL, Aliases: []string{"keepurl"}, Usage: "Keep the providing URL in archive instead of downloading file from it. (If set, no checksum will be generated for file integrity check. You must ensure the file won't be changed.)"} PkgBuildCmd = Flag{Type: String, Name: flagkey.PkgBuildCmd, Usage: "Build command for builder to run with"} PkgOutput = Flag{Type: String, Name: flagkey.PkgOutput, Short: "o", Usage: "Output filename to save archive content"} PkgStatus = Flag{Type: String, Name: flagkey.PkgStatus, Usage: `Filter packages by status`} diff --git a/pkg/fission-cli/flag/key/key.go b/pkg/fission-cli/flag/key/key.go index cd8d42635..914fb3c71 100644 --- a/pkg/fission-cli/flag/key/key.go +++ b/pkg/fission-cli/flag/key/key.go @@ -109,7 +109,6 @@ const ( PkgEnvironment = "env" PkgSrcArchive = "sourcearchive" PkgDeployArchive = "deployarchive" - PkgKeepURL = "keeparchiveurl" PkgBuildCmd = "buildcmd" PkgOutput = Output PkgStatus = "status" diff --git a/test/tests/test_create_fn_with_url.sh b/test/tests/test_create_fn_with_url.sh index 417a1e87c..955f92245 100755 --- a/test/tests/test_create_fn_with_url.sh +++ b/test/tests/test_create_fn_with_url.sh @@ -51,21 +51,4 @@ response=$(curl --retry 5 http://$FISSION_ROUTER/$fn) log "Checking for valid response" echo $response | grep -i hello -log "Update function with file URL" -fission fn update --name $fn --env $env --code ${code_url} --keepurl - -pkg=$(kubectl -n default get functions ${fn} -o yaml|grep hello-js|awk '{print $2}') -url=$(kubectl -n default get packages ${pkg} -o yaml|grep "://"|awk '{print $2}') - -if [ ${url} != ${code_url} ]; then - log "have different code url: ${url} vs. ${code_url}" - exit 1 -fi - -log "Doing an HTTP GET on the function's route" -response=$(curl --retry 5 http://$FISSION_ROUTER/$fn) - -log "Checking for valid response" -echo $response | grep -i hello - log "All done."