From f8e98f9f1405689f4b71b99999431d2e7a0ad08f 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 ecfc66a3..3f8fb923 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 db2808cc..34628dc2 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 d57b180e..f6d2e796 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 ac4133d7..a737035a 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 9ccf174c..56bebdc3 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 e0c9ca24..d2871dcf 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 a7393989..8c6df24e 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 59d3414d..5166bff3 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 cd8d4263..914fb3c7 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 417a1e87..955f9224 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."