Cherrypick PR 1080 into release-1.9

pkg/loadbalancers/addresses.go

@@ -35,7 +35,11 @@ func (l *L7) checkStaticIP() (err error) {
 	}
 	managedStaticIPName := l.namer.ForwardingRule(namer.HTTPProtocol)
 	// Don't manage staticIPs if the user has specified an IP.
-	if address, manageStaticIP := l.getEffectiveIP(); !manageStaticIP {
+	address, manageStaticIP, err := l.getEffectiveIP()
+	if err != nil {
+		return err
+	}
+	if !manageStaticIP {
 		klog.V(3).Infof("Not managing user specified static IP %v", address)
 		if flags.F.EnableDeleteUnusedFrontends {
 			// Delete ingress controller managed static ip if exists.

pkg/loadbalancers/forwarding_rules.go

@@ -21,7 +21,7 @@ import (
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/ingress-gce/pkg/annotations"
 	"k8s.io/ingress-gce/pkg/composite"
@@ -42,7 +42,10 @@ func (l *L7) checkHttpForwardingRule() (err error) {
 		return fmt.Errorf("cannot create forwarding rule without proxy")
 	}
 	name := l.namer.ForwardingRule(namer.HTTPProtocol)
-	address, _ := l.getEffectiveIP()
+	address, _, err := l.getEffectiveIP()
+	if err != nil {
+		return err
+	}
 	fw, err := l.checkForwardingRule(name, l.tp.SelfLink, address, httpDefaultPortRange)
 	if err != nil {
 		return err
@@ -57,7 +60,10 @@ func (l *L7) checkHttpsForwardingRule() (err error) {
 		return nil
 	}
 	name := l.namer.ForwardingRule(namer.HTTPSProtocol)
-	address, _ := l.getEffectiveIP()
+	address, _, err := l.getEffectiveIP()
+	if err != nil {
+		return err
+	}
 	fws, err := l.checkForwardingRule(name, l.tps.SelfLink, address, httpsDefaultPortRange)
 	if err != nil {
 		return err
@@ -150,15 +156,14 @@ func (l *L7) checkForwardingRule(name, proxyLink, ip, portRange string) (fw *com
 }
 
 // getEffectiveIP returns a string with the IP to use in the HTTP and HTTPS
-// forwarding rules, and a boolean indicating if this is an IP the controller
-// should manage or not.
-func (l *L7) getEffectiveIP() (string, bool) {
+// forwarding rules, a boolean indicating if this is an IP the controller
+// should manage or not and an error if the specified IP was not found.
+func (l *L7) getEffectiveIP() (string, bool, error) {
 
 	// A note on IP management:
 	// User specifies a different IP on startup:
 	//	- We create a forwarding rule with the given IP.
-	//		- If this ip doesn't exist in GCE, we create another one in the hope
-	//		  that they will rectify it later on.
+	//		- If this ip doesn't exist in GCE, an error is thrown which fails ingress creation.
 	//	- In the happy case, no static ip is created or deleted by this controller.
 	// Controller allocates a staticIP/ephemeralIP, but user changes it:
 	//  - We still delete the old static IP, but only when we tear down the
@@ -175,16 +180,16 @@ func (l *L7) getEffectiveIP() (string, bool) {
 		// Existing static IPs allocated to forwarding rules will get orphaned
 		// till the Ingress is torn down.
 		if ip, err := l.cloud.GetGlobalAddress(l.runtimeInfo.StaticIPName); err != nil || ip == nil {
-			klog.Warningf("The given static IP name %v doesn't translate to an existing global static IP, ignoring it and allocating a new IP: %v",
-				l.runtimeInfo.StaticIPName, err)
+			return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing global static IP.",
+				l.runtimeInfo.StaticIPName)
 		} else {
-			return ip.Address, false
+			return ip.Address, false, nil
 		}
 	}
 	if l.ip != nil {
-		return l.ip.Address, true
+		return l.ip.Address, true, nil
 	}
-	return "", true
+	return "", true, nil
 }
 
 // ensureForwardingRule creates a forwarding rule with the given name, if it does not exist. It updates the existing

pkg/loadbalancers/loadbalancers_test.go

@@ -987,6 +987,38 @@ func TestCreateBothLoadBalancers(t *testing.T) {
 	}
 }
 
+// Test StaticIP annotation behavior.
+// When a non-existent StaticIP value is specified, ingress creation must fail.
+func TestStaticIP(t *testing.T) {
+	j := newTestJig(t)
+	gceUrlMap := utils.NewGCEURLMap()
+	gceUrlMap.DefaultBackend = &utils.ServicePort{NodePort: 31234, BackendNamer: j.namer}
+	gceUrlMap.PutPathRulesForHost("bar.example.com", []utils.PathRule{{Path: "/bar", Backend: utils.ServicePort{NodePort: 30000, BackendNamer: j.namer}}})
+	ing := newIngress()
+	ing.Annotations = map[string]string{
+		"StaticIPNameKey": "teststaticip",
+	}
+	lbInfo := &L7RuntimeInfo{
+		AllowHTTP:    true,
+		TLS:          []*TLSCerts{{Key: "key", Cert: "cert"}},
+		UrlMap:       gceUrlMap,
+		Ingress:      ing,
+		StaticIPName: "teststaticip",
+	}
+
+	if _, err := j.pool.Ensure(lbInfo); err == nil {
+		t.Fatalf("expected error ensuring ingress with non-existent static ip")
+	}
+	// Create static IP
+	err := j.fakeGCE.ReserveGlobalAddress(&compute.Address{Name: "teststaticip", Address: "1.2.3.4"})
+	if err != nil {
+		t.Fatalf("ip address reservation failed - %v", err)
+	}
+	if _, err := j.pool.Ensure(lbInfo); err != nil {
+		t.Fatalf("unexpected error %v", err)
+	}
+}
+
 // Test setting frontendconfig Ssl policy
 func TestFrontendConfigSslPolicy(t *testing.T) {
 	flags.F.EnableFrontendConfig = true