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

wskdeploy failing to read configuration file from user home directory for 386 version #1014

Closed
jthomas opened this issue Nov 28, 2018 · 8 comments
Assignees

Comments

@jthomas
Copy link
Member

jthomas commented Nov 28, 2018

The 386 release of the tool fails to retrieve the user's home directory from the system. This causes it to expect the wskprops file in the current directory rather than the user's home directory.

example

  • Run a docker run -it ubuntu container and retrieve the latest release archive of the tool from the release page.

  • Using a simple manifest run the 386 & amd64 versions of the tool with the -v flag and check the different CfgFile property values.

root@8651a121af02:~/wskdeploy/linux/amd64# ./wskdeploy -v -m manifest.yaml
Info: deploy using deployment file []...

Info: Configuration:
      > ApiHost: []
      > Auth: []
      > Namespace: []
      > ApiVersion: []
      > CfgFile: [/root/.wskprops]
      > CliVersion: [latest]
      > ProjectPath: [.]
      > DeploymentPath: []
      > ManifestPath: [manifest.yaml]
      > Preview: [false]
      > Strict: [false]
      > Key: []
      > Cert: []
      > Managed: [false]
      > ProjectName: []
      > ApigwAccessToken: []
      > Verbose: [true]
      > Trace: [false]
      > Sync: [false]
      > Report: [false]
      > Param: [[]]
      > ParamFile: []
 ./wskdeploy -v -m ../amd64/manifest.yaml
Info: deploy using deployment file []...

Info: Configuration:
      > ApiHost: []
      > Auth: []
      > Namespace: []
      > ApiVersion: []
      > CfgFile: [.wskprops]
      > CliVersion: [latest]
      > ProjectPath: [.]
      > DeploymentPath: []
      > ManifestPath: [../amd64/manifest.yaml]
      > Preview: [false]
      > Strict: [false]
      > Key: []
      > Cert: []
      > Managed: [false]
      > ProjectName: []
      > ApigwAccessToken: []
      > Verbose: [true]
      > Trace: [false]
      > Sync: [false]
      > Report: [false]
      > Param: [[]]
      > ParamFile: []

cause

The cause is due to user.Current method not being available for 386.

Here's a test file to show the issue...

package main

import "fmt"
import "os"
import "os/user"

func main() {
	usr, err := user.Current()
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}

	fmt.Println(usr.HomeDir)
}
# GOOS=linux GOARCH=386 go build main.go
# ./main
user: Current not implemented on linux/386

solution

I've found an existing workaround in another project - that we might want to add:
https://github.com/schachmat/ingo/blob/4b355b487cca8ac509823a2b410a1d9c79fb214c/in.go#L35-L41

func fallbackHome() string {
	if home := os.Getenv("HOME"); home != "" {
		return home
	}
	// For Windows.
	return os.Getenv("UserProfile")
}

This can be used when user.Current() returns an error.

@mrutkows
Copy link
Contributor

mrutkows commented Nov 28, 2018

Thanks James... and it looks easy to slot in that fallback function on error

func GetHomeDirectory() string {
	usr, err := user.Current()
	if err != nil {
	    return fallbackHome()
	}
	return usr.HomeDir
}

@mrutkows
Copy link
Contributor

and I believe while we are at it, we should emit a warning if we drop into looking for a "fallback" method and add Verbose/Trace to indicate where we ended up finding the value we will attempt to use (and the value itself)

@jthomas
Copy link
Member Author

jthomas commented Nov 28, 2018

Looks like the wsk CLI handles this using an external library.
https://github.com/apache/incubator-openwhisk-cli/blob/461f94fafe405feb3c664a43f6c117bac4d3c27f/commands/property.go#L26

We might want to follow that approach instead.

@mrutkows
Copy link
Contributor

mrutkows commented Nov 28, 2018

@jthomas Yeah was grepping around, will look at as well.. the lib. (i.e., https://github.com/mitchellh/go-homedir) it appears to be a single file under MIT license which if we switch to we would need to note.

@mrutkows
Copy link
Contributor

mrutkows commented Nov 28, 2018

@jthomas @pritidesai be aware we believe that the long-term fix (i.e., use a robust library like the one you found) within go-client is the correct fix and that would cause us then to clean up wskdeploy to not have ANY fallback methods. That is, we would only use the ReadProps from go-client exclusively (once it impl. a platform-aware solution) where as today we use a hybrid of the (failing) go-client and rely upon the utils/misc.go code you found

For now, we have a PR to do a quick fix.

Priti volunteered to open issues address this in go-client and then leveraged afterwards in wskdeploy.

@mrutkows
Copy link
Contributor

Looks like James Dubee added this lib. and "Expand()" to the CLI as part of the Go formatting PR: apache/openwhisk-cli@c615efb

@jthomas
Copy link
Member Author

jthomas commented Nov 29, 2018

I've checked the new version and it works - thanks for the quick turnaround.

@jthomas jthomas closed this as completed Nov 29, 2018
@mrutkows
Copy link
Contributor

mrutkows commented Nov 29, 2018

@pritidesai @jthomas Thanks James for bringing this issue to us and also pointing us to the quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants