Merge pull request #58359 from MrHohn/e2e-ingress-modified-healthcheck

test/e2e/framework/firewall_util.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
@@ -371,19 +370,3 @@ func WaitForFirewallRule(gceCloud *gcecloud.GCECloud, fwName string, exist bool,
 	}
 	return fw, nil
 }
-
-func GetClusterID(c clientset.Interface) (string, error) {
-	cm, err := c.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(gcecloud.UIDConfigMapName, metav1.GetOptions{})
-	if err != nil || cm == nil {
-		return "", fmt.Errorf("error getting cluster ID: %v", err)
-	}
-	clusterID, clusterIDExists := cm.Data[gcecloud.UIDCluster]
-	providerID, providerIDExists := cm.Data[gcecloud.UIDProvider]
-	if !clusterIDExists {
-		return "", fmt.Errorf("cluster ID not set")
-	}
-	if providerIDExists {
-		return providerID, nil
-	}
-	return clusterID, nil
-}

test/e2e/framework/ingress_utils.go

@@ -1109,6 +1109,42 @@ func (j *IngressTestJig) TryDeleteIngress() {
 	}
 }
 
+// getIngressAddress returns the ips/hostnames associated with the Ingress.
+func getIngressAddress(client clientset.Interface, ns, name string) ([]string, error) {
+	ing, err := client.ExtensionsV1beta1().Ingresses(ns).Get(name, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+	addresses := []string{}
+	for _, a := range ing.Status.LoadBalancer.Ingress {
+		if a.IP != "" {
+			addresses = append(addresses, a.IP)
+		}
+		if a.Hostname != "" {
+			addresses = append(addresses, a.Hostname)
+		}
+	}
+	return addresses, nil
+}
+
+// WaitForIngressAddress waits for the Ingress to acquire an address.
+func WaitForIngressAddress(c clientset.Interface, ns, ingName string, timeout time.Duration) (string, error) {
+	var address string
+	err := wait.PollImmediate(10*time.Second, timeout, func() (bool, error) {
+		ipOrNameList, err := getIngressAddress(c, ns, ingName)
+		if err != nil || len(ipOrNameList) == 0 {
+			Logf("Waiting for Ingress %v to acquire IP, error %v", ingName, err)
+			if IsRetryableAPIError(err) {
+				return false, nil
+			}
+			return false, err
+		}
+		address = ipOrNameList[0]
+		return true, nil
+	})
+	return address, err
+}
+
 // WaitForIngress waits till the ingress acquires an IP, then waits for its
 // hosts/urls to respond to a protocol check (either http or https). If
 // waitForNodePort is true, the NodePort of the Service is verified before
@@ -1168,6 +1204,14 @@ func (j *IngressTestJig) pollServiceNodePort(ns, name string, port int) {
 	ExpectNoError(PollURL(u, "", 30*time.Second, j.PollInterval, &http.Client{Timeout: IngressReqTimeout}, false))
 }
 
+func (j *IngressTestJig) GetDefaultBackendNodePort() (int32, error) {
+	defaultSvc, err := j.Client.CoreV1().Services(metav1.NamespaceSystem).Get(defaultBackendName, metav1.GetOptions{})
+	if err != nil {
+		return 0, err
+	}
+	return defaultSvc.Spec.Ports[0].NodePort, nil
+}
+
 // GetIngressNodePorts returns related backend services' nodePorts.
 // Current GCE ingress controller allows traffic to the default HTTP backend
 // by default, so retrieve its nodePort if includeDefaultBackend is true.

test/e2e/framework/util.go

@@ -4140,42 +4140,6 @@ func OpenWebSocketForURL(url *url.URL, config *restclient.Config, protocols []st
 	return websocket.DialConfig(cfg)
 }
 
-// getIngressAddress returns the ips/hostnames associated with the Ingress.
-func getIngressAddress(client clientset.Interface, ns, name string) ([]string, error) {
-	ing, err := client.ExtensionsV1beta1().Ingresses(ns).Get(name, metav1.GetOptions{})
-	if err != nil {
-		return nil, err
-	}
-	addresses := []string{}
-	for _, a := range ing.Status.LoadBalancer.Ingress {
-		if a.IP != "" {
-			addresses = append(addresses, a.IP)
-		}
-		if a.Hostname != "" {
-			addresses = append(addresses, a.Hostname)
-		}
-	}
-	return addresses, nil
-}
-
-// WaitForIngressAddress waits for the Ingress to acquire an address.
-func WaitForIngressAddress(c clientset.Interface, ns, ingName string, timeout time.Duration) (string, error) {
-	var address string
-	err := wait.PollImmediate(10*time.Second, timeout, func() (bool, error) {
-		ipOrNameList, err := getIngressAddress(c, ns, ingName)
-		if err != nil || len(ipOrNameList) == 0 {
-			Logf("Waiting for Ingress %v to acquire IP, error %v", ingName, err)
-			if IsRetryableAPIError(err) {
-				return false, nil
-			}
-			return false, err
-		}
-		address = ipOrNameList[0]
-		return true, nil
-	})
-	return address, err
-}
-
 // Looks for the given string in the log of a specific pod container
 func LookForStringInLog(ns, podName, container, expectedString string, timeout time.Duration) (result string, err error) {
 	return LookForString(expectedString, timeout, func() string {
@@ -4797,6 +4761,22 @@ func (p *E2ETestNodePreparer) CleanupNodes() error {
 	return encounteredError
 }
 
+func GetClusterID(c clientset.Interface) (string, error) {
+	cm, err := c.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(gcecloud.UIDConfigMapName, metav1.GetOptions{})
+	if err != nil || cm == nil {
+		return "", fmt.Errorf("error getting cluster ID: %v", err)
+	}
+	clusterID, clusterIDExists := cm.Data[gcecloud.UIDCluster]
+	providerID, providerIDExists := cm.Data[gcecloud.UIDProvider]
+	if !clusterIDExists {
+		return "", fmt.Errorf("cluster ID not set")
+	}
+	if providerIDExists {
+		return providerID, nil
+	}
+	return clusterID, nil
+}
+
 // CleanupGCEResources cleans up GCE Service Type=LoadBalancer resources with
 // the given name. The name is usually the UUID of the Service prefixed with an
 // alpha-numeric character ('a') to work around cloudprovider rules.

test/e2e/network/ingress.go

@@ -19,6 +19,7 @@ package network
 import (
 	"fmt"
 	"path/filepath"
+	"strings"
 	"time"
 
 	extensions "k8s.io/api/extensions/v1beta1"
@@ -28,10 +29,12 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/authentication/serviceaccount"
+	gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce"
 	"k8s.io/kubernetes/test/e2e/framework"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	compute "google.golang.org/api/compute/v1"
 )
 
 const (
@@ -245,6 +248,75 @@ var _ = SIGDescribe("Loadbalancing: L7", func() {
 			jig.WaitForIngress(false)
 		})
 
+		It("should not reconcile manually modified health check for ingress", func() {
+			By("Creating a basic HTTP ingress and wait for it to come up.")
+			jig.CreateIngress(filepath.Join(framework.IngressManifestPath, "http"), ns, nil, nil)
+			jig.WaitForIngress(true)
+
+			// Get cluster UID.
+			clusterID, err := framework.GetClusterID(f.ClientSet)
+			Expect(err).NotTo(HaveOccurred())
+			// Get default backend nodeport.
+			defaultBackendNodePort, err := jig.GetDefaultBackendNodePort()
+			Expect(err).NotTo(HaveOccurred())
+
+			// Filter health check using cluster UID as the suffix.
+			By("Retrieving relevant health check resources from GCE.")
+			gceCloud := gceController.Cloud.Provider.(*gcecloud.GCECloud)
+			hcs, err := gceCloud.ListHealthChecks()
+			Expect(err).NotTo(HaveOccurred())
+			ingressHCs := []*compute.HealthCheck{}
+			for _, hc := range hcs {
+				if strings.HasSuffix(hc.Name, clusterID) {
+					Expect(hc.HttpHealthCheck).ToNot(Equal(nil))
+					// Skip the default backend healthcheck as that shouldn't be customized.
+					if hc.HttpHealthCheck.Port == int64(defaultBackendNodePort) {
+						continue
+					}
+					ingressHCs = append(ingressHCs, hc)
+				}
+			}
+
+			Expect(len(ingressHCs)).ToNot(Equal(0))
+			hcToChange := ingressHCs[0]
+			By(fmt.Sprintf("Modifying health check %v without involving ingress.", hcToChange.Name))
+			// Change timeout from 60s to 25s.
+			hcToChange.TimeoutSec = 25
+			// Change path from /healthz to /.
+			hcToChange.HttpHealthCheck.RequestPath = "/"
+			err = gceCloud.UpdateHealthCheck(hcToChange)
+			Expect(err).NotTo(HaveOccurred())
+
+			// Add one more path to ingress to trigger resource syncing.
+			By("Adding a new path to ingress and wait for it to take effect.")
+			jig.Update(func(ing *extensions.Ingress) {
+				ing.Spec.Rules = append(ing.Spec.Rules, extensions.IngressRule{
+					Host: "ingress.test.com",
+					IngressRuleValue: extensions.IngressRuleValue{
+						HTTP: &extensions.HTTPIngressRuleValue{
+							Paths: []extensions.HTTPIngressPath{
+								{
+									Path: "/test",
+									// Copy backend from the first rule.
+									Backend: ing.Spec.Rules[0].HTTP.Paths[0].Backend,
+								},
+							},
+						},
+					},
+				})
+			})
+			// Wait for change to take effect before checking the health check resource.
+			jig.WaitForIngress(false)
+
+			// Validate the modified fields on health check are intact.
+			By("Checking if the modified health check is unchanged.")
+			hcAfterSync, err := gceCloud.GetHealthCheck(hcToChange.Name)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(hcAfterSync.HttpHealthCheck).ToNot(Equal(nil))
+			Expect(hcAfterSync.TimeoutSec).To(Equal(hcToChange.TimeoutSec))
+			Expect(hcAfterSync.HttpHealthCheck.RequestPath).To(Equal(hcToChange.HttpHealthCheck.RequestPath))
+		})
+
 		It("multicluster ingress should get instance group annotation", func() {
 			name := "echomap"
 			jig.CreateIngress(filepath.Join(framework.IngressManifestPath, "http"), ns, map[string]string{