Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds annotation kubernetes.io/ingress.reserve-global-static-ip-name to signal GLBC to manage static IP #1182

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions pkg/annotations/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ const (
// responsibility to create/delete it.
RegionalStaticIPNameKey = "kubernetes.io/ingress.regional-static-ip-name"

// ReserveGlobalStaticIPNameKey tells the Ingress controller to manage a specific
// GCE static ip for its forwarding rules. If specified, the Ingress controller
// reserves and assigns the static ip by this name to the forwarding rules of the
// given ingress. It differs from StaticIPNameKey, since the controller does manage
// this ip. Do not use the same value for both this annotation and GlobalStaticIPNameKey.
ReserveGlobalStaticIPNameKey = "kubernetes.io/ingress.reserve-global-static-ip-name"

// PreSharedCertKey represents the specific pre-shared SSL
// certificate for the Ingress controller to use. The controller *does not*
// manage this certificate, it is the users responsibility to create/delete it.
Expand Down Expand Up @@ -119,17 +126,13 @@ func FromIngress(ing *v1beta1.Ingress) *Ingress {
return &Ingress{ing.Annotations}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment

// AllowHTTP returns the allowHTTP flag. True by default.
func (ing *Ingress) AllowHTTP() bool {
val, ok := ing.v[AllowHTTPKey]
// ReserveGlobalStaticIPName returns the value for the ReserveGlobalStaticIPNameKey annotation.
func (ing *Ingress) ReserveGlobalStaticIPName() string {
val, ok := ing.v[ReserveGlobalStaticIPNameKey]
if !ok {
return true
}
v, err := strconv.ParseBool(val)
if err != nil {
return true
return ""
}
return v
return val
}

// UseNamedTLS returns the name of the GCE SSL certificate. Empty by default.
Expand Down Expand Up @@ -177,6 +180,19 @@ func (ing *Ingress) RegionalStaticIPName() string {
return val
}

// AllowHTTP returns the allowHTTP flag. True by default.
func (ing *Ingress) AllowHTTP() bool {
val, ok := ing.v[AllowHTTPKey]
if !ok {
return true
}
v, err := strconv.ParseBool(val)
if err != nil {
return true
}
return v
}

func (ing *Ingress) IngressClass() string {
val, ok := ing.v[IngressClassKey]
if !ok {
Expand Down
29 changes: 22 additions & 7 deletions pkg/annotations/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ import (

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 string
ing *v1beta1.Ingress
allowHTTP bool
useNamedTLS string
staticIPName string
ingressClass string
reserveGlobalStaticIPName string
wantErr bool
}{
{
desc: "Empty ingress",
Expand Down Expand Up @@ -72,6 +73,17 @@ func TestIngress(t *testing.T) {
staticIPName: "1.2.3.4",
ingressClass: "gce",
},
{
ing: &v1beta1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
ReserveGlobalStaticIPNameKey: "1.2.3.4-managed",
},
},
},
allowHTTP: true,
reserveGlobalStaticIPName: "1.2.3.4-managed",
},
} {
ing := FromIngress(tc.ing)

Expand All @@ -95,5 +107,8 @@ func TestIngress(t *testing.T) {
if x := ing.IngressClass(); x != tc.ingressClass {
t.Errorf("ingress %+v; IngressClass() = %v, want %v", tc.ing, x, tc.ingressClass)
}
if x := ing.ReserveGlobalStaticIPName(); x != tc.reserveGlobalStaticIPName {
t.Errorf("ingress %+v; ReserveGlobalStaticIPName() = %v, want %v", tc.ing, x, tc.reserveGlobalStaticIPName)
}
}
}
15 changes: 8 additions & 7 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,13 +685,14 @@ func (lbc *LoadBalancerController) toRuntimeInfo(ing *v1beta1.Ingress, urlMap *u
}

return &loadbalancers.L7RuntimeInfo{
TLS: tls,
TLSName: annotations.UseNamedTLS(),
Ingress: ing,
AllowHTTP: annotations.AllowHTTP(),
StaticIPName: staticIPName,
UrlMap: urlMap,
FrontendConfig: feConfig,
TLS: tls,
TLSName: annotations.UseNamedTLS(),
Ingress: ing,
AllowHTTP: annotations.AllowHTTP(),
StaticIPName: staticIPName,
ReserveGlobalStaticIPName: annotations.ReserveGlobalStaticIPName(),
UrlMap: urlMap,
FrontendConfig: feConfig,
}, nil
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/loadbalancers/addresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func (l *L7) checkStaticIP() (err error) {
return fmt.Errorf("will not create static IP without a forwarding rule")
}
managedStaticIPName := l.namer.ForwardingRule(namer.HTTPProtocol)
// Don't manage staticIPs if the user has specified an IP.
if l.runtimeInfo.ReserveGlobalStaticIPName != "" && l.ip == nil {
managedStaticIPName = l.runtimeInfo.ReserveGlobalStaticIPName
}
// Don't manage staticIPs if the user has specified an IP and if ReserveGlobalStaticIPName was not provided.
address, manageStaticIP, err := l.getEffectiveIP()
if err != nil {
return err
Expand Down
5 changes: 5 additions & 0 deletions pkg/loadbalancers/forwarding_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ func (l *L7) getEffectiveIP() (string, bool, error) {
// User specifies a different IP on startup:
// - We create a forwarding rule with the given IP.
// - If this ip doesn't exist in GCE, an error is thrown which fails ingress creation.
// - If the ReserveGlobalStaticIPName annotation is provided and this ip doesn't exist in GCE,
// it should be created.
// - 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
Expand All @@ -177,6 +179,9 @@ func (l *L7) getEffectiveIP() (string, bool, error) {
// till the Ingress is torn down.
// TODO(shance): Replace version
if ip, err := composite.GetAddress(l.cloud, key, meta.VersionGA); err != nil || ip == nil {
if l.runtimeInfo.ReserveGlobalStaticIPName != "" && l.ip == nil {
return "", true, nil
}
return "", false, fmt.Errorf("the given static IP name %v doesn't translate to an existing static IP.",
l.runtimeInfo.StaticIPName)
} else {
Expand Down
4 changes: 4 additions & 0 deletions pkg/loadbalancers/l7.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ type L7RuntimeInfo struct {
StaticIPName string
// The name of the static IP subnet, this is only used for L7-ILB Ingress static IPs
StaticIPSubnet string
// The name of a Global Static IP that the ingress controller should manage.
// If specified, the static IP will be reserved with the cloud provider and
// the IP associated with this name is used in the Forwarding Rules for this loadbalancer.
ReserveGlobalStaticIPName string
// UrlMap is our internal representation of a url map.
UrlMap *utils.GCEURLMap
// FrontendConfig is the type which encapsulates features for the load balancer.
Expand Down
101 changes: 101 additions & 0 deletions pkg/loadbalancers/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,107 @@ func TestStaticIP(t *testing.T) {
}
}

// Test ReserveGlobalStaticIPName annotation behavior.
// When ReserveGlobalStaticIPName is specified, a static IP with its value should be created.
func TestReserveGlobalStaticIPName(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{
"ReserveGlobalStaticIPNameKey": "testmanagedstaticip",
}
lbInfo := &L7RuntimeInfo{
AllowHTTP: true,
TLS: []*translator.TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Ingress: ing,
ReserveGlobalStaticIPName: "testmanagedstaticip",
}

if _, err := j.pool.Ensure(lbInfo); err != nil {
t.Fatalf("expected no error ensuring ingress with non-existent static ip with ReserveGlobalStaticIPName set - %v", err)
}
// Get the reserved static IP value
if _, err := j.fakeGCE.GetGlobalAddress("testmanagedstaticip"); err != nil {
t.Fatalf("ip address reservation failed - %v", err)
}
}

// When an existing ReserveGlobalStaticIPName value is specified,
// the behavior shouldn't be different from not specifying ReserveGlobalStaticIP (ingress creation must pass).
func TestReserveGlobalStaticIPNameExistingValue(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{
"ReserveGlobalStaticIPNameKey": "testmanagedstaticip",
}
lbInfo := &L7RuntimeInfo{
AllowHTTP: true,
TLS: []*translator.TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Ingress: ing,
ReserveGlobalStaticIPName: "testmanagedstaticip",
}

// The only difference from the default behavior is that, with ReserveGlobalStaticIPName set
// and by specifying a non-existent IP, the IP will be managed by GLBC.
// Create static IP
err := j.fakeGCE.ReserveGlobalAddress(&compute.Address{Name: "testmanagedstaticip", 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)
}
}

// When ReserveGlobalStaticIPName is specified at a later time,
// the ingress update should fail.
// This will be simulated by reserving a static IP and going through the same behavior as StaticIPName.
// That is the same behavior as to when a TLS pre-shared cert is specified and a static IP is reserved.
func TestReserveGlobalStaticIPNameLateChange(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: []*translator.TLSCerts{{Key: "key", Cert: "cert"}},
UrlMap: gceUrlMap,
Ingress: ing,
StaticIPName: "teststaticip",
}
// 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("expected no error ensuring ingress - %v", err)
}
ing.Annotations = map[string]string{
"ReserveGlobalStaticIPNameKey": "testmanagedstaticip",
}
lbInfo.ReserveGlobalStaticIPName = "testmanagedstaticip"

if _, err := j.pool.Ensure(lbInfo); err != nil {
t.Fatalf("expected no error ensuring ingress - %v", err)
}
// static IP shouldn't be created
if _, err := j.fakeGCE.GetGlobalAddress("testmanagedstaticip"); err == nil {
t.Fatalf("expected error getting a static ip, got no error")
}
}

// Test setting frontendconfig Ssl policy
func TestFrontendConfigSslPolicy(t *testing.T) {
flags.F.EnableFrontendConfig = true
Expand Down