Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

remove healthcheck and cleanup controller flags #899

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

kschoche
Copy link
Contributor

@kschoche kschoche commented Apr 7, 2021

Changes proposed in this PR:

How I've tested this PR:
bats tests, these flags have defaults in consul-k8s so not passing them should still pass.

How I expect reviewers to test this PR:
bats tests

Checklist:

  • Bats tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@kschoche kschoche requested a review from a team April 7, 2021 15:43
@kschoche kschoche self-assigned this Apr 7, 2021
@kschoche kschoche requested review from ndhanushkodi and thisisnotashwin and removed request for a team April 7, 2021 15:43
}

@test "client/ConfigMap: check_update_interval is set when health checks enabled" {
@test "client/ConfigMap: check_update_interval is set when connect is enabled" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that health checks are "always on" we will do this anytime connect is enabled.

update ci to point to latest feature-tproxy build
@kschoche kschoche force-pushed the tproxy-remove-cleanup-hc-controller-flags branch from 0f42db9 to ea056a4 Compare April 12, 2021 14:29
@kschoche kschoche marked this pull request as ready for review April 12, 2021 15:40
Copy link
Contributor

@ndhanushkodi ndhanushkodi left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -93,8 +93,8 @@ func TestConnectInject(t *testing.T) {
}
}

// Test the cleanup controller that cleans up force-killed pods.
func TestConnectInject_CleanupController(t *testing.T) {
// Test the endpoints controller cleans up force-killed pods.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Are there any healthchecks-specific acceptance tests? I didn't see any when looking but thought I'd ask.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. The health check test is part of the standard connect test (there are some extra assertions about the health checks).

@@ -93,8 +93,8 @@ func TestConnectInject(t *testing.T) {
}
}

// Test the cleanup controller that cleans up force-killed pods.
func TestConnectInject_CleanupController(t *testing.T) {
// Test the endpoints controller cleans up force-killed pods.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1. The health check test is part of the standard connect test (there are some extra assertions about the health checks).

Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

💥

@kschoche kschoche merged commit b738aff into feature-tproxy Apr 13, 2021
@kschoche kschoche deleted the tproxy-remove-cleanup-hc-controller-flags branch April 13, 2021 13:54
thisisnotashwin pushed a commit that referenced this pull request Apr 15, 2021
* remove healthcheck and cleanup controller flags
ishustava pushed a commit that referenced this pull request Apr 16, 2021
* remove healthcheck and cleanup controller flags
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.

4 participants