Skip to content

Commit

Permalink
Implements regional static IP support for L7-ILB ingress
Browse files Browse the repository at this point in the history
This is done via a regional-static-ip annotation on the ingress
  • Loading branch information
spencerhance committed Jul 13, 2020
1 parent fdcb4bf commit d5344ec
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 34 deletions.
46 changes: 41 additions & 5 deletions pkg/annotations/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package annotations

import (
"errors"
"strconv"

"k8s.io/api/networking/v1beta1"
"k8s.io/ingress-gce/pkg/flags"
)

const (
Expand All @@ -34,12 +36,19 @@ const (
// rules for port 443 based on the TLS section.
AllowHTTPKey = "kubernetes.io/ingress.allow-http"

// StaticIPNameKey tells the Ingress controller to use a specific GCE
// GlobalStaticIPNameKey tells the Ingress controller to use a specific GCE
// static ip for its forwarding rules. If specified, the Ingress controller
// assigns the static ip by this name to the forwarding rules of the given
// Ingress. The controller *does not* manage this ip, it is the users
// responsibility to create/delete it.
StaticIPNameKey = "kubernetes.io/ingress.global-static-ip-name"
GlobalStaticIPNameKey = "kubernetes.io/ingress.global-static-ip-name"

// RegionalStaticIPNameKey tells the Ingress controller to use a specific GCE
// internal static ip for its forwarding rules. If specified, the Ingress controller
// assigns the static ip by this name to the forwarding rules of the given
// Ingress. The controller *does not* manage this ip, it is the users
// responsibility to create/delete it.
RegionalStaticIPNameKey = "kubernetes.io/ingress.regional-static-ip-name"

// PreSharedCertKey represents the specific pre-shared SSL
// certificate for the Ingress controller to use. The controller *does not*
Expand All @@ -50,7 +59,7 @@ const (

// IngressClassKey picks a specific "class" for the Ingress. The controller
// only processes Ingresses with this annotation either unset, or set
// to either gceIngessClass or the empty string.
// to either gceIngressClass or the empty string.
IngressClassKey = "kubernetes.io/ingress.class"
GceIngressClass = "gce"
GceMultiIngressClass = "gce-multi-cluster"
Expand Down Expand Up @@ -131,8 +140,35 @@ func (ing *Ingress) UseNamedTLS() string {
return val
}

func (ing *Ingress) StaticIPName() string {
val, ok := ing.v[StaticIPNameKey]
func (ing *Ingress) StaticIPName() (string, error) {
if !flags.F.EnableL7Ilb {
return ing.GlobalStaticIPName(), nil
}

globalIp := ing.GlobalStaticIPName()
regionalIp := ing.RegionalStaticIPName()

if globalIp != "" && regionalIp != "" {
return "", errors.New("Error: both global-static-ip and regional-static-ip cannot be specified")
}

if regionalIp != "" {
return regionalIp, nil
}

return globalIp, nil
}

func (ing *Ingress) GlobalStaticIPName() string {
val, ok := ing.v[GlobalStaticIPNameKey]
if !ok {
return ""
}
return val
}

func (ing *Ingress) RegionalStaticIPName() string {
val, ok := ing.v[RegionalStaticIPNameKey]
if !ok {
return ""
}
Expand Down
42 changes: 36 additions & 6 deletions pkg/annotations/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,49 @@ import (

"k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-gce/pkg/flags"
)

func TestIngress(t *testing.T) {
for _, tc := range []struct {
desc string
ing *v1beta1.Ingress
allowHTTP bool
useNamedTLS string
staticIPName string
ingressClass string
wantErr bool
}{
{
desc: "Empty ingress",
ing: &v1beta1.Ingress{},
allowHTTP: true, // defaults to true.
},
{
desc: "Global and Regional StaticIP Specified",
ing: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
AllowHTTPKey: "false",
IngressClassKey: "gce",
PreSharedCertKey: "shared-cert-key",
StaticIPNameKey: "1.2.3.4",
GlobalStaticIPNameKey: "1.2.3.4",
RegionalStaticIPNameKey: "10.0.0.0",
IngressClassKey: GceL7ILBIngressClass,
},
},
},
ingressClass: GceL7ILBIngressClass,
staticIPName: "",
allowHTTP: true,
wantErr: true,
},
{
desc: "Test most annotations",
ing: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
AllowHTTPKey: "false",
IngressClassKey: "gce",
PreSharedCertKey: "shared-cert-key",
GlobalStaticIPNameKey: "1.2.3.4",
},
},
},
Expand All @@ -53,14 +74,23 @@ func TestIngress(t *testing.T) {
},
} {
ing := FromIngress(tc.ing)

if tc.ingressClass == GceL7ILBIngressClass {
flags.F.EnableL7Ilb = true
}

if x := ing.AllowHTTP(); x != tc.allowHTTP {
t.Errorf("ingress %+v; AllowHTTP() = %v, want %v", tc.ing, x, tc.allowHTTP)
}
if x := ing.UseNamedTLS(); x != tc.useNamedTLS {
t.Errorf("ingress %+v; UseNamedTLS() = %v, want %v", tc.ing, x, tc.useNamedTLS)
}
if x := ing.StaticIPName(); x != tc.staticIPName {
t.Errorf("ingress %+v; StaticIPName() = %v, want %v", tc.ing, x, tc.staticIPName)
staticIp, err := ing.StaticIPName()
if (err != nil) != tc.wantErr {
t.Errorf("ingress: %+v, err = %v, wantErr = %v", tc.ing, err, tc.wantErr)
}
if staticIp != tc.staticIPName {
t.Errorf("ingress %+v; GlobalStaticIPName() = %v, want %v", tc.ing, staticIp, tc.staticIPName)
}
if x := ing.IngressClass(); x != tc.ingressClass {
t.Errorf("ingress %+v; IngressClass() = %v, want %v", tc.ing, x, tc.ingressClass)
Expand Down
2 changes: 1 addition & 1 deletion pkg/composite/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const (
// The other types that are discovered as dependencies will simply be wrapped with a composite struct
// The format of the map is ServiceName -> k8s-cloud-provider wrapper name
var MainServices = map[string]string{
"Address": "Addresses",
"Address": "Addresses",
"BackendService": "BackendServices",
"ForwardingRule": "ForwardingRules",
"HealthCheck": "HealthChecks",
Expand Down
7 changes: 6 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,12 +673,17 @@ func (lbc *LoadBalancerController) toRuntimeInfo(ing *v1beta1.Ingress, urlMap *u
feConfig = feConfig.DeepCopy()
}

staticIPName, err := annotations.StaticIPName()
if err != nil {
return nil, err
}

return &loadbalancers.L7RuntimeInfo{
TLS: tls,
TLSName: annotations.UseNamedTLS(),
Ingress: ing,
AllowHTTP: annotations.AllowHTTP(),
StaticIPName: annotations.StaticIPName(),
StaticIPName: staticIPName,
UrlMap: urlMap,
FrontendConfig: feConfig,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/fuzz/features/static_ip.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (v *staticIPValidator) ConfigureAttributes(env fuzz.ValidatorEnv, ing *v1be

// CheckResponse implements fuzz.FeatureValidator
func (v *staticIPValidator) CheckResponse(host, path string, resp *http.Response, body []byte) (fuzz.CheckResponseAction, error) {
addrName := annotations.FromIngress(v.ing).StaticIPName()
addrName := annotations.FromIngress(v.ing).GlobalStaticIPName()
if addrName == "" {
return fuzz.CheckResponseContinue, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/fuzz/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func (i *IngressBuilder) AddStaticIP(name string) *IngressBuilder {
if i.ing.Annotations == nil {
i.ing.Annotations = make(map[string]string)
}
i.ing.Annotations[annotations.StaticIPNameKey] = name
i.ing.Annotations[annotations.GlobalStaticIPNameKey] = name
return i
}

Expand Down
24 changes: 19 additions & 5 deletions pkg/loadbalancers/addresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ import (
"fmt"
"net/http"

"google.golang.org/api/compute/v1"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/klog"
)

// checkStaticIP reserves a static IP allocated to the Forwarding Rule.
// checkStaticIP reserves a regional or global static IP allocated to the Forwarding Rule.
func (l *L7) checkStaticIP() (err error) {
isInternal := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(&l.ingress)

if l.fw == nil || l.fw.IPAddress == "" {
return fmt.Errorf("will not create static IP without a forwarding rule")
}
Expand All @@ -50,10 +53,21 @@ func (l *L7) checkStaticIP() (err error) {
return nil
}

ip, _ := l.cloud.GetGlobalAddress(managedStaticIPName)
key, err := l.CreateKey(managedStaticIPName)
if err != nil {
return err
}

ip, _ := composite.GetAddress(l.cloud, key, meta.VersionGA)
if ip == nil {
klog.V(3).Infof("Creating static ip %v", managedStaticIPName)
err = l.cloud.ReserveGlobalAddress(&compute.Address{Name: managedStaticIPName, Address: l.fw.IPAddress})
address := &composite.Address{Name: managedStaticIPName, Address: l.fw.IPAddress, Version: meta.VersionGA}
if isInternal {
// Used for L7 ILB
address.AddressType = "INTERNAL"
}

err = composite.CreateAddress(l.cloud, key, address)
if err != nil {
if utils.IsHTTPErrorCode(err, http.StatusConflict) ||
utils.IsHTTPErrorCode(err, http.StatusBadRequest) {
Expand All @@ -63,7 +77,7 @@ func (l *L7) checkStaticIP() (err error) {
}
return err
}
ip, err = l.cloud.GetGlobalAddress(managedStaticIPName)
ip, err = composite.GetAddress(l.cloud, key, meta.VersionGA)
if err != nil {
return err
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/loadbalancers/forwarding_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,17 @@ func (l *L7) getEffectiveIP() (string, bool, error) {
// TODO: Handle the last case better.

if l.runtimeInfo.StaticIPName != "" {
key, err := l.CreateKey(l.runtimeInfo.StaticIPName)
if err != nil {
return "", false, err
}

// 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 {
return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing global static IP.",
l.runtimeInfo.StaticIPName)
// TODO(shance): Replace version
if ip, err := composite.GetAddress(l.cloud, key, meta.VersionGA); err != nil || ip == nil {
klog.Warningf("The given static IP name %v doesn't translate to an existing static IP, ignoring it and allocating a new IP: %v",
l.runtimeInfo.StaticIPName, err)
} else {
return ip.Address, false, nil
}
Expand Down
84 changes: 84 additions & 0 deletions pkg/loadbalancers/forwarding_rules_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package loadbalancers

import (
"testing"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/legacy-cloud-providers/gce"
)

func TestGetEffectiveIP(t *testing.T) {
testCases := []struct {
desc string
address *composite.Address
scope meta.KeyType
wantIp string
wantManaged bool
wantErr bool
}{
{
desc: "L7 ILB with Address created",
address: &composite.Address{Name: "test-ilb", Address: "10.2.3.4"},
scope: meta.Regional,
wantIp: "10.2.3.4",
wantManaged: false,
wantErr: false,
},
{
desc: "L7 ILB without address created",
scope: meta.Regional,
wantManaged: true,
wantErr: false,
},
{
desc: "XLB with Address created",
address: &composite.Address{Name: "test-ilb", Address: "35.2.3.4"},
scope: meta.Global,
wantIp: "35.2.3.4",
wantManaged: false,
wantErr: false,
},
{
desc: "XLB without Address created",
scope: meta.Global,
wantManaged: true,
wantErr: false,
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
l7 := L7{
cloud: fakeGCE,
scope: tc.scope,
runtimeInfo: &L7RuntimeInfo{StaticIPName: ""},
}

// Create Address if specified
if tc.address != nil {
key, err := l7.CreateKey(tc.address.Name)
if err != nil {
t.Fatal(err)
}
err = composite.CreateAddress(fakeGCE, key, tc.address)
if err != nil {
t.Fatal(err)
}
l7.runtimeInfo.StaticIPName = tc.address.Name
}

ip, managed, err := l7.getEffectiveIP()
if (err != nil) != tc.wantErr {
t.Errorf("getEffectiveIP() error = %v, wantErr %v", err, tc.wantErr)
return
}
if tc.address != nil && ip != tc.wantIp {
t.Errorf("getEffectiveIP() ip = %v, want %v", ip, tc.wantIp)
}
if managed != tc.wantManaged {
t.Errorf("getEffectiveIP() managed = %v, want %v", managed, tc.wantManaged)
}
})
}
}
Loading

0 comments on commit d5344ec

Please sign in to comment.