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

chore(test) add test for HealthCheck policy #1987

Merged
merged 19 commits into from
May 21, 2021
Merged

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented May 14, 2021

Summary

Besides the test current PR introduces a new binary test-server. It's built using cobra so could be easily extended for other purposes. Now it has only one command:

$ test-server health-check --send foo --recv bar

which runs a server that can respond to health checker messages over TCP.

This binary is embedded into kuma-universal image. I don't think we have to build it (and kuma-universal as well) when running general targets. So the targets like:

  • make build
  • make images
  • make docker/build
  • make docker/load
  • make docker/save

should not build auxiliary binaries and images that are needed only for testing. Probably we have to create separate targets for all binaries that we're not shipping like make test/build, make test/images, or something like this. Right now I just explicitly call make docker/build/kuma-universal where needed.

Maybe there are should be 2 types of targets:

release/build: build/kuma-cp build/kuma-dp build/kuma-init build/kuma-prometheus

test/build: build/test-listener

And the general target for all

build: release/build test/build

@kumahq/kuma-maintainers what do you think?

Full changelog

  • add a new binary test-server
  • add e2e test for HealthCheck

Issues resolved

Fix #XXX

Documentation

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya requested a review from a team as a code owner May 14, 2021 05:51
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a comment

Choose a reason for hiding this comment

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

I'm ok with idea of having separate targets

test/e2e/healthcheck/healthcheck_universal.go Outdated Show resolved Hide resolved
test/e2e/healthcheck/healthcheck_universal.go Outdated Show resolved Hide resolved
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
mk/test.mk Outdated Show resolved Hide resolved
test/e2e/healthcheck/universal/policy_http.go Outdated Show resolved Hide resolved
test/e2e/healthcheck/universal/policy_http.go Show resolved Hide resolved
test/e2e/healthcheck/universal/policy_http.go Outdated Show resolved Hide resolved
// wait cluster 'test-server' to be marked as unhealthy
Eventually(func() bool {
cmd := []string{"/bin/bash", "-c", "\"curl localhost:30001/clusters | grep test-server\""}
stdout, _, err := cluster.ExecWithRetries("", "", "dp-demo-client", cmd...)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just Exec? Do we need retries on top of retries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically these are different retries: ExecWithRetries will retry if you failed to exec cmd, ssh problem, or something like that, whereas Eventually waits correct response from localhost:30001/clusters

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya merged commit 5a8d5bc into master May 21, 2021
@lobkovilya lobkovilya deleted the chore/healthcheck-test branch May 21, 2021 09:42
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