Skip to content

Commit

Permalink
Add tests for GC to-be-deleted NEGs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sawsa307
sawsa307 committed Dec 2, 2024
1 parent ed1037f commit 7d86b8c
Showing 1 changed file with 137 additions and 93 deletions.
230 changes: 137 additions & 93 deletions pkg/neg/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const (
negName1 = "neg1"

defaultTestSubnet = "default"
additionalTestSubnet = "additional-subnet"
defaultTestSubnetURL = "https://www.googleapis.com/compute/v1/projects/mock-project/regions/test-region/subnetworks/default"
)

Expand Down Expand Up @@ -1186,7 +1187,10 @@ func TestNegCRDeletions(t *testing.T) {
}

func TestGarbageCollectionNegCrdEnabled(t *testing.T) {
t.Parallel()
prevFlag := flags.F.EnableMultiSubnetClusterPhase1
defer func() {
flags.F.EnableMultiSubnetClusterPhase1 = prevFlag
}()

svc := &v1.Service{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -1234,7 +1238,8 @@ func TestGarbageCollectionNegCrdEnabled(t *testing.T) {

// expectGenNamedNegGC indicates that the Neg GC only occurs if using a generated name
// expectNegGC will take precedence over this value
expectGenNamedNegGC bool
expectGenNamedNegGC bool
includeToBeDeletedNeg bool
}{
{desc: "neg config not in svcPortMap, marked for deletion",
negsExist: true,
Expand Down Expand Up @@ -1319,6 +1324,16 @@ func TestGarbageCollectionNegCrdEnabled(t *testing.T) {
negDesc: matchingDesc.String(),
expectedNegCount: 2,
},
{desc: "neg config in svcPortMap, include to-be-deleted NEGs",
negsExist: true,
markedForDeletion: false,
desiredConfig: true,
includeToBeDeletedNeg: true,
expectNegGC: false,
expectCrGC: false,
expectedNegCount: 2, // We should expect NEG to be deleted but not GC NEG CR
negDesc: matchingDesc.String(),
},
{desc: "negs don't exist, config not in svcPortMap",
negsExist: false,
markedForDeletion: false,
Expand Down Expand Up @@ -1371,119 +1386,148 @@ func TestGarbageCollectionNegCrdEnabled(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
for _, customName := range []bool{true, false} {
for _, networkEndpointType := range []negtypes.NetworkEndpointType{negtypes.VmIpPortEndpointType, negtypes.NonGCPPrivateEndpointType, negtypes.VmIpEndpointType} {

kubeClient := fake.NewSimpleClientset()
manager, testCloud, _ := NewTestSyncerManager(kubeClient)
svcNegClient := manager.svcNegClient

if tc.negCrGCError != nil {
svcNegClientFake := svcNegClient.(*negfake.Clientset)
svcNegClientFake.PrependReactor("delete", "servicenetworkendpointgroups", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &negv1beta1.ServiceNetworkEndpointGroup{}, tc.negCrGCError
})
}
for _, enableMultiSubnetClusterPhase1 := range []bool{true, false} {
if !enableMultiSubnetClusterPhase1 && tc.includeToBeDeletedNeg {
continue
}
flags.F.EnableMultiSubnetClusterPhase1 = enableMultiSubnetClusterPhase1
for _, customName := range []bool{true, false} {
for _, networkEndpointType := range []negtypes.NetworkEndpointType{negtypes.VmIpPortEndpointType, negtypes.NonGCPPrivateEndpointType, negtypes.VmIpEndpointType} {

kubeClient := fake.NewSimpleClientset()
manager, testCloud, _ := NewTestSyncerManager(kubeClient)
svcNegClient := manager.svcNegClient

if tc.negCrGCError != nil {
svcNegClientFake := svcNegClient.(*negfake.Clientset)
svcNegClientFake.PrependReactor("delete", "servicenetworkendpointgroups", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &negv1beta1.ServiceNetworkEndpointGroup{}, tc.negCrGCError
})
}

manager.serviceLister.Add(svc)
fakeNegCloud := manager.cloud
manager.serviceLister.Add(svc)
fakeNegCloud := manager.cloud

version := meta.VersionGA
if networkEndpointType == negtypes.VmIpEndpointType {
version = meta.VersionAlpha
}
version := meta.VersionGA
if networkEndpointType == negtypes.VmIpEndpointType {
version = meta.VersionAlpha
}

// Create NEG to be GC'ed
negName := manager.namer.NEG("test", "test", port80)
if customName {
negName = "test"
}
// Create NEG to be GC'ed
negName := manager.namer.NEG("test", "test", port80)
nonDefaultNegName := manager.namer.NonDefaultSubnetNEG("test", "test", additionalTestSubnet, port80)

if customName {
negName = "test"
var err error
nonDefaultNegName, err = manager.namer.NonDefaultSubnetCustomNEG("test", additionalTestSubnet)
if err != nil {
t.Fatalf("Failed to generate custom name for non-default subnet NEG: %v", err)
}
}

for _, zone := range zones {
fakeNegCloud.CreateNetworkEndpointGroup(&composite.NetworkEndpointGroup{
Version: version,
Name: negName,
NetworkEndpointType: string(networkEndpointType),
Description: tc.negDesc,
}, zone, klog.TODO())
}
for _, zone := range zones {
if err := fakeNegCloud.CreateNetworkEndpointGroup(&composite.NetworkEndpointGroup{
Version: version,
Name: negName,
NetworkEndpointType: string(networkEndpointType),
Description: tc.negDesc,
}, zone, klog.TODO()); err != nil {
t.Fatalf("Failed to create NEG %s in zone %s: %v", negName, zone, err)
}

if tc.includeToBeDeletedNeg {
if err := fakeNegCloud.CreateNetworkEndpointGroup(&composite.NetworkEndpointGroup{
Version: version,
Name: nonDefaultNegName,
NetworkEndpointType: string(networkEndpointType),
Description: tc.negDesc,
}, zone, klog.TODO()); err != nil {
t.Fatalf("Failed to create NEG %s in zone %s: %v", nonDefaultNegName, zone, err)
}
}
}

gcPortInfo := negtypes.PortInfo{PortTuple: negtypes.SvcPortTuple{Port: port80}, NegName: negName}
cr := createNegCR(svc, serviceKey{namespace: testServiceNamespace, name: testServiceName}, gcPortInfo)
if tc.markedForDeletion {
now := metav1.Now()
cr.SetDeletionTimestamp(&now)
}
gcPortInfo := negtypes.PortInfo{PortTuple: negtypes.SvcPortTuple{Port: port80}, NegName: negName}
cr := createNegCR(svc, serviceKey{namespace: testServiceNamespace, name: testServiceName}, gcPortInfo)
if tc.markedForDeletion {
now := metav1.Now()
cr.SetDeletionTimestamp(&now)
}

if !tc.emptyNegRefList {
if !tc.malformedNegSelflink {
cr.Status.NetworkEndpointGroups = getNegObjectRefs(t, fakeNegCloud, zones, negName, version, negv1beta1.ActiveState)
} else {
cr.Status.NetworkEndpointGroups = []negv1beta1.NegObjectReference{{SelfLink: ""}}
if !tc.emptyNegRefList {
if !tc.malformedNegSelflink {
cr.Status.NetworkEndpointGroups = getNegObjectRefs(t, fakeNegCloud, zones, negName, version, negv1beta1.ActiveState)
if tc.includeToBeDeletedNeg {
cr.Status.NetworkEndpointGroups = append(cr.Status.NetworkEndpointGroups, getNegObjectRefs(t, fakeNegCloud, zones, nonDefaultNegName, version, negv1beta1.ToBeDeletedState)...)
}
} else {
cr.Status.NetworkEndpointGroups = []negv1beta1.NegObjectReference{{SelfLink: ""}}
}
}
}

if _, err := manager.svcNegClient.NetworkingV1beta1().ServiceNetworkEndpointGroups(cr.Namespace).Create(context2.TODO(), &cr, metav1.CreateOptions{}); err != nil {
t.Fatalf("failed to create neg cr: %v", err)
}
if _, err := manager.svcNegClient.NetworkingV1beta1().ServiceNetworkEndpointGroups(cr.Namespace).Create(context2.TODO(), &cr, metav1.CreateOptions{}); err != nil {
t.Fatalf("failed to create neg cr: %v", err)
}

populateSvcNegCache(t, manager, svcNegClient, testServiceNamespace)
populateSvcNegCache(t, manager, svcNegClient, testServiceNamespace)

if tc.desiredConfig {
// Add config to svc map
serviceKey := getServiceKey(testServiceNamespace, testServiceName)
ports := make(negtypes.PortInfoMap)
ports[negtypes.PortInfoMapKey{ServicePort: port80}] = gcPortInfo
if tc.desiredConfig {
// Add config to svc map
serviceKey := getServiceKey(testServiceNamespace, testServiceName)
ports := make(negtypes.PortInfoMap)
ports[negtypes.PortInfoMapKey{ServicePort: port80}] = gcPortInfo

manager.svcPortMap[serviceKey] = ports
}
manager.svcPortMap[serviceKey] = ports
}

if !tc.negsExist {
for _, zone := range []string{negtypes.TestZone1, negtypes.TestZone2} {
fakeNegCloud.DeleteNetworkEndpointGroup(negName, zone, version, klog.TODO())
if !tc.negsExist {
for _, zone := range []string{negtypes.TestZone1, negtypes.TestZone2} {
fakeNegCloud.DeleteNetworkEndpointGroup(negName, zone, version, klog.TODO())
}
}
}

if tc.negGCError != nil {
mockCloud := testCloud.Compute().(*cloud.MockGCE)
mockNEG := mockCloud.NetworkEndpointGroups().(*cloud.MockNetworkEndpointGroups)
if tc.negGCError != nil {
mockCloud := testCloud.Compute().(*cloud.MockGCE)
mockNEG := mockCloud.NetworkEndpointGroups().(*cloud.MockNetworkEndpointGroups)

for _, zone := range tc.negGCErrorZone {
mockNEG.DeleteError[*meta.ZonalKey(negName, zone)] = tc.negGCError
for _, zone := range tc.negGCErrorZone {
mockNEG.DeleteError[*meta.ZonalKey(negName, zone)] = tc.negGCError
}
}
}

err := manager.GC()
if !tc.expectErr && err != nil {
t.Fatalf("failed to GC: %v", err)
}
if tc.expectErr && err == nil {
t.Errorf("expected GC to error")
}
err := manager.GC()
if !tc.expectErr && err != nil {
t.Fatalf("failed to GC: %v", err)
}
if tc.expectErr && err == nil {
t.Errorf("expected GC to error")
}

negs, err := fakeNegCloud.AggregatedListNetworkEndpointGroup(version, klog.TODO())
if err != nil {
t.Errorf("failed getting negs from cloud: %s", err)
}
negs, err := fakeNegCloud.AggregatedListNetworkEndpointGroup(version, klog.TODO())
if err != nil {
t.Errorf("failed getting negs from cloud: %s", err)
}

numExistingNegs := len(negs)
numExistingNegs := len(negs)

expectNegGC := tc.expectNegGC || (tc.expectGenNamedNegGC && !customName)
if tc.negsExist && expectNegGC && numExistingNegs != 0 {
t.Errorf("expected negs to be GCed, but found %d", numExistingNegs)
}
if tc.negsExist && !expectNegGC && numExistingNegs != tc.expectedNegCount {
t.Errorf("expected %d negs in the cloud, but found %d", tc.expectedNegCount, numExistingNegs)
}
expectNegGC := tc.expectNegGC || (tc.expectGenNamedNegGC && !customName)
if tc.negsExist && expectNegGC && numExistingNegs != 0 {
t.Errorf("expected negs to be GCed, but found %d", numExistingNegs)
}
if tc.negsExist && !expectNegGC && numExistingNegs != tc.expectedNegCount {
t.Errorf("expected %d negs in the cloud, but found %d", tc.expectedNegCount, numExistingNegs)
}

crs := getNegCRs(t, svcNegClient, testServiceNamespace)
crDeleted := checkForNegCRDeletion(crs, negName)
crs := getNegCRs(t, svcNegClient, testServiceNamespace)
crDeleted := checkForNegCRDeletion(crs, negName)

if tc.expectCrGC && !crDeleted {
t.Errorf("expected neg %s to be deleted", negName)
}
if !tc.expectCrGC && crDeleted && !tc.markedForDeletion {
t.Errorf("expected neg %s to not be deleted", negName)
if tc.expectCrGC && !crDeleted {
t.Errorf("expected neg %s to be deleted", negName)
}
if !tc.expectCrGC && crDeleted && !tc.markedForDeletion {
t.Errorf("expected neg %s to not be deleted", negName)
}
}
}
}
Expand Down

0 comments on commit 7d86b8c

Please sign in to comment.