Skip to content

Commit 99637c3

Browse files
rosmorosmo
authored
Merge pull request #159 from rohandvora/svcneg
Support getting zones using svcneg resources.
2 parents 05704ba + e7cef8b commit 99637c3

7 files changed

Lines changed: 228 additions & 97 deletions

File tree

‎config/rbac/role.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,11 @@ rules:
3535
- get
3636
- patch
3737
- update
38+
- apiGroups:
39+
- networking.gke.io
40+
resources:
41+
- servicenetworkendpointgroups
42+
verbs:
43+
- list
44+
- watch
45+

‎controllers/autoneg.go‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,18 @@ import (
2121
"encoding/json"
2222
"errors"
2323
"fmt"
24+
"regexp"
25+
"slices"
2426
"sort"
2527
"time"
2628

2729
backoff "github.com/cenkalti/backoff/v5"
2830
"google.golang.org/api/compute/v1"
2931
"google.golang.org/api/googleapi"
32+
apierrors "k8s.io/apimachinery/pkg/api/errors"
33+
"k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1"
34+
"sigs.k8s.io/controller-runtime/pkg/client"
35+
"sigs.k8s.io/controller-runtime/pkg/log"
3036
)
3137

3238
const (
@@ -48,6 +54,7 @@ const (
4854
var (
4955
errConfigInvalid = errors.New("autoneg configuration invalid")
5056
errJSONInvalid = errors.New("json malformed")
57+
zoneRE = regexp.MustCompile(`zones/([^/]+)`)
5158
)
5259

5360
type errNotFound struct {
@@ -463,7 +470,8 @@ func validateNewConfig(config AutonegConfig) error {
463470
return nil
464471
}
465472

466-
func getStatuses(namespace string, name string, annotations map[string]string, r *ServiceReconciler) (s Statuses, valid bool, err error) {
473+
func getStatuses(ctx context.Context, namespace string, name string, annotations map[string]string, r *ServiceReconciler) (s Statuses, valid bool, err error) {
474+
logger := log.FromContext(ctx)
467475
// Read the current cloud.google.com/neg annotation
468476
tmp, ok := annotations[negAnnotation]
469477
if ok {
@@ -596,6 +604,60 @@ func getStatuses(namespace string, name string, annotations map[string]string, r
596604
if err = json.Unmarshal([]byte(tmp), &s.negStatus); err != nil {
597605
return
598606
}
607+
// Check if we should use ServiceNetworkEndpointGroup custom resource to get the NEG zones.
608+
if r.UseSvcNeg {
609+
logger.Info("Getting zones using svcneg custom resources")
610+
var zones []string
611+
zones, err = zonesFromSvcNeg(ctx, r, namespace, &s.negStatus)
612+
if err != nil {
613+
return
614+
}
615+
// Update the zones.
616+
logger.Info("Got zones from svcnegs", "zones", zones)
617+
s.negStatus.Zones = zones
618+
}
599619
}
620+
600621
return
601622
}
623+
624+
func zonesFromSvcNeg(ctx context.Context, reader client.Reader, namespace string, negStatus *NEGStatus) ([]string, error) {
625+
logger := log.FromContext(ctx)
626+
zones := []string{}
627+
negsProcessed := map[string]bool{}
628+
for _, neg := range negStatus.NEGs {
629+
if _, ok := negsProcessed[neg]; ok {
630+
continue
631+
}
632+
negsProcessed[neg] = true
633+
svcNeg := v1beta1.ServiceNetworkEndpointGroup{}
634+
err := reader.Get(ctx, client.ObjectKey{
635+
Namespace: namespace, Name: neg,
636+
}, &svcNeg)
637+
if apierrors.IsNotFound(err) {
638+
logger.Info("SvcNeg not found", "neg", neg)
639+
continue
640+
}
641+
if err != nil {
642+
return nil, fmt.Errorf("failed to get svcneg %s: %w", neg, err)
643+
}
644+
for _, negRef := range svcNeg.Status.NetworkEndpointGroups {
645+
negZone := zone(negRef)
646+
if !slices.Contains(zones, negZone) {
647+
zones = append(zones, negZone)
648+
}
649+
}
650+
}
651+
return zones, nil
652+
}
653+
654+
func zone(ref v1beta1.NegObjectReference) string {
655+
// Of the format: https://www.googleapis.com/compute/beta/projects/<project-id>/zones/<zone>/networkEndpointGroups/<neg>
656+
matches := zoneRE.FindStringSubmatch(ref.SelfLink)
657+
// The first submatch (index 0) is the entire matched string "zones/us-central1-c"
658+
// The second submatch (index 1) is the content of the first capturing group "us-central1-c"
659+
if len(matches) > 1 {
660+
return matches[1]
661+
}
662+
return ""
663+
}

‎controllers/autoneg_test.go‎

Lines changed: 94 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,19 @@ package controllers
1818

1919
import (
2020
"context"
21-
"google.golang.org/api/option"
2221
"math"
2322
"net/http"
2423
"net/http/httptest"
2524
"reflect"
2625
"testing"
2726

27+
"google.golang.org/api/option"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
29+
2830
"google.golang.org/api/compute/v1"
31+
apierrors "k8s.io/apimachinery/pkg/api/errors"
32+
"k8s.io/apimachinery/pkg/runtime/schema"
33+
"k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1"
2934
"k8s.io/utils/pointer"
3035
)
3136

@@ -285,7 +290,7 @@ func TestGetStatuses(t *testing.T) {
285290
DeregisterNEGsOnAnnotationRemoval: true,
286291
}
287292
for _, st := range statusTests {
288-
_, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler)
293+
_, valid, err := getStatuses(context.Background(), "ns", "test", st.annotations, &serviceReconciler)
289294
if err != nil && !st.err {
290295
t.Errorf("Set %q: expected no error, got one: %v", st.name, err)
291296
}
@@ -308,7 +313,7 @@ func TestGetOldStatuses(t *testing.T) {
308313
DeregisterNEGsOnAnnotationRemoval: true,
309314
}
310315
for _, st := range oldStatusTests {
311-
_, valid, err := getStatuses("ns", "test", st.annotations, &serviceReconciler)
316+
_, valid, err := getStatuses(context.Background(), "ns", "test", st.annotations, &serviceReconciler)
312317
if err != nil && !st.err {
313318
t.Errorf("Set %q: expected no error, got one: %v", st.name, err)
314319
}
@@ -330,7 +335,7 @@ func TestGetStatusesServiceNameNotAllowed(t *testing.T) {
330335
AllowServiceName: false,
331336
}
332337
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
333-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
338+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
334339
if err != nil {
335340
t.Errorf("Expected no error, got one: %v", err)
336341
}
@@ -349,7 +354,7 @@ func TestGetStatusesServiceNameAllowed(t *testing.T) {
349354
AllowServiceName: true,
350355
}
351356
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
352-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
357+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
353358
if err != nil {
354359
t.Errorf("Expected no error, got one: %v", err)
355360
}
@@ -368,7 +373,7 @@ func TestGetStatusesOnlyAutonegStatusAnnotation(t *testing.T) {
368373
AllowServiceName: true,
369374
DeregisterNEGsOnAnnotationRemoval: true,
370375
}
371-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegStatusAnnotation: validAutonegStatus}, &serviceReconciler)
376+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegStatusAnnotation: validAutonegStatus}, &serviceReconciler)
372377
if err != nil {
373378
t.Errorf("Expected no error, got one: %v", err)
374379
}
@@ -391,7 +396,7 @@ func TestGetStatusesOnlyOldAutonegStatusAnnotation(t *testing.T) {
391396
AllowServiceName: true,
392397
DeregisterNEGsOnAnnotationRemoval: true,
393398
}
394-
statuses, valid, err := getStatuses("ns", "test", map[string]string{oldAutonegStatusAnnotation: validAutonegStatus}, &serviceReconciler)
399+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{oldAutonegStatusAnnotation: validAutonegStatus}, &serviceReconciler)
395400
if err != nil {
396401
t.Errorf("Expected no error, got one: %v", err)
397402
}
@@ -415,7 +420,7 @@ func TestDefaultMaxRatePerEndpointWhenOverrideIsSet(t *testing.T) {
415420
MaxRatePerEndpointDefault: 1234,
416421
}
417422
validConf := `{"backend_services":{"80":[{"name":"http-be","max_rate_per_endpoint":100}]}}`
418-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
423+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
419424
if err != nil {
420425
t.Errorf("Expected no error, got one: %v", err)
421426
}
@@ -438,7 +443,7 @@ func TestDefaultMaxRatePerEndpointWhenOverrideIsNotSet(t *testing.T) {
438443
MaxRatePerEndpointDefault: 1234,
439444
}
440445
validConf := `{"backend_services":{"80":[{"name":"http-be"}]}}`
441-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
446+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
442447
if err != nil {
443448
t.Errorf("Expected no error, got one: %v", err)
444449
}
@@ -461,7 +466,7 @@ func TestDefaultConnectionPerEndpointWhenOverrideIsSet(t *testing.T) {
461466
MaxConnectionsPerEndpointDefault: 1234,
462467
}
463468
validConf := `{"backend_services":{"80":[{"name":"http-be","max_connections_per_endpoint":100}]}}`
464-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
469+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
465470
if err != nil {
466471
t.Errorf("Expected no error, got one: %v", err)
467472
}
@@ -484,7 +489,7 @@ func TestDefaultMaxConnectionsEndpointWhenOverrideIsNotSet(t *testing.T) {
484489
MaxConnectionsPerEndpointDefault: 1234,
485490
}
486491
validConf := `{"backend_services":{"80":[{"name":"http-be"}]}}`
487-
statuses, valid, err := getStatuses("ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
492+
statuses, valid, err := getStatuses(context.Background(), "ns", "test", map[string]string{autonegAnnotation: validConf}, &serviceReconciler)
488493
if err != nil {
489494
t.Errorf("Expected no error, got one: %v", err)
490495
}
@@ -1006,3 +1011,81 @@ func TestReconcileBackendsDeletionWithMissingBackend(t *testing.T) {
10061011
t.Errorf("ReconcileBackends() got err: %v, want none", err)
10071012
}
10081013
}
1014+
1015+
type fakeReader struct {
1016+
client.Reader
1017+
svcNeg *v1beta1.ServiceNetworkEndpointGroup
1018+
getErr error
1019+
}
1020+
1021+
func (r *fakeReader) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
1022+
if r.svcNeg != nil {
1023+
r.svcNeg.DeepCopyInto(obj.(*v1beta1.ServiceNetworkEndpointGroup))
1024+
}
1025+
return r.getErr
1026+
}
1027+
1028+
func TestZonesFromSvcNeg(t *testing.T) {
1029+
tests := []struct {
1030+
name string
1031+
negStatus *NEGStatus
1032+
svcNeg *v1beta1.ServiceNetworkEndpointGroup
1033+
getSvcNegErr error
1034+
wantZones []string
1035+
wantErr bool
1036+
}{
1037+
{
1038+
name: "success",
1039+
svcNeg: &v1beta1.ServiceNetworkEndpointGroup{
1040+
Status: v1beta1.ServiceNetworkEndpointGroupStatus{
1041+
NetworkEndpointGroups: []v1beta1.NegObjectReference{
1042+
{
1043+
SelfLink: "https://www.googleapis.com/compute/beta/projects/test-project/zones/zone1/networkEndpointGroups/neg_name",
1044+
},
1045+
{
1046+
SelfLink: "https://www.googleapis.com/compute/beta/projects/test-project/zones/zone2/networkEndpointGroups/neg_name",
1047+
},
1048+
},
1049+
},
1050+
},
1051+
negStatus: &NEGStatus{
1052+
NEGs: map[string]string{"80": fakeNeg, "90": fakeNeg2},
1053+
},
1054+
wantZones: []string{"zone1", "zone2"},
1055+
wantErr: false,
1056+
},
1057+
{
1058+
name: "svcneg not found",
1059+
getSvcNegErr: apierrors.NewNotFound(schema.GroupResource{}, ""),
1060+
negStatus: &NEGStatus{
1061+
NEGs: map[string]string{"80": fakeNeg},
1062+
},
1063+
wantZones: []string{},
1064+
wantErr: false,
1065+
},
1066+
{
1067+
name: "get svcneg error",
1068+
getSvcNegErr: apierrors.NewForbidden(schema.GroupResource{}, "", nil),
1069+
negStatus: &NEGStatus{
1070+
NEGs: map[string]string{"80": fakeNeg},
1071+
},
1072+
wantErr: true,
1073+
},
1074+
}
1075+
1076+
for _, tt := range tests {
1077+
t.Run(tt.name, func(t *testing.T) {
1078+
r := &fakeReader{
1079+
svcNeg: tt.svcNeg,
1080+
getErr: tt.getSvcNegErr,
1081+
}
1082+
zones, err := zonesFromSvcNeg(context.Background(), r, "test", tt.negStatus)
1083+
if (err != nil) != tt.wantErr {
1084+
t.Errorf("ZonesFromSvcNeg() error = %v, wantErr %v", err, tt.wantErr)
1085+
}
1086+
if !reflect.DeepEqual(zones, tt.wantZones) {
1087+
t.Errorf("ZonesFromSvcNeg() zones = %v, want %v", zones, tt.wantZones)
1088+
}
1089+
})
1090+
}
1091+
}

‎controllers/service_controller.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
apierrors "k8s.io/apimachinery/pkg/api/errors"
2828
"k8s.io/apimachinery/pkg/runtime"
2929
"k8s.io/client-go/tools/record"
30+
"k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1"
3031
ctrl "sigs.k8s.io/controller-runtime"
3132
"sigs.k8s.io/controller-runtime/pkg/client"
3233
"sigs.k8s.io/controller-runtime/pkg/log"
@@ -50,6 +51,7 @@ type ServiceReconciler struct {
5051
AlwaysReconcile bool
5152
ReconcileDuration *time.Duration
5253
DeregisterNEGsOnAnnotationRemoval bool
54+
UseSvcNeg bool
5355
}
5456

5557
//+kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;update;patch
@@ -80,7 +82,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
8082
return r.reconcileResult(err)
8183
}
8284

83-
status, ok, err := getStatuses(svc.Namespace, svc.Name, svc.ObjectMeta.Annotations, r)
85+
status, ok, err := getStatuses(ctx, svc.Namespace, svc.Name, svc.ObjectMeta.Annotations, r)
8486
// Is this service using autoneg?
8587
if !ok {
8688
return r.reconcileResult(nil)
@@ -137,6 +139,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
137139
// Write changes to the service object.
138140
if deleting {
139141
// Remove finalizer and clear status
142+
logger.Info("Removing finalizer")
140143
svc.ObjectMeta.Finalizers = removeString(svc.ObjectMeta.Finalizers, oldAutonegFinalizer)
141144
svc.ObjectMeta.Finalizers = removeString(svc.ObjectMeta.Finalizers, autonegFinalizer)
142145
delete(svc.ObjectMeta.Annotations, autonegStatusAnnotation)
@@ -207,6 +210,12 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
207210

208211
// SetupWithManager sets up the controller with the Manager.
209212
func (r *ServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
213+
if r.UseSvcNeg {
214+
return ctrl.NewControllerManagedBy(mgr).
215+
For(&corev1.Service{}).
216+
Owns(&v1beta1.ServiceNetworkEndpointGroup{}).
217+
Complete(r)
218+
}
210219
return ctrl.NewControllerManagedBy(mgr).
211220
For(&corev1.Service{}).
212221
Complete(r)

0 commit comments

Comments
 (0)