From b32e73a2fad8e433a8fa7c03eba8aa418dabe9f4 Mon Sep 17 00:00:00 2001
From: k8s-infra-cherrypick-robot
 <90416843+k8s-infra-cherrypick-robot@users.noreply.github.com>
Date: Sun, 15 Oct 2023 19:28:12 -0700
Subject: [PATCH] Make sure we don't mask LB tests failures and fix what was
 failing (#2360) (#2430)

* Fix shared LBs tests

PR #2190 prohibited sharing an LB that is internal for security reasons.
This commit fixes the shared LBs tests to not create internal LBs.

* Make sure we don't mask LB tests failures

In `test-lb-service.sh` we do `trap "delete_resources" EXIT` to make
sure we cleanup resources on a test failure. In there, we only fetched
the `$?` after making a check for `${AUTO_CLEAN_UP}`, which itself
alters the code to 0, so function always returns success. This means
tests can never really fail.

This commit fixes it by making sure `$ERROR_CODE` is fetched at the very
beginning of the cleanup function.

Co-authored-by: Michal Dulko <mdulko@redhat.com>
---
 tests/e2e/cloudprovider/test-lb-service.sh | 30 ++++++++++++++++------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/tests/e2e/cloudprovider/test-lb-service.sh b/tests/e2e/cloudprovider/test-lb-service.sh
index c168aa2d1b..6199709284 100755
--- a/tests/e2e/cloudprovider/test-lb-service.sh
+++ b/tests/e2e/cloudprovider/test-lb-service.sh
@@ -20,12 +20,12 @@ LB_SUBNET_NAME=${LB_SUBNET_NAME:-"private-subnet"}
 AUTO_CLEAN_UP=${AUTO_CLEAN_UP:-"true"}
 
 function delete_resources() {
+  ERROR_CODE="$?"
+
   if [[ ${AUTO_CLEAN_UP} != "true" ]]; then
     exit ${ERROR_CODE}
   fi
 
-  ERROR_CODE="$?"
-
   printf "\n>>>>>>> Deleting k8s services\n"
   kubectl -n ${NAMESPACE} get svc -o name | xargs -r kubectl -n $NAMESPACE delete
   printf "\n>>>>>>> Deleting k8s deployments\n"
@@ -34,6 +34,12 @@ function delete_resources() {
   printf "\n>>>>>>> Deleting openstack load balancer \n"
   openstack loadbalancer delete test_shared_user_lb --cascade
 
+  printf "\n>>>>>>> Deleting openstack FIPs \n"
+  fips=$(openstack floating ip list --tag occm-test -f value -c ID)
+  for fip in $fips; do
+      openstack floating ip delete ${fip}
+  done
+
   if [[ "$ERROR_CODE" != "0" ]]; then
     printf "\n>>>>>>> Dump openstack-cloud-controller-manager logs \n"
     pod_name=$(kubectl -n kube-system get pod -l k8s-app=openstack-cloud-controller-manager -o name | awk 'NR==1 {print}')
@@ -438,7 +444,6 @@ metadata:
   name: ${service1}
   namespace: $NAMESPACE
   annotations:
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
 spec:
   type: LoadBalancer
@@ -474,7 +479,6 @@ metadata:
   name: ${service2}
   namespace: $NAMESPACE
   annotations:
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
     loadbalancer.openstack.org/load-balancer-id: "$lbID"
 spec:
@@ -521,7 +525,6 @@ metadata:
   name: ${service2}
   namespace: $NAMESPACE
   annotations:
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
     loadbalancer.openstack.org/load-balancer-id: "$lbID"
 spec:
@@ -580,7 +583,6 @@ metadata:
   name: ${service3}
   namespace: $NAMESPACE
   annotations:
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
     loadbalancer.openstack.org/load-balancer-id: "$lbID"
 spec:
@@ -614,7 +616,6 @@ metadata:
   name: ${service4}
   namespace: $NAMESPACE
   annotations:
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
     loadbalancer.openstack.org/load-balancer-id: "$lbID"
 spec:
@@ -725,6 +726,20 @@ function test_shared_user_lb {
     printf "\n>>>>>>> Waiting for openstack load balancer $lbID ACTIVE after creating listener \n"
     wait_for_loadbalancer $lbID
 
+    printf "\n>>>>>>> Getting an external network \n"
+    extNetID=$(openstack network list --external -f value -c ID | head -1)
+    if [[ -z extNetID ]]; then
+        printf "\n>>>>>>> FAIL: failed to find an external network\n"
+        exit 1
+    fi
+    fip=$(openstack floating ip create --tag occm-test -f value -c id ${extNetID})
+    if [ $? -ne 0 ]; then
+        printf "\n>>>>>>> FAIL: failed to create FIP\n"
+        exit 1
+    fi
+    vip=$(openstack loadbalancer show $lbID -f value -c vip_port_id)
+    openstack floating ip set --port ${vip} ${fip}
+
     local service1="test-shared-user-lb"
     printf "\n>>>>>>> Create Service ${service1}\n"
     cat <<EOF | kubectl apply -f -
@@ -735,7 +750,6 @@ metadata:
   namespace: $NAMESPACE
   annotations:
     loadbalancer.openstack.org/load-balancer-id: "$lbID"
-    service.beta.kubernetes.io/openstack-internal-load-balancer: "true"
     loadbalancer.openstack.org/enable-health-monitor: "false"
 spec:
   type: LoadBalancer