Skip to content

Conversation

@pleshakov
Copy link
Contributor

Proposed changes

Support match in TransportServer health checks
Based on the original work from @soneillf5

@github-actions github-actions bot added documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements labels May 26, 2021
Copy link
Contributor

@soneillf5 soneillf5 left a comment

Choose a reason for hiding this comment

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

Can you add a test to the python test suite that confirms the functionality of this change?

@pleshakov pleshakov requested a review from soneillf5 May 27, 2021 19:50
@soneillf5 soneillf5 self-requested a review May 31, 2021 08:25
Copy link
Contributor

@soneillf5 soneillf5 left a comment

Choose a reason for hiding this comment

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

I need to leave a comment to request changes.

@pleshakov pleshakov requested a review from vepatel June 1, 2021 04:36
assert len(endpoints) is 3

@pytest.mark.skip_for_nginx_oss
def test_tcp_passing_healthcheck_with_match(
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that's fine for now that its added and working but we don't need to verify all the attribs as nginx plus is doing lb and the UI is provided by the same.


func TestGenerateTransportServerHealthChecks(t *testing.T) {
upstreamName := "dns-tcp"
generatedUpsteamName := "ts_namespace_name_dns-tcp"
Copy link
Contributor

Choose a reason for hiding this comment

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

is there one for dns-udp too?

@pleshakov pleshakov force-pushed the feature/ts-hc-match branch from 699597d to 123003b Compare June 2, 2021 17:49
@pleshakov pleshakov merged commit 89ba98e into master Jun 2, 2021
@pleshakov pleshakov deleted the feature/ts-hc-match branch June 2, 2021 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants