GoogleCloudPlatform / marketplace-k8s-app-example

Apache License 2.0
23 stars 19 forks source link

Write basic tester for nginx #53

Closed vcanaa closed 6 years ago

moxious commented 6 years ago

I'm at the phase of trying to write my tester container. The skeleton that's provided (I'm looking mainly at nginx here https://github.com/GoogleCloudPlatform/marketplace-k8s-app-example/tree/master/nginx/apptest) seems like a good starting point.

I noticed several things that are missing, where it doesn't seem this tester can yet work. I'm also having a bit of a rough time connecting the concepts I'm seeing in the code to the onboarding guide. In particular:

Sorry if this is a dupe, I brought some of these issues up in comments on the onboarding guide. Then I noticed this issue, I'm guessing that this tester just isn't at a usable state yet?

vcanaa commented 6 years ago

Hi moxious,

Thanks for running through this so diligently. I addressed some of your comments on the onboarding doc.

You are right, the tests writen so far are dummies. I am replacing them soon. They were first writen to validate the app/verify command.

I think one important thing the doc is missing is the general idea of how we assert that the app works.

We require the app to provide a tester job, that is all.

The nested chart is used to overlay the charts in the deployer in test mode. In most cases we expect it to simply contain the tester job. But it is also capable of overwriting some of the app charts if needed.

moxious commented 6 years ago

@vcanaa I see the code you merged in that PR. Apologies if I'm missing something here, but I'm having a hard time connecting some items we discussed on a call about the testing approach, and the template code I'm seeing here.

What I've been thinking is that the test container should be layered, that is it's everything that's in the deployment container, plus additional stuff necessary for testing.

In the PR code, particularly this bit right here: https://github.com/GoogleCloudPlatform/marketplace-k8s-app-example/blob/master/nginx/Makefile#L71

What I see is that the docker container is an unrelated separate container, I see how the bash/curl tests are supposed to work, but I'm missing the bigger integrated picture here. Namely:

(1) for this test container, it didn't include the original YAML templates for resources to deploy, and so its seems to assume that the test container runs separately with the assumption the application was pre-deployed (that seems not to match what we discussed on the call) (2) I don't see the mechanism of how the test container even gets run. Within marketplace tools, there's a set of related scripts without any comments at the top that are a bit hard to decipher. For example, there's start.sh, driver.sh, deploy.sh, deploy_with_tests.sh, and others. I've been using start.sh to simulate what marketplace will do with my deployer. It seems using start.sh for the testing approach probably isn't' right, because I don't see any additional flags in that script to indicate that.

So is this github issue still open because you're planning more mods here, or am I misunderstanding something?

huyhg commented 6 years ago

Hi David,

(1) The resources for deploying the actual test is here. If you have an existing helm test (with test-success hook), the framework should also recognize it and deploy the test during integration testing.

(2) make app/verify should be used to run an integration test. If you look at that target, it calls driver.sh. The driver can be invoked manually as well--the parameters just need to be filled in similar to calling start.sh.

driver.sh creates an isolated namespace to deploy the test app into. It in turns also uses start.sh to create the deployer container, but with a new entrypoint /bin/deploy_with_tests.sh (the default entrypoint is /bin/deploy.sh). This entrypoint, in addition to deploying the regular app similar to deploy.sh, it will wait for the app to become ready and run the tests as defined in (1). The driver simply waits for the deployer to finish.

(1) should be the only thing you need to do to add tests.

I will file a follow up issue to have better documentation in our tools.

Also, AFAICT, this issue is closed by the referenced PR #67.

vcanaa commented 6 years ago

Thanks Huy for the explanation.

I would also like to bring attention to this, where I believe there is some confusion:

What I've been thinking is that the test container should be layered, that is it's everything that's in the deployment container, plus additional stuff necessary for testing.

That tester container in the example is a container that runs curl GET and do asserts on the response. Not to be confused with the deployer.

moxious commented 6 years ago

OK. The answers you guys are giving at first confused me more, but poking into it a bit a missing piece in my understanding is that app/verify isn't defined by any of your sample applications, it's defined in app.Makefile as part of the marketplace tools. Hence all of the references to driver.sh also can't be found in the sample applications, which had me scratching my head a lot.

Separate issues are being filed for some small things I'm running into as part of that, I can tell you guys are mostly on a linux baseline. ;)

huyhg commented 6 years ago

Thanks David. It looks like we have proper forums to discuss bugs for app/verify in other issues. I'm closing this issue because the original issue this was opened for has been addressed.