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 31, 2020
1 parent 9da839a commit ff47b8b
Show file tree
Hide file tree
Showing 12 changed files with 325 additions and 33 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 @@ -672,12 +672,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 @@ -261,7 +261,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
29 changes: 24 additions & 5 deletions pkg/loadbalancers/addresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ 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) {
if l.fw == nil || l.fw.IPAddress == "" {
return fmt.Errorf("will not create static IP without a forwarding rule")
Expand All @@ -50,10 +51,17 @@ 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 := l.newStaticAddress(managedStaticIPName)

err = composite.CreateAddress(l.cloud, key, address)
if err != nil {
if utils.IsHTTPErrorCode(err, http.StatusConflict) ||
utils.IsHTTPErrorCode(err, http.StatusBadRequest) {
Expand All @@ -63,11 +71,22 @@ 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
}
}
l.ip = ip
return nil
}

func (l *L7) newStaticAddress(name string) *composite.Address {
isInternal := flags.F.EnableL7Ilb && utils.IsGCEL7ILBIngress(&l.ingress)
address := &composite.Address{Name: name, Address: l.fw.IPAddress, Version: meta.VersionGA}
if isInternal {
// Used for L7 ILB
address.AddressType = "INTERNAL"
}

return address
}
72 changes: 72 additions & 0 deletions pkg/loadbalancers/addresses_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package loadbalancers

import (
"testing"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/google/go-cmp/cmp"
"k8s.io/api/networking/v1beta1"
"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/flags"
)

func TestNewStaticAddress(t *testing.T) {
testCases := []struct {
desc string
ip string
name string
isInternal bool
expected *composite.Address
}{
{
desc: "external static address",
ip: "1.2.3.4",
name: "external-addr",
isInternal: false,
expected: &composite.Address{Name: "external-addr", Version: meta.VersionGA, Address: "1.2.3.4"},
},
{
desc: "internal static address",
ip: "10.2.3.4",
name: "internal-addr",
isInternal: true,
expected: &composite.Address{Name: "internal-addr", Version: meta.VersionGA, Address: "10.2.3.4", AddressType: "INTERNAL"},
},
}

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
l7 := &L7{
ingress: v1beta1.Ingress{Spec: v1beta1.IngressSpec{}},
fw: &composite.ForwardingRule{IPAddress: tc.ip},
}

if tc.isInternal {
flags.F.EnableL7Ilb = true
l7.ingress.Annotations = map[string]string{annotations.IngressClassKey: "gce-internal"}
}

result := l7.newStaticAddress(tc.name)
if diff := cmp.Diff(tc.expected, result); diff != "" {
t.Errorf("Got diff for Address (-want +got):\n%s", diff)
}
})
}
}
10 changes: 8 additions & 2 deletions pkg/loadbalancers/forwarding_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,16 @@ 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.",
// TODO(shance): Replace version
if ip, err := composite.GetAddress(l.cloud, key, meta.VersionGA); err != nil || ip == nil {
return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing static IP.",
l.runtimeInfo.StaticIPName)
} else {
return ip.Address, false, nil
Expand Down
Loading

0 comments on commit ff47b8b

Please sign in to comment.