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

Alternative TLS Load Test #5097

Open
wants to merge 8 commits into
base: v3.2.x
Choose a base branch
from

Conversation

delexagon
Copy link

  • I changed src/tests/Makefile because it did not run. First, I fixed an apparent error in generating test.conf (two /s are generated in a directory path). Second, I changed it so that certs/server.pem and certs/ca.pem are generated before the server is started (this may not be desirable behavior). I would recommend looking over the Makefile to see if it's acceptable.
  • I do not know the 'proper way' to add this test to the Makefile, because it seems very different from existing tests. I set it to run under 'all' but not under 'tests' in the testing Makefile.
  • Docker generates an excessive amount of output, which may not be desirable when also running many other tests in a Makefile.
  • I created a different pull request because this is a significantly different, and I think superior, strategy towards using Docker than what I had before.

@delexagon
Copy link
Author

It seems like *.pem files are ignored by git, so certs does not have the necessary files to run the test off the bat. Also, the output directory is empty and isn't created which is awkward, so I'll fix that. Seeing as the Makefile also didn't work because of the lack of a .pem file, I'm not sure whether it's good form for me to generate them myself for the test, even if they will not be used by the application itself.

@alandekok
Copy link
Member

the PEM files are ignored by git. They eventually expire, and then CI fails.

It's better to just regenerate them for the tests.

The test outputs should usually go into ${BUILD_PATH}/tests/tls-load. You can then add a rule:

.PHONY: $(BUILD_PATH)/tests/tls-load
$(BUILD_PATH)/tests/tls-load:
	${Q}mkdir -p $@

And the tests can then depend on $(BUILD_PATH)/tests/tls-load. The make framework will take care of creating the output directory. So you don't need to create an empty output directory and commit it to git.

src/tests/Makefile Outdated Show resolved Hide resolved
@delexagon
Copy link
Author

Alright, I have resolved all of the discussed issues. I am still not confident that this is the right way to deal with Making the test.

@delexagon
Copy link
Author

Also, because I am running .so files directly generated by make within a Docker image, I do not think it will work if the base Docker image used for the test is different than the OS that the user is currently running, which seems to me to be a severe constraint of the script as is. I'm not sure whether it would be worthwhile writing something that attempts to find a docker image associated with the user's current OS and using it as the base?

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.

None yet

2 participants