Add ingress-controller test #107
Conversation
|
/test test |
|
/cc @dulek |
|
/retest |
|
/cc @MaysaMacedo |
|
/test test-kuryr |
| @@ -553,6 +613,7 @@ var _ = g.Describe("[sig-installer][Suite:openshift/openstack][lb][Serial] The O | |||
| o.Expect(successConnCount >= minSuccessConn).To(o.BeTrue(), "Found less successful connections (%d) than the minimum expected of '%d'", successConnCount, minSuccessConn) | |||
| e2e.Logf("Found expected number of successfull connections: '%d'", connNumber) | |||
| }) | |||
|
|
|||
| o.Expect(err).NotTo(o.HaveOccurred(), "router-%q not found in openshift-ingress namespace", name) | ||
| o.Expect(svc.Spec.Type).Should(o.Equal(v1.ServiceTypeLoadBalancer), "Unexpected svc Type: %q", svc.Spec.Type) | ||
| loadBalancerId := svc.GetAnnotations()["loadbalancer.openstack.org/load-balancer-id"] | ||
| o.Expect(loadBalancerId).ShouldNot(o.BeEmpty(), "load-balancer-id annotation missing") | ||
| o.Expect(svc.Status.LoadBalancer.Ingress).ShouldNot(o.BeEmpty(), "svc.Status.LoadBalancer.Ingress should not be empty") | ||
| svcIp := svc.Status.LoadBalancer.Ingress[0].IP | ||
| o.Expect(svcIp).ShouldNot(o.BeEmpty(), "FIP missing on svc Status") | ||
| o.Expect(svc.Spec.ExternalTrafficPolicy).Should(o.Equal(v1.ServiceExternalTrafficPolicyTypeLocal), | ||
| "Unexpected ExternalTrafficPolicy on svc specs") |
There was a problem hiding this comment.
Feels like we're testing what should be covered already in origin here, but I don't see it as an important issue.
There was a problem hiding this comment.
This test includes also openstack checks. Origin is only openshift.
| g.By("Test that the canary service is accessible through new ingressController") | ||
| route, err := oc.AdminRouteClient().RouteV1().Routes("openshift-ingress-canary").Get(context.Background(), "canary", metav1.GetOptions{}) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "canary route not found") | ||
| for i := 0; i < 100; i++ { |
There was a problem hiding this comment.
I wonder if checking 100 times won't increase flakiness of the tests. It's just a question here, maybe it's just fine?
There was a problem hiding this comment.
As far as I check in D/S setups and the U/S gate, it is stable. My intention here is to confirm that the traffic is correctly routed to the canary app so there is no loss of queries (as we were observing with this ETP:Local thing).
There was a problem hiding this comment.
Okay, that indeed makes sense.
|
Looks good, please fix the conflict, answer my questions inline and let's proceed with this. |
openshift-merge-robot
added
the
needs-rebase
| route, err := oc.AdminRouteClient().RouteV1().Routes("openshift-ingress-canary").Get(context.Background(), "canary", metav1.GetOptions{}) | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "canary route not found") | ||
| for i := 0; i < 100; i++ { | ||
| resp, err := httpsGetWithCustomLookup(route.Spec.Host, svcIp) |
There was a problem hiding this comment.
is only testing request for port 443 enough?
I don't have a strong opinion here, I just started thinking about this when read your comment on line 453.
There was a problem hiding this comment.
I found issues testing the connectivty to port 80 because the app is redirecting to 443 and then I hit cert issues.
We are covering multiple pools in the test "[sig-installer][Suite:openshift/openstack][lb][Serial] The Openstack platform should re-use an existing UDP Amphora LoadBalancer when new svc is created on Openshift with the proper annotation"
openshift-merge-robot
removed
the
needs-rebase
https://issues.redhat.com/browse/OSASINFRA-2412 Tech debt in openstack-test: This test creates an ingress-controller matching the canary route label. It checks that the openstack loadbalancer is created with the expected attributes and access 100 times the service from outside to confirm that the functionality is provided using the expected new path.
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dulek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@rlobillo: all tests passed! Full PR test history. Your PR dashboard. 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. |
https://issues.redhat.com/browse/OSASINFRA-2412
Tech debt in openstack-test: This test creates an ingress-controller matching the canary route label. It checks that the openstack loadbalancer is created with the expected attributes and access 100 times the service from outside to confirm that the functionality is provided using the expected new path (The openstack loadbalancer).