From 727a50e8c189eddb56d27802abb4a11f31bf1fda Mon Sep 17 00:00:00 2001
From: Swetha Repakula <srepakula@google.com>
Date: Thu, 30 Jul 2020 16:20:28 -0700
Subject: [PATCH] Ignore deletion timestamp when ensuring NEG CR

 - svcPortMap is the source of truth, so continue neg creation and
 syncing even if corresponding NEG CR has a deletion timestamp
---
 pkg/neg/manager.go      | 16 +++++++---------
 pkg/neg/manager_test.go |  2 +-
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/pkg/neg/manager.go b/pkg/neg/manager.go
index 844b5476ed..1e139d3db6 100644
--- a/pkg/neg/manager.go
+++ b/pkg/neg/manager.go
@@ -140,6 +140,13 @@ func (manager *syncerManager) EnsureSyncers(namespace, name string, newPorts neg
 		syncerKey := getSyncerKey(namespace, name, svcPort, portInfo)
 		syncer, ok := manager.syncerMap[syncerKey]
 		if !ok {
+
+			// To ensure that a NEG CR always exists during the lifecyle of a NEG, do not create a syncer for the NEG until the NEG CR is successfully created. This will reduce the possibility of invalid states and reduces complexity of garbage collection
+			if err := manager.ensureSvcNegCR(key, portInfo); err != nil {
+				errList = append(errList, err)
+				continue
+			}
+
 			// determine the implementation that calculates NEG endpoints on each sync.
 			epc := negsyncer.GetEndpointsCalculator(manager.nodeLister, manager.podLister, manager.zoneGetter,
 				syncerKey, portInfo.RandomizeEndpoints)
@@ -163,10 +170,6 @@ func (manager *syncerManager) EnsureSyncers(namespace, name string, newPorts neg
 			manager.syncerMap[syncerKey] = syncer
 		}
 
-		if err := manager.ensureSvcNegCR(key, portInfo); err != nil {
-			errList = append(errList, err)
-		}
-
 		if syncer.IsStopped() {
 			if err := syncer.Start(); err != nil {
 				errList = append(errList, err)
@@ -430,11 +433,6 @@ func (manager *syncerManager) ensureSvcNegCR(svcKey serviceKey, portInfo negtype
 		return err
 	}
 
-	if !negCR.GetDeletionTimestamp().IsZero() {
-		// Allow GC to complete
-		return fmt.Errorf("Neg CR already exists with this name and is marked for deletion. Allow GC to complete before recreating.")
-	}
-
 	needUpdate, err := ensureNegCRLabels(negCR, labels)
 	if err != nil {
 		return err
diff --git a/pkg/neg/manager_test.go b/pkg/neg/manager_test.go
index 3f0e0efa42..f9f3c9d475 100644
--- a/pkg/neg/manager_test.go
+++ b/pkg/neg/manager_test.go
@@ -842,7 +842,7 @@ func TestNegCRDuplicateCreations(t *testing.T) {
 			svc:               svc1,
 			svcTuple:          svcTuple1,
 			markedForDeletion: true,
-			expectErr:         true,
+			expectErr:         false,
 			crExists:          true,
 		},
 		{desc: "same service, different port configuration, original cr is not marked for deletion",