Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

katatestutils: Use the configured virtiofs daemon path#2687

Merged
fidencio merged 1 commit into
kata-containers:masterfrom
c3d:gh2686-test
Jun 10, 2020
Merged

katatestutils: Use the configured virtiofs daemon path#2687
fidencio merged 1 commit into
kata-containers:masterfrom
c3d:gh2686-test

Conversation

@c3d
Copy link
Copy Markdown
Member

@c3d c3d commented May 13, 2020

The current path is hardcoded as follows:
virtio_fs_daemon = "/path/to/virtiofsd"

Switch to using the value of config.VirtioFSDaemon instead.

Fixes: #2686

Signed-off-by: Christophe de Dinechin dinechin@redhat.com

Copy link
Copy Markdown
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fidencio
Copy link
Copy Markdown
Member

/test

@grahamwhaley
Copy link
Copy Markdown
Contributor

CIs bombed - probably an easy code fix?

17:48:23 INFO: Running 'go test' as current user on package 'github.com/kata-containers/runtime/cli' with flags '-v -race -timeout 30s'
17:48:24 # github.com/kata-containers/runtime/pkg/katatestutils
17:48:24 pkg/katatestutils/utils.go:68:32: config.VirtioFSDaemon undefined (type RuntimeConfigOptions has no field or method VirtioFSDaemon)
17:48:49 FAIL	github.com/kata-containers/runtime/cli [build failed]

@fidencio
Copy link
Copy Markdown
Member

fidencio commented May 14, 2020

Yeah, I see the issue: We can only point to values defined here: https://github.com/kata-containers/runtime/blob/master/pkg/katatestutils/utils.go#L11

So, we'd have to define the variable there as part of this patch.

@c3d, could you address this issue? ^

The reason I think it was as it was is because it's this path is never ever used in the tests? In any case, still worth to fix.

@fidencio
Copy link
Copy Markdown
Member

fidencio commented Jun 5, 2020

/test-ubuntu

@fidencio
Copy link
Copy Markdown
Member

fidencio commented Jun 5, 2020

13:20:33 --- FAIL: TestConfigLoadConfiguration (0.00s)

expected:
VirtioFSDaemon: /tmp/kata-222390771/load-config-579229878/test/virtiofsd

got:
VirtioFSDaemon: /path/to/virtiofsd

And the same happens for

13:20:33 --- FAIL: TestConfigLoadConfigurationFailBrokenSymLink

And for

13:20:33 --- FAIL: TestConfigLoadConfigurationFailSymLinkLoop (0.00s)

Comment thread pkg/katautils/config_test.go Outdated
VhostUserStorePath: defaultVhostUserStorePath,
SharedFS: sharedFS,
VirtioFSDaemon: "/path/to/virtiofsd",
VirtioFSDaemon: virtiofsdaemon,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c3d, here it should be virtioFSdaemon, as that's the naming used above in this file.

Here's a log of travis, which didn't return back to us, pointing to this error:
https://travis-ci.org/github/kata-containers/runtime/jobs/695051557#L1070

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retrying 😄

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Travis has passed, but results were not reported back:
https://travis-ci.org/github/kata-containers/runtime/builds/695129068

The current path is hardcoded as follows:
  virtio_fs_daemon = "/path/to/virtiofsd"

Switch to using the value of config.VirtioFSDaemon instead.

Fixes: kata-containers#2686

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
@fidencio
Copy link
Copy Markdown
Member

fidencio commented Jun 5, 2020

/test-ubuntu

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2020

Codecov Report

Merging #2687 into master will increase coverage by 6.12%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2687      +/-   ##
==========================================
+ Coverage   45.55%   51.67%   +6.12%     
==========================================
  Files         118      118              
  Lines       17283    18960    +1677     
==========================================
+ Hits         7873     9798    +1925     
+ Misses       8533     8020     -513     
- Partials      877     1142     +265     

@fidencio
Copy link
Copy Markdown
Member

fidencio commented Jun 9, 2020

I'm closing and re-opening this PR with the hope that it'll bring us the travis results.

@fidencio fidencio closed this Jun 9, 2020
@fidencio fidencio reopened this Jun 9, 2020
@fidencio
Copy link
Copy Markdown
Member

/test-ubuntu

1 similar comment
@fidencio
Copy link
Copy Markdown
Member

/test-ubuntu

@fidencio
Copy link
Copy Markdown
Member

/test-vfio

@fidencio
Copy link
Copy Markdown
Member

/test-arm

@fidencio
Copy link
Copy Markdown
Member

/test-power

@fidencio
Copy link
Copy Markdown
Member

/test-clh

Copy link
Copy Markdown

@jodh-intel jodh-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @c3d.

lgtm

@fidencio
Copy link
Copy Markdown
Member

jenkins-ubuntu-18-04-vfio:
@devimc, same error as in #2685 (comment)

08:49:03 + ssh_vm 'sudo /home/jenkins/run.sh'
08:49:03 + cmd='sudo /home/jenkins/run.sh'
08:49:03 + ssh -q -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -i /tmp/kata-containers/vfio-test/key -p 10022 jenkins@127.0.15.1 'sudo /home/jenkins/run.sh'
08:49:03 ++ handle_error 260
08:49:03 ++ local exit_code=255
08:49:03 ++ local line_number=260
08:49:03 ++ echo 'Failed at 260: ssh -q -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -i "${ssh_key_file}" -p "${vm_port}" "${USER}@${vm_ip}" "${cmd}"'
08:49:03 Failed at 260: ssh -q -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -i "${ssh_key_file}" -p "${vm_port}" "${USER}@${vm_ip}" "${cmd}"
08:49:03 ++ exit 255
08:49:03 + cleanup
08:49:03 + sudo killall -9 qemu-system-x86_64
08:49:03 Build step 'Execute shell' marked build as failure

It doesn't look related to this patch.

@devimc
Copy link
Copy Markdown

devimc commented Jun 10, 2020

thanks @fidencio , I have filed an issue kata-containers/tests#2628

@fidencio fidencio merged commit 4f48a0e into kata-containers:master Jun 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

katatestutils should use config.VirtioFSDaemon

6 participants