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

Update endpoints for NEGs from non-default subnet. #2728

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
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
6 changes: 3 additions & 3 deletions pkg/backends/ig_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import (

const (
defaultTestZone = "zone-a"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"
)

func newTestIGLinker(fakeGCE *gce.Cloud, fakeInstancePool instancegroups.Manager) *instanceGroupLinker {
Expand All @@ -59,7 +59,7 @@ func TestLink(t *testing.T) {
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())

nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.AddFakeNodes(fakeZoneGetter, defaultTestZone, "test-instance")

fakeNodePool := instancegroups.NewManager(&instancegroups.ManagerConfig{
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestLinkWithCreationModeError(t *testing.T) {
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())

nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.AddFakeNodes(fakeZoneGetter, defaultTestZone, "test-instance")

fakeNodePool := instancegroups.NewManager(&instancegroups.ManagerConfig{
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func newTestJig(fakeGCE *gce.Cloud) *Jig {
fakeIGs := instancegroups.NewEmptyFakeInstanceGroups()

nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.AddFakeNodes(fakeZoneGetter, defaultTestZone, "test-instance")

fakeInstancePool := instancegroups.NewManager(&instancegroups.ManagerConfig{
Expand Down
2 changes: 1 addition & 1 deletion pkg/backends/regional_ig_linker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func newTestRegionalIgLinker(fakeGCE *gce.Cloud, backendPool *Backends, l4Namer
fakeIGs := instancegroups.NewEmptyFakeInstanceGroups()

nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.AddFakeNodes(fakeZoneGetter, usCentral1AZone, "test-instance1")
zonegetter.AddFakeNodes(fakeZoneGetter, "us-central1-c", "test-instance2")

Expand Down
6 changes: 1 addition & 5 deletions pkg/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func NewControllerContext(
logger,
)
// The subnet specified in gce.conf is considered as the default subnet.
context.ZoneGetter = zonegetter.NewZoneGetter(context.NodeInformer, context.Cloud.SubnetworkURL())
context.ZoneGetter = zonegetter.NewZoneGetter(context.NodeInformer, context.NodeTopologyInformer, context.Cloud.SubnetworkURL())
context.InstancePool = instancegroups.NewManager(&instancegroups.ManagerConfig{
Cloud: context.Cloud,
Namer: context.ClusterNamer,
Expand Down Expand Up @@ -328,10 +328,6 @@ func (ctx *ControllerContext) HasSynced() bool {
funcs = append(funcs, ctx.FirewallInformer.HasSynced)
}

if ctx.NodeTopologyInformer != nil {
funcs = append(funcs, ctx.NodeTopologyInformer.HasSynced)
}

for _, f := range funcs {
if !f() {
return false
Expand Down
8 changes: 5 additions & 3 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import (
"k8s.io/ingress-gce/pkg/utils/zonegetter"
)

const defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
const defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"

var (
nodePortCounter = 30000
Expand All @@ -71,9 +71,11 @@ func newLoadBalancerController() *LoadBalancerController {
kubeClient := fake.NewSimpleClientset()
backendConfigClient := backendconfigclient.NewSimpleClientset()
svcNegClient := svcnegclient.NewSimpleClientset()
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
vals := gce.DefaultTestClusterValues()
vals.SubnetworkURL = defaultTestSubnetURL
fakeGCE := gce.NewFakeGCECloud(vals)
nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.AddFakeNodes(fakeZoneGetter, fakeZone, "test-node")

(fakeGCE.Compute().(*cloud.MockGCE)).MockGlobalForwardingRules.InsertHook = loadbalancers.InsertGlobalForwardingRuleHook
Expand Down
2 changes: 1 addition & 1 deletion pkg/instancegroups/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestSync(t *testing.T) {
config.HasSynced = func() bool {
return true
}
config.ZoneGetter = zonegetter.NewFakeZoneGetter(informer, defaultTestSubnetURL, false)
config.ZoneGetter = zonegetter.NewFakeZoneGetter(informer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)

controller := NewController(config, logr.Logger{})

Expand Down
11 changes: 6 additions & 5 deletions pkg/instancegroups/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ package instancegroups

import (
"fmt"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-gce/pkg/utils"
"net/http"
"strings"
"testing"

apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-gce/pkg/utils"

"google.golang.org/api/googleapi"
"k8s.io/klog/v2"

Expand All @@ -42,14 +43,14 @@ const (
testZoneC = "dark-moon1-c"
basePath = "/basepath/projects/project-id/"

defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"
)

var defaultNamer = namer.NewNamer("uid1", "fw1", klog.TODO())

func newNodePool(f Provider, maxIGSize int) Manager {
nodeInformer := zonegetter.FakeNodeInformer()
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)

pool := NewManager(&ManagerConfig{
Cloud: f,
Expand Down
2 changes: 2 additions & 0 deletions pkg/l4lb/l4controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"k8s.io/ingress-gce/pkg/test"
"k8s.io/ingress-gce/pkg/utils/common"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/ingress-gce/pkg/utils/zonegetter"
)

const (
Expand Down Expand Up @@ -852,6 +853,7 @@ func newServiceController(t *testing.T, fakeGCE *gce.Cloud) *L4Controller {
NumL4Workers: 5,
}
ctx := context.NewControllerContext(kubeClient, nil, nil, nil, svcNegClient, nil, nil, nil, kubeClient /*kube client to be used for events*/, fakeGCE, namer, "" /*kubeSystemUID*/, ctxConfig, klog.TODO())
ctx.ZoneGetter = zonegetter.NewFakeZoneGetter(ctx.NodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
// Add some nodes so that NEG linker kicks in during ILB creation.
nodes, err := test.CreateAndInsertNodes(ctx.Cloud, []string{"instance-1"}, vals.ZoneName)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/l4lb/l4netlbcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const (
shortSessionAffinityIdleTimeout = int32(20) // 20 sec could be used for regular Session Affinity
longSessionAffinityIdleTimeout = int32(2 * 60) // 2 min or 120 sec for Strong Session Affinity

defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"
)

var (
Expand Down
7 changes: 4 additions & 3 deletions pkg/neg/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import (
svcnegclient "k8s.io/ingress-gce/pkg/svcneg/client/clientset/versioned"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/endpointslices"
namer2 "k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/ingress-gce/pkg/utils/patch"
"k8s.io/ingress-gce/pkg/utils/zonegetter"
"k8s.io/klog/v2"
Expand All @@ -68,7 +68,7 @@ type Controller struct {
gcPeriod time.Duration
recorder record.EventRecorder
namer negtypes.NetworkEndpointGroupNamer
l4Namer namer2.L4ResourcesNamer
l4Namer namer.L4ResourcesNamer
zoneGetter *zonegetter.ZoneGetter
networkResolver network.Resolver

Expand Down Expand Up @@ -135,7 +135,7 @@ func NewController(
gkeNetworkParamSetInformer cache.SharedIndexInformer,
nodeTopologyInformer cache.SharedIndexInformer,
hasSynced func() bool,
l4Namer namer2.L4ResourcesNamer,
l4Namer namer.L4ResourcesNamer,
defaultBackendService utils.ServicePort,
cloud negtypes.NetworkEndpointGroupCloud,
zoneGetter *zonegetter.ZoneGetter,
Expand Down Expand Up @@ -182,6 +182,7 @@ func NewController(
syncerMetrics := syncMetrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger)
manager := newSyncerManager(
namer,
l4Namer,
recorder,
cloud,
zoneGetter,
Expand Down
7 changes: 4 additions & 3 deletions pkg/neg/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func newTestControllerWithParamsAndContext(kubeClient kubernetes.Interface, test
}
nodeInformer := zonegetter.FakeNodeInformer()
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)

return NewController(
kubeClient,
Expand Down Expand Up @@ -1756,7 +1756,7 @@ func validateServiceAnnotationWithPortInfoMap(t *testing.T, svc *apiv1.Service,

nodeInformer := zonegetter.FakeNodeInformer()
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zones, _ := zoneGetter.ListZones(negtypes.NodeFilterForEndpointCalculatorMode(portInfoMap.EndpointsCalculatorMode()), klog.TODO())
if !sets.NewString(expectZones...).Equal(sets.NewString(zones...)) {
t.Errorf("Unexpected zones listed by the predicate function, got %v, want %v", zones, expectZones)
Expand Down Expand Up @@ -1837,7 +1837,7 @@ func validateServiceStateAnnotationExceptNames(t *testing.T, svc *apiv1.Service,
}
nodeInformer := zonegetter.FakeNodeInformer()
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
// This routine is called from tests verifying L7 NEGs.
zones, _ := zoneGetter.ListZones(negtypes.NodeFilterForEndpointCalculatorMode(negtypes.L7Mode), klog.TODO())

Expand Down Expand Up @@ -2115,6 +2115,7 @@ func newTestNode(name string, unschedulable bool) *apiv1.Node {
Name: name,
},
Spec: apiv1.NodeSpec{
PodCIDR: "10.100.1.0/24",
Unschedulable: unschedulable,
},
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/neg/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
svcnegclient "k8s.io/ingress-gce/pkg/svcneg/client/clientset/versioned"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/common"
"k8s.io/ingress-gce/pkg/utils/namer"
"k8s.io/ingress-gce/pkg/utils/patch"
"k8s.io/ingress-gce/pkg/utils/zonegetter"
"k8s.io/klog/v2"
Expand All @@ -64,6 +65,7 @@ func (k serviceKey) Key() string {
// syncerManager contains all the active syncer goroutines and manage their lifecycle.
type syncerManager struct {
namer negtypes.NetworkEndpointGroupNamer
l4Namer namer.L4ResourcesNamer
recorder record.EventRecorder
cloud negtypes.NetworkEndpointGroupCloud
zoneGetter *zonegetter.ZoneGetter
Expand Down Expand Up @@ -118,6 +120,7 @@ type syncerManager struct {
}

func newSyncerManager(namer negtypes.NetworkEndpointGroupNamer,
l4Namer namer.L4ResourcesNamer,
recorder record.EventRecorder,
cloud negtypes.NetworkEndpointGroupCloud,
zoneGetter *zonegetter.ZoneGetter,
Expand All @@ -140,6 +143,7 @@ func newSyncerManager(namer negtypes.NetworkEndpointGroupNamer,

return &syncerManager{
namer: namer,
l4Namer: l4Namer,
recorder: recorder,
cloud: cloud,
zoneGetter: zoneGetter,
Expand Down Expand Up @@ -248,6 +252,8 @@ func (manager *syncerManager) EnsureSyncers(namespace, name string, newPorts neg
manager.lpConfig,
manager.enableDualStackNEG,
portInfo.NetworkInfo,
manager.namer,
manager.l4Namer,
)
manager.syncerMap[syncerKey] = syncer
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/neg/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,17 @@ const (
negName1 = "neg1"

defaultTestSubnet = "default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"
)

func NewTestSyncerManager(kubeClient kubernetes.Interface) (*syncerManager, *gce.Cloud, *negtypes.TestContext) {
testContext := negtypes.NewTestContextWithKubeClient(kubeClient)
nodeInformer := zonegetter.FakeNodeInformer()
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
manager := newSyncerManager(
testContext.NegNamer,
testContext.L4Namer,
record.NewFakeRecorder(100),
negtypes.NewAdapter(testContext.Cloud),
zoneGetter,
Expand Down
4 changes: 2 additions & 2 deletions pkg/neg/readiness/reflector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
)

const (
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/proj/regions/us-central1/subnetworks/default"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"

defaultTestSubnet = "default"
nonDefaultTestSubnet = "non-default"
Expand All @@ -59,7 +59,7 @@ func (f *fakeLookUp) ReadinessGateEnabled(syncerKey negtypes.NegSyncerKey) bool
}

func newTestReadinessReflector(testContext *negtypes.TestContext, enableMultiSubnetCluster bool) *readinessReflector {
fakeZoneGetter := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, defaultTestSubnetURL, enableMultiSubnetCluster)
fakeZoneGetter := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, testContext.NodeTopologyInformer, defaultTestSubnetURL, enableMultiSubnetCluster)
reflector := NewReadinessReflector(
testContext.KubeClient,
testContext.KubeClient,
Expand Down
10 changes: 6 additions & 4 deletions pkg/neg/syncers/endpoints_calculator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ import (
// The L7 implementation is tested in TestToZoneNetworkEndpointMapUtil.
func TestLocalGetEndpointSet(t *testing.T) {
nodeInformer := zonegetter.FakeNodeInformer()
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zonegetter.SetNodeTopologyHasSynced(zoneGetter, func() bool { return true })
defaultNetwork := network.NetworkInfo{IsDefault: true, K8sNetwork: "default", SubnetworkURL: defaultTestSubnetURL}
prevFlag := flags.F.EnableMultiSubnetCluster
defer func() { flags.F.EnableMultiSubnetCluster = prevFlag }()
Expand Down Expand Up @@ -196,8 +197,9 @@ func nodeInterfacesAnnotation(t *testing.T, network, ip string) string {
// TestClusterGetEndpointSet verifies the GetEndpointSet method implemented by the ClusterL4EndpointsCalculator.
func TestClusterGetEndpointSet(t *testing.T) {
nodeInformer := zonegetter.FakeNodeInformer()
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(nodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
zonegetter.PopulateFakeNodeInformer(nodeInformer, false)
zonegetter.SetNodeTopologyHasSynced(zoneGetter, func() bool { return true })
defaultNetwork := network.NetworkInfo{IsDefault: true, K8sNetwork: "default", SubnetworkURL: defaultTestSubnetURL}
prevFlag := flags.F.EnableMultiSubnetCluster
defer func() { flags.F.EnableMultiSubnetCluster = prevFlag }()
Expand Down Expand Up @@ -397,10 +399,10 @@ func TestValidateEndpoints(t *testing.T) {
nodeLister := testContext.NodeInformer.GetIndexer()
serviceLister := testContext.ServiceInformer.GetIndexer()
zonegetter.PopulateFakeNodeInformer(testContext.NodeInformer, true)
zoneGetter := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, defaultTestSubnetURL, false)
zoneGetter := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, false)
L7EndpointsCalculator := NewL7EndpointsCalculator(zoneGetter, podLister, nodeLister, serviceLister, svcPort, klog.TODO(), testContext.EnableDualStackNEG, metricscollector.FakeSyncerMetrics())

zoneGetterMSC := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, defaultTestSubnetURL, true)
zoneGetterMSC := zonegetter.NewFakeZoneGetter(testContext.NodeInformer, zonegetter.FakeNodeTopologyInformer(), defaultTestSubnetURL, true)
L7EndpointsCalculatorMSC := NewL7EndpointsCalculator(zoneGetterMSC, podLister, nodeLister, serviceLister, svcPort, klog.TODO(), testContext.EnableDualStackNEG, metricscollector.FakeSyncerMetrics())
L7EndpointsCalculatorMSC.enableMultiSubnetCluster = true
L4LocalEndpointCalculator := NewLocalL4EndpointsCalculator(listers.NewNodeLister(nodeLister), zoneGetter, fmt.Sprintf("%s/%s", testServiceName, testServiceNamespace), klog.TODO(), &network.NetworkInfo{SubnetworkURL: defaultTestSubnetURL}, negtypes.L4InternalLB)
Expand Down
Loading