Update e2e testing nodePort service listening on same port but different protocols #81419

mgdevstack · 2019-08-14T14:02:30Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

/kind feature

/kind flake

What this PR does / why we need it:

Updates should be able to update NodePorts with two same port numbers but different protocols to test service reachability and improved e2e Name and behavior.
Removes should use same NodePort with same port but different protocols as this behaviour would be tested by above updated e2e.

Which issue(s) this PR fixes:

Fixes #81418

Special notes for your reviewer:
Changes are made to make e2e eligible for Conformance.
Source of Issue : #77865 (comment)

Does this PR introduce a user-facing change?:

None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/area conformance
/sig testing
@kubernetes/cncf-conformance-wg
@kubernetes/sig-node-pr-reviews
@kubernetes/sig-architecture-pr-reviews

mgdevstack · 2019-08-14T14:04:23Z

Once merged, we can promote this e2e to Conformance in this release cycle v1.16 which is very near.
/priority important-soon

mattjmcnaughton

/lgtm

Probably a good idea to have someone more involved with the conformance work review this as well, but it looks good to me :)

mgdevstack · 2019-08-15T19:38:43Z

To gain attention for reviewing :)
/cc @johnbelamaric @spiffxp

mattjmcnaughton · 2019-08-16T14:21:54Z

/assign @ixdy

johnbelamaric · 2019-08-16T23:29:05Z

/approve

oomichi

/lgtm

oomichi · 2019-08-16T23:40:10Z

test/e2e/network/service.go

+		jig.CheckServiceReachability(ns, nodePortService, execPod)
+
+		if nodePortCounts := len(nodePortService.Spec.Ports); nodePortCounts != 2 {
+			e2elog.Failf("new service should have two Ports but found %d Ports", nodePortCounts)


nit: Here can be framework.ExpectEqual(len(nodePortService.Spec.Ports), 2)

oomichi · 2019-08-16T23:47:58Z

test/e2e/framework/service/jig.go

+		Client:       c,
+		Name:         j.Name,
+		Image:        framework.ServeHostnameImage,
+		Command:      []string{"/agnhost", "serve-hostname", "--http=false", "--tcp", "--udp"},


nit: I am wondering we can merge this new function into the above existing function because the difference is just this command here. But at this time, it is fine and we can take care later.

Thanks for the suggestion, I was in the same mindset to have single function and leverage flags as args but to keep PR simple, I have not introduced here. As you mentioned, I can create another PR to refactor it.

Following is raw code.

var ServeHostNameImageDefaultFlags = map[string]string{ HTTP: "--http", TCP: "--tcp", UDP: "--udp", } func enableServeHostNameImageCmdFlags(enableFlags ...string) (enabledFlags []string){ gomega.Expect(len(enableFlags)).NotTo(gomega.BeNumerically(">", 2), "HTTP only or TCP/UDP requests can be served but not both simultaneously") var flags = make(map[string]string,len(ServeHostNameImageDefaultFlags)) for _,f := range enableFlags{ switch f{ case TCP, UDP: framework.ExpectNotEqual(flags[f],ServeHostNameImageDefaultFlags[HTTP], "HTTP flag can not be enabled along with TCP/UDP") flags[f] = "--http=false" flags[f] = ServeHostNameImageDefaultFlags[f] case HTTP: if flags[TCP] != "" || flags[UDP] != ""{ framework.Failf("TCP/UDP flag is enabled so HTTP flag can not be enabled") } flags[f] = ServeHostNameImageDefaultFlags[f] default: framework.Failf("Unexpected protocol flags are passed. Valid protocol flags are HTTP, TCP and UDP") } } for _, f := range flags{ enabledFlags = append(enableFlags, f) } return } flags := enableServeHostNameImageCmdFlags(TCP, UDP)

@mgdevstack could you please create a follow up issue for this, so we dont forget?
once we get that we can cancel the hold on this.

mgdevstack · 2019-08-22T14:31:35Z

test/e2e/network/service.go

 		}
-		for _, port := range newService.Spec.Ports {
+		for _, port := range nodePortService.Spec.Ports {
 			if port.NodePort == 0 {


nit: framework.ExpectNotEqual(port.NodePort, 0, errorMessage...)

fejta-bot · 2019-09-05T03:55:38Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

adelina-t · 2019-09-05T07:43:49Z

That seems unrelated test failure.

/test pull-kubernetes-e2e-aks-engine-azure-windows

Yes, it is unrelated. Because of an image change for redis, that is unsupported by windows, the guestbook application cannot run, thus the test will fail. It was supposed to be removed from windows e2e presubmit, but seems it was an oversight that I shall fix ASAP.

I ran the aks-engine-azure-windows job on this PR because I wanted to see if the changes don't impact Windows.

fejta-bot · 2019-09-05T09:52:42Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-05T14:04:38Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-05T18:58:07Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-05T23:52:06Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-06T04:46:05Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

mgdevstack · 2019-09-06T06:09:41Z

E2E non-related to this PR are failing on Azure job which requires fix mentioned in #81419 (comment) and milestone is not added for this release i.e. 1.16 so holding this PR to stop repeated retest.
/hold

k8s-ci-robot · 2019-09-06T07:20:33Z

@mgdevstack: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-aks-engine-azure-windows	e8ce310eb881003401f2bf08bfef9da56243a5a9	link	`/test pull-kubernetes-e2e-aks-engine-azure-windows`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

johnbelamaric · 2019-09-26T22:34:49Z

@mgdevstack please rebase and resolve conflicts

…ent protocols

alejandrox1

/lgtm
/milestone v1.17

alejandrox1 · 2019-09-29T19:25:58Z

test/e2e/framework/service/jig.go

+		Client:       c,
+		Name:         j.Name,
+		Image:        framework.ServeHostnameImage,
+		Command:      []string{"/agnhost", "serve-hostname", "--http=false", "--tcp", "--udp"},


@mgdevstack could you please create a follow up issue for this, so we dont forget?
once we get that we can cancel the hold on this.

alejandrox1 · 2019-09-29T19:27:27Z

/milestone v1.17

timothysc · 2019-09-30T17:13:22Z

/assign @johnbelamaric

mgdevstack · 2019-10-01T07:01:16Z

/hold cancel
As per #81419 (comment) tracking issue under #83347

k8s-ci-robot requested review from cmluciano and timothysc August 14, 2019 14:03

k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 14, 2019

mgdevstack changed the title ~~Update e2e testing nodePort service listening on same port but differ…~~ Update e2e testing nodePort service listening on same port but different protocols Aug 14, 2019

mattjmcnaughton approved these changes Aug 15, 2019

View reviewed changes

k8s-ci-robot assigned mattjmcnaughton Aug 15, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2019

k8s-ci-robot assigned ixdy Aug 16, 2019

oomichi reviewed Aug 16, 2019

View reviewed changes

k8s-ci-robot assigned oomichi Aug 16, 2019

spiffxp added this to To Triage in conformance-definition Aug 21, 2019

spiffxp moved this from To Triage to Needs Review in conformance-definition Aug 21, 2019

mgdevstack commented Aug 22, 2019

View reviewed changes

mgdevstack force-pushed the checkServiceReachability branch from beb02d1 to b4e407c Compare August 22, 2019 17:05

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 6, 2019

timothysc removed their request for review September 10, 2019 20:52

smarterclayton moved this from Needs Approval to Needs Review in conformance-definition Sep 12, 2019

johnbelamaric moved this from Needs Review to In Progress in conformance-definition Sep 26, 2019

Update e2e testing nodePort service listening on same port but differ…

99475f9

…ent protocols

mgdevstack force-pushed the checkServiceReachability branch from e8ce310 to 99475f9 Compare September 28, 2019 01:19

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 28, 2019

alejandrox1 reviewed Sep 29, 2019

View reviewed changes

k8s-ci-robot assigned alejandrox1 Sep 29, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 29, 2019

k8s-ci-robot added this to the v1.17 milestone Sep 29, 2019

timothysc removed the area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework label Sep 30, 2019

k8s-ci-robot assigned johnbelamaric Sep 30, 2019

mgdevstack mentioned this pull request Oct 1, 2019

Refactor service jig function to accept different flags for framework.ServeHostnameImage #83347

Closed

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 1, 2019

k8s-ci-robot merged commit 78940cd into kubernetes:master Oct 1, 2019

conformance-definition automation moved this from In Progress to Done Oct 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update e2e testing nodePort service listening on same port but different protocols #81419

Update e2e testing nodePort service listening on same port but different protocols #81419

mgdevstack commented Aug 14, 2019 •

edited

mgdevstack commented Aug 14, 2019

mattjmcnaughton left a comment

mgdevstack commented Aug 15, 2019 •

edited

mattjmcnaughton commented Aug 16, 2019

johnbelamaric commented Aug 16, 2019

oomichi left a comment

oomichi Aug 16, 2019

mgdevstack Aug 22, 2019

oomichi Aug 16, 2019

mgdevstack Aug 22, 2019

alejandrox1 Sep 29, 2019

mgdevstack Aug 22, 2019

mgdevstack Aug 22, 2019

fejta-bot commented Sep 5, 2019

adelina-t commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 6, 2019

mgdevstack commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

johnbelamaric commented Sep 26, 2019

alejandrox1 left a comment •

edited

alejandrox1 Sep 29, 2019

alejandrox1 commented Sep 29, 2019

timothysc commented Sep 30, 2019

mgdevstack commented Oct 1, 2019

Update e2e testing nodePort service listening on same port but different protocols #81419

Update e2e testing nodePort service listening on same port but different protocols #81419

Conversation

mgdevstack commented Aug 14, 2019 • edited

mgdevstack commented Aug 14, 2019

mattjmcnaughton left a comment

Choose a reason for hiding this comment

mgdevstack commented Aug 15, 2019 • edited

mattjmcnaughton commented Aug 16, 2019

johnbelamaric commented Aug 16, 2019

oomichi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fejta-bot commented Sep 5, 2019

adelina-t commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 5, 2019

fejta-bot commented Sep 6, 2019

mgdevstack commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

johnbelamaric commented Sep 26, 2019

alejandrox1 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alejandrox1 commented Sep 29, 2019

timothysc commented Sep 30, 2019

mgdevstack commented Oct 1, 2019

mgdevstack commented Aug 14, 2019 •

edited

mgdevstack commented Aug 15, 2019 •

edited

alejandrox1 left a comment •

edited