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

ssh deployer errors with "unknown keys [location_file]" #100

Closed
aranw opened this issue Mar 4, 2023 · 16 comments · Fixed by #101
Closed

ssh deployer errors with "unknown keys [location_file]" #100

aranw opened this issue Mar 4, 2023 · 16 comments · Fixed by #101
Assignees

Comments

@aranw
Copy link

aranw commented Mar 4, 2023

I've tried both setting up my own test weaver application and using the collatz example and both times when running

❯ weaver ssh deploy weaver.toml

I get the following error message

unable to parse ssh config: section "ssh" has unknown keys [locations_file]

I've tried looking at the code but nothing seems to stand out to me at a quick glance as to why this is erroring. I don't fully understand how all the code works just yet but thought I'd raise the issue and continue debugging

@rgrandl rgrandl self-assigned this Mar 4, 2023
@dnephin
Copy link
Contributor

dnephin commented Mar 4, 2023

Nice bug report! I'm just learning the code as well, but I may have an idea about this problem.

type sshConfigSchema struct {
LocationsFile string `serverweaver_metric:"locations_file"`
}
parsed := &sshConfigSchema{}
if err := runtime.ParseConfigSection(sshKey, shortSSHKey, app, parsed); err != nil {
return nil, fmt.Errorf("unable to parse ssh config: %w", err)

I believe the serverweaver_metric struct tag on line 182 is wrong. It doesn't appear anywhere else in the code, and ParseConfigSection (which receives that struct) seems to use https://pkg.go.dev/github.com/burntsushi/toml#Decode to decode the config. That function expects a toml struct tag.

That leaves the locations_file field from the config as "undecoded", which causes the error here:

weaver/runtime/config.go

Lines 87 to 88 in 35727ae

if unknown := md.Undecoded(); len(unknown) != 0 {
return fmt.Errorf("section %q has unknown keys %v", key, unknown)

I suspect this diff should fix it, but I'm not familiar enough with the testing of this package to add test coverage for the change:

diff --git a/internal/tool/ssh/deploy.go b/internal/tool/ssh/deploy.go
index 6a6e276..9be6534 100644
--- a/internal/tool/ssh/deploy.go
+++ b/internal/tool/ssh/deploy.go
@@ -179,7 +179,7 @@ func getLocations(app *protos.AppConfig) ([]string, error) {
 	const shortSSHKey = "ssh"
 
 	type sshConfigSchema struct {
-		LocationsFile string `serverweaver_metric:"locations_file"`
+		LocationsFile string `toml:"locations_file"`
 	}
 	parsed := &sshConfigSchema{}
 	if err := runtime.ParseConfigSection(sshKey, shortSSHKey, app, parsed); err != nil {

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 4, 2023

Hi @aranw. Thanks for pointing out the issue. @dnephin this is one of the problem, great find :). There is another small issue we introduced while renaming the project. I am creating a fix.

@aranw
Copy link
Author

aranw commented Mar 4, 2023

Ah thanks for finding this so quickly. I didn't get too deep into identifying the issue before having to go out. I'll try this diff either later tonight or tomorrow at some point

@rgrandl rgrandl linked a pull request Mar 4, 2023 that will close this issue
@rgrandl
Copy link
Collaborator

rgrandl commented Mar 4, 2023

Sorry for the inconvenience. Please try the diff and let us know if you have any issues running the SSH deployer.

@aranw
Copy link
Author

aranw commented Mar 4, 2023

Yeah that looks like it has progressed a little further now onto a new error relating to creating deployment directory

Thanks for the quick turn around with a fix!

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 4, 2023

Hmm, can you share the error? I tried on my local machine and it works fine. Are you testing on your local machine or on multiple machines?

@aranw
Copy link
Author

aranw commented Mar 4, 2023

It is a remote server

unable to create deployment directory at location #####: exit status 1

##### is the name of my host alias in my ssh config file

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 4, 2023

Got it. We create a deployment directory on each remote machine where Service Weaver will be deployed. Right now we try to create the directory using ssh and the location is os.TempDir()/deploymentId. https://github.com/ServiceWeaver/weaver/blob/main/internal/tool/ssh/deploy.go#L147

Can you try if simply creating the remote directory over SSH works? e.g., ssh #### "mkdir -p /tmp/bla". I am wondering whether you have some permission issues. Are you running linux machines?

@aranw
Copy link
Author

aranw commented Mar 4, 2023

@rgrandl yeah running that command it worked fine

@aranw
Copy link
Author

aranw commented Mar 4, 2023

I added some output to the command to see what the tool was doing and this seems to be the error

mkdir: cannot create directory ‘/var/folders’: Permission denied

Not sure where it is getting /var/folders yet need to trace the code a bit more

@aranw
Copy link
Author

aranw commented Mar 4, 2023

This is the command it is running

I'm guessing this is a folder on my local machine

/usr/bin/ssh -v ### mkdir -p /var/folders/sq/3sdsh9n14p790p7xwhdlvdzc0000gn/T/915574ab-82d5-47f1-8efa-47f5062f03cf

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 4, 2023

Hmm, this is interesting. On linux, the temp dir is /tmp. What is your setup and which OS are you running? It feels like macOS?

It seems like your ssh client doesn't have privileges to create a directory in /var/folders, maybe? Alternatively you can try to change the os.tempDir() to a custom directory where you can do writes over SSH and check if it works for now.

I have a macOS laptop and I will give it a try as well. To be fair, we tested the SSH deployer only on linux machines.

@aranw
Copy link
Author

aranw commented Mar 4, 2023

Ahh so I understand what is happening now

remoteDepDir := filepath.Join(os.TempDir(), dep.Id)

On this line you call os.TempDir() but because I am running this from my laptop (macOS) that call to os.TempDir() returns a temp directory path for my local machine not for the remote ssh host that I am connecting too

Edit: I can create a separate issue for this if you want?

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 5, 2023

That's a good point. Yeah, we didn't try to deploy it using SSH from our laptop on a cluster. Thanks for the finding. Sure, send us a fix. Thanks for fixing this :).

@aranw
Copy link
Author

aranw commented Mar 5, 2023

@rgrandl I'll write up a new issue and little summary of the problem later today. I did some digging into the issue last night but was late for me (GMT timezone) so ended up not working on a fix. I have an idea for a possible fix but will discuss in more detail a possible solution in the issue later

@rgrandl
Copy link
Collaborator

rgrandl commented Mar 5, 2023

@aranw thanks a lot for your help. Sounds good.

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

Successfully merging a pull request may close this issue.

3 participants