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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Oct 1, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion test/e2e/framework/service/jig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ func (j *TestJig) CheckServiceReachability(namespace string, svc *v1.Service, po
}
}

// CreateServicePods creates a replication controller with the label same as service
// CreateServicePods creates a replication controller with the label same as service. Service listens to HTTP.
func (j *TestJig) CreateServicePods(c clientset.Interface, ns string, replica int) {
config := testutils.RCConfig{
Client: c,
Expand All @@ -875,3 +875,20 @@ func (j *TestJig) CreateServicePods(c clientset.Interface, ns string, replica in
err := framework.RunRC(config)
framework.ExpectNoError(err, "Replica must be created")
}

// CreateTCPUDPServicePods creates a replication controller with the label same as service. Service listens to TCP and UDP.
func (j *TestJig) CreateTCPUDPServicePods(c clientset.Interface, ns string, replica int) {
config := testutils.RCConfig{
Client: c,
Name: j.Name,
Image: framework.ServeHostnameImage,
Command: []string{"/agnhost", "serve-hostname", "--http=false", "--tcp", "--udp"},
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

Namespace: ns,
Labels: j.Labels,
PollInterval: 3 * time.Second,
Timeout: framework.PodReadyBeforeTimeout,
Replicas: replica,
}
err := framework.RunRC(config)
framework.ExpectNoError(err, "Replica must be created")
}
116 changes: 39 additions & 77 deletions test/e2e/network/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,11 +932,12 @@ var _ = SIGDescribe("Services", func() {

/*
Testname: Service, update NodePort, same port different protocol
Description: Create a service of type ClusterIP to accept TCP requests. Service creation MUST be successful by assigning ClusterIP to the service.
When service type is updated to NodePort to support two protocols i.e. TCP and UDP for same assigned service port 80, service update MUST be successful by allocating two NodePorts to the service.
TODO: Test Service reachability, good to include this check in Conformance perspective.
Description: Create a service to accept TCP requests. By default, created service MUST be of type ClusterIP and an ClusterIP MUST be assigned to the service.
When service type is updated to NodePort supporting TCP protocol, it MUST be reachable on nodeIP over allocated NodePort to serve TCP requests.
When this NodePort service is updated to use two protocols i.e. TCP and UDP for same assigned service port 80, service update MUST be successful by allocating two NodePorts to the service and
service MUST be able to serve both TCP and UDP requests over same service port 80.
*/
ginkgo.It("should be able to update NodePorts with two same port numbers but different protocols", func() {
ginkgo.It("should be able to update service type to NodePort listening on same port number but different protocols", func() {
serviceName := "nodeport-update-service"
ns := f.Namespace.Name
jig := e2eservice.NewTestJig(cs, serviceName)
Expand All @@ -949,36 +950,50 @@ var _ = SIGDescribe("Services", func() {
framework.ExpectNoError(err, "failed to delete service: %s in namespace: %s", serviceName, ns)
}()
jig.SanityCheckService(tcpService, v1.ServiceTypeClusterIP)
svcPort := int(tcpService.Spec.Ports[0].Port)
framework.Logf("service port TCP: %d", svcPort)
framework.Logf("Service Port TCP: %v", tcpService.Spec.Ports[0].Port)

ginkgo.By("changing the TCP service to type=NodePort")
nodePortService := jig.UpdateServiceOrFail(ns, tcpService.Name, func(s *v1.Service) {
s.Spec.Type = v1.ServiceTypeNodePort
s.Spec.Ports = []v1.ServicePort{
{
Name: "tcp-port",
Port: 80,
Protocol: v1.ProtocolTCP,
TargetPort: intstr.FromInt(9376),
},
}
})

jig.CreateTCPUDPServicePods(cs, ns, 2)
execPod := e2epod.CreateExecPodOrFail(cs, ns, "execpod", nil)
jig.CheckServiceReachability(ns, nodePortService, execPod)

// Change the services to NodePort and add a UDP port.
ginkgo.By("changing the TCP service to type=NodePort and add a UDP port")
newService := jig.UpdateServiceOrFail(ns, tcpService.Name, func(s *v1.Service) {
ginkgo.By("Updating NodePort service to listen TCP and UDP based requests over same Port")
nodePortService = jig.UpdateServiceOrFail(ns, tcpService.Name, func(s *v1.Service) {
s.Spec.Type = v1.ServiceTypeNodePort
s.Spec.Ports = []v1.ServicePort{
{
Name: "tcp-port",
Port: 80,
Protocol: v1.ProtocolTCP,
Name: "tcp-port",
Port: 80,
Protocol: v1.ProtocolTCP,
TargetPort: intstr.FromInt(9376),
},
{
Name: "udp-port",
Port: 80,
Protocol: v1.ProtocolUDP,
Name: "udp-port",
Port: 80,
Protocol: v1.ProtocolUDP,
TargetPort: intstr.FromInt(9376),
},
}
})
jig.SanityCheckService(newService, v1.ServiceTypeNodePort)
if len(newService.Spec.Ports) != 2 {
framework.Failf("new service should have two Ports")
}
for _, port := range newService.Spec.Ports {
if port.NodePort == 0 {
framework.Failf("new service failed to allocate NodePort for Port %s", port.Name)
}
jig.CheckServiceReachability(ns, nodePortService, execPod)
nodePortCounts := len(nodePortService.Spec.Ports)
framework.ExpectEqual(nodePortCounts, 2, "updated service should have two Ports but found %d Ports", nodePortCounts)

framework.Logf("new service allocates NodePort %d for Port %s", port.NodePort, port.Name)
for _, port := range nodePortService.Spec.Ports {
framework.ExpectNotEqual(port.NodePort, 0, "NodePort service failed to allocate NodePort for Port %s", port.Name)
framework.Logf("NodePort service allocates NodePort: %d for Port: %s over Protocol: %s", port.NodePort, port.Name, port.Protocol)
}
})

Expand Down Expand Up @@ -1133,59 +1148,6 @@ var _ = SIGDescribe("Services", func() {

})

/*
Testname: Service, NodePort, same port different protocols
Description: Create a service of type NodePort listening on port 53 for two protocols TCP and UDP.
Service creation MUST be successful by assigning a ClusterIP and two unique nodePorts for each protocol, making service reachable on every node's IP and nodePort.
TODO: Test Service reachability, good to include this check in Conformance perspective.
*/
ginkgo.It("should use same NodePort with same port but different protocols", func() {
serviceName := "nodeports"
ns := f.Namespace.Name

t := e2eservice.NewServerTest(cs, ns, serviceName)
defer func() {
defer ginkgo.GinkgoRecover()
errs := t.Cleanup()
if len(errs) != 0 {
framework.Failf("errors in cleanup: %v", errs)
}
}()

ginkgo.By("creating service " + serviceName + " with same NodePort but different protocols in namespace " + ns)
service := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: t.ServiceName,
Namespace: t.Namespace,
},
Spec: v1.ServiceSpec{
Selector: t.Labels,
Type: v1.ServiceTypeNodePort,
Ports: []v1.ServicePort{
{
Name: "tcp-port",
Port: 53,
Protocol: v1.ProtocolTCP,
},
{
Name: "udp-port",
Port: 53,
Protocol: v1.ProtocolUDP,
},
},
},
}
result, err := t.CreateService(service)
framework.ExpectNoError(err, "failed to create service: %s in namespace: %s", serviceName, ns)

if len(result.Spec.Ports) != 2 {
framework.Failf("got unexpected len(Spec.Ports) for new service: %v", result)
}
if result.Spec.Ports[0].NodePort != result.Spec.Ports[1].NodePort {
framework.Failf("should use same NodePort for new service: %v", result)
}
})

ginkgo.It("should prevent NodePort collisions", func() {
// TODO: use the ServiceTestJig here
baseName := "nodeport-collision-"
Expand Down