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

testing: Add vagrant based integration tests #45

Closed
wants to merge 7 commits into from

Conversation

neuhalje
Copy link
Contributor

@neuhalje neuhalje commented Apr 29, 2016

This patch will add test servers to use with t_client.sh. This will make testing on different systems much easier. Bonus: testing on a laptop is a breeze by "one command integration tests".

Example

./run_integration.sh ubuntu_precise64
# does the following
# - start an Ubuntu Precise VM
# - Copy your current source code to the VM
# - configure & make on the VM
# - Run the server on the VM
# - Run t_client.sh *with the REMOTE_HOST pointing to the VM*
# - The outputs of client & server are colorized AND interleaved for better diagnosis
# - Tear down the server

Virtual machines are defined by Vagrant scripts. Virtualbox is used for virtualisation.

Each of the virtual machines is configured to compile openvpn and run a server.

Inside the vm a openvpn server can be launched to run t_client.sh tests against it. Sample launcher scripts are provided.

screen shot 2016-05-09 at 16 17 18

@neuhalje neuhalje force-pushed the integration_tests branch 2 times, most recently from cc5444c to 1ff73f8 Compare May 10, 2016 17:27
@mattock
Copy link
Member

mattock commented May 19, 2016

I've used Vagrant quite a bit in the past, so I'll take care of reviewing this. Right now I don't have VirtualBox installed, so the review will take a bit of time.

@mattock
Copy link
Member

mattock commented May 31, 2016

I tested this a bit with latest Vagrant+Virtualbox and most of it seemed to work fine out of the box - excellent work!

I'm just wondering what kind of test setup we want exactly... right now, the "real" test server runs several OpenVPN server instances configured differently and listening on different ports. Each of the clients (buildslaves) connect to each of the server instances in turn. This is something we might (or might not) want to replicate here.

Another option would be to focus on building and running the server-side code on different VM instances, each running a different OS, and run the tests to ensure that the virtual host can connect to each of them. There could be several differently configured OpenVPN server instances on each of the VMs. Is this what you're trying achieve here?

In any case, I think we should strive for maximum automation, so that any developer is able to recreate the t_client.sh test environment with minimal tinkering and knowledge about t_client.sh. Which brings me to a couple of issues I noticed:

  • It seems that the line RUN_SUDO=sudo in t_client.rc needs to be enabled: at least on Fedora doing a "sudo -s" changes $HOME to /root, which means that $HOME/.vagrant can no longer be found.
  • The EXCEPT*_ values in t_client.rc-sample are not correct for the Vagrant VMs, which causes t_client.sh fail, even though OpenVPN connection is established correctly

What about having a tailored t_client.rc-vagrant file that fixes the two issues above? Making the VPN IP addresses predictable probably takes some effort, but would be worth it imho.

This is probably unrelated to this patch, but I noticed that the "stopping OpenVPN" part in t_client.sh hangs (indefinitely?) on Fedora 23.

@neuhalje
Copy link
Contributor Author

Hi

I tested this a bit with latest Vagrant+Virtualbox and most of it seemed to work fine out of the box - excellent work!

Thanks! And thank you for testing!
I'm just wondering what kind of test setup we want exactly... right now, the "real" test server runs several OpenVPN server instances configured differently and listening on different ports. Each of the clients (buildslaves) connect to each of the server instances in turn. This is something we might (or might not) want to replicate here.

basically “n Client Systems x n Server Systems”? This is what I see as a good case for a “is this ready for a patch submit” test.
Another option would be to focus on building and running the server-side code on different VM instances, each running a different OS, and run the tests to ensure that the virtual host can connect to each of them. There could be several differently configured OpenVPN server instances on each of the VMs. Is this what you're trying achieve here?

basically “1 Client System x n Server Systems”?

Both are viable routes that might not be exclusive (e.g. maybe they are more or less the same with a slightly different configuration)

IMHO the use cases are:

  1. Iterative development (“1 x 1”): One VM as server or client endpoint. The other side on the development machine.

  2. Feature test (“1 x n”): Multiple VMs as server or client endpoint. The other side on the development machine.

  3. Final test (“n x n”): Multiple VMs as server AND client endpoint.

Use case #1 is a MUST. Use case #2 is a SHOULD, and #3 its a COULD

My high level goals:

  1. Make it easy for developers to set up different test & build machines to test new features against. (hypothesis: done with this PR, except for maybe some bugs)

  2. Make integration with existing test tooling as easy as possible (still not quite there with this PR).

  3. Make it easy to separate”Test Definition” from “Execution Environment” (still not quite started with this PR).
    I.e. a test should consist of a client configuration, a server configuration, and (a maybe fixed) test function.
    This is basically what t_client.sh does right now with the addition of automatic address configuration, and automatic server launches.

In any case, I think we should strive for maximum automation, so that any developer is able to recreate the t_client.sh test environment with minimal tinkering and knowledge about t_client.sh. Which brings me to a couple of issues I noticed:

Yes!

It seems that the line RUN_SUDO=sudo in t_client.rc needs to be enabled: at least on Fedora doing a "sudo -s" changes $HOME to /root, which means that $HOME/.vagrant can no longer be found.
—> Bug
The EXCEPT*_ values in t_client.rc-sample are not correct for the Vagrant VMs, which causes t_client.sh fail, even though OpenVPN connection is established correctly
—> Bug

What about having a tailored t_client.rc-vagrant file that fixes the two issues above? Making the VPN IP addresses predictable probably takes some effort, but would be worth it imho

Sounds good. I’ll try to improve the scripts next weekend!
This is probably unrelated to this patch, but I noticed that the "stopping OpenVPN" part in t_client.sh hangs (indefinitely?) on Fedora 23.

—> Bug

Next iteration :-)

Cheers
Jens


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub #45 (comment), or mute the thread https://github.com/notifications/unsubscribe/ADqgEtaQGQc9CwYPmRJDPzAGVy0_-GoCks5qHE9egaJpZM4ISoWt.

@mattock
Copy link
Member

mattock commented Jun 2, 2016

basically “n Client Systems x n Server Systems”? This is what I see as a good case for a “is this ready for a patch submit” test.

Right now we do this after the patch is in Git "master", because buildbot polls the "master" branch. We could and intend to add more testing branches that allow us to trigger builds before pushing anything to master.

IMHO the use cases are:

  1. Iterative development (“1 x 1”): One VM as server or client endpoint. The other side on the development machine.

This is a good start and a reasonable sanity test. We can implement 2) and 3) as needed.

Use case #1 is a MUST. Use case #2 is a SHOULD, and #3 its a COULD

I agree, as we're probably targeting individual developers with Vagrant. The project as a whole can afford more complex testing systems, as those are built only once and the maintenance cost is thus low.

@mattock
Copy link
Member

mattock commented Aug 16, 2016

@neuhalje : any progress on fixing the issues that I identified?

One further issue I noticed is the use of #!/bin/bash in some of the scripts. The scripts might include some bashisms, which need to be removed also. While most developers probably run Linux, Vagrant and Virtualbox also run on FreeBSD, which does not have Bash installed by default. So it would be preferable If the scripts were converted to POSIX shell format to avoid complaints later on.

As the last step the commits could probably be squashed to a few larger ones with no ill effects.

@dsommers dsommers changed the title Add vagrant based integration tests testing: Add vagrant based integration tests Aug 17, 2016
@mattock
Copy link
Member

mattock commented Oct 5, 2016

I'll look into the bashisms to get this moving forward. This PR might benefit from the new t_client.rc EXPECT_IFCONFIG* auto-detection and caching patch that just got merged.

Provide a virtual machine based 'in situ' integration test setup that
integrates t_client.sh with phoenix-style test servers.

E.g. running `tests/server/ubuntu_precise64/launch_t_client.sh` would

- Create a fresh Ubuntu Precise VM
- Install everything needed to build openvpn
- Compile openvpn from the current source
- Start the openvpn server inside the VM
- Optionally: Locally run t_client.sh against this servers IP

Details:

- Each of the virtual machines is configured to compile openvpn and
  run a server from the current source.

- Inside the vm a openvpn server can be launched to run `t_client.sh`
  tests against it. Sample launcher scripts are provided.

- Virtual machines defined by [Vagrant](https://www.vagrantup.com) scripts.
  VirtualBox is used for virtualisation.

Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
This script will prepare (compile and start openvpn) a VM, run t_client.sh
against it, and then halt the VM.

The local and remote output is merged in the console and colorized to highlight
the interaction between client and server.

Tested on MacOS X.

Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Split up sourcing of t_client.rc and running sudo. Also always source
tc_client.sh regardless of we already run as root (preparation for
further usage of t_client.rc parameters).
@neuhalje
Copy link
Contributor Author

Ok, back again. I'm going to try to get the scripts ksh compatible.

@neuhalje
Copy link
Contributor Author

@mattock : Samuli, I have been absent from OpenVPN development for a few, aehm, days. Mayber you could help me out:

I already removed the EXPECT_IFCONFIG* statements from t_client.rc-sample. Is that enough to implement df0b00c?

@mattock
Copy link
Member

mattock commented Jan 16, 2017

@neuhalje : it's been a while since I implemented the IP caching, but two things need to be done:

  • Remove EXCEPT_IFCONFIG entries from t_client.rc
  • Include t_client_ips.rc in t_client.rc (see t_client.rc-sample): otherwise t_client.sh will add duplicate EXCEPT_IFCONFIG lines to t_client_ips.rc on every run.

@cron2
Copy link
Contributor

cron2 commented Jan 17, 2017

Independent from the rest of this, the "run tests needing 'ld --wrap' only where supported" sounds like something we want to have right away... can you send that one as standalone patch relative to master to the -devel list? thanks

(Actually, I want the functionality, but we should check whether ld --wrap works, instead of just declaring "only on Linux" - FreeBSD has clang with the GNU ld, so it works perfectly fine there, for example. So the first hunk needs to be "configure runs a ld --wrap test" not "on Linux")

--wrap is a feature of ld that is only available on Linux systems.
http://www.mockator.com/projects/mockator/wiki/Wrap_Functions

"Alas, this feature is *only available with the GNU tool chain on Linux*.
GCC for Mac OS X does not offer the linker flag --wrap. "
@neuhalje
Copy link
Contributor Author

I pushed the 'ld --wrap' fix as PR #79
It still needs some test though

@neuhalje
Copy link
Contributor Author

neuhalje commented Jan 17, 2017

@cron2 Gert, any clue on how to run a ld --wrap-works test? My autotools skills are sub-zero

@cron2
Copy link
Contributor

cron2 commented Jan 18, 2017 via email

@chipitsine
Copy link
Contributor

[ilia@localhost ~]$ ld --help | grep wrap
  --wrap SYMBOL               Use wrapper functions for SYMBOL
[ilia@localhost ~]$ 

@neuhalje
Copy link
Contributor Author

on macos:

 ld --help
ld64: For information on command line options please use 'man ld'.

@chipitsine
Copy link
Contributor

try 'ld --help 2>/dev/null'

@neuhalje
Copy link
Contributor Author

Let's continue the '--wrap' topic on PR#79

@chipitsine
Copy link
Contributor

chipitsine commented Jan 18, 2017

sorry, I did not notice "vagrant" in the topic

@neuhalje
Copy link
Contributor Author

probably dead by now .. cleaning up

@neuhalje neuhalje closed this Jun 12, 2020
@mattock
Copy link
Member

mattock commented Jun 16, 2020

This PR was a good idea and I intended to get this merged. But I had never time to do it, nor does it look likely that I ever will 😞.

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 this pull request may close these issues.

4 participants