Repository: nginx/kubernetes-ingress

Issue:
[Bug]: Uninstalling helm chart can cause issues with NGINX Plus mgmt config
### Version

edge

### What Kubernetes platforms are you running on?

Kind

### Steps to reproduce

When running NIC installed with NGINX plus  and configured to deliver usage reports to a custom endpoint.  If the helm chart is uninstalled the nginx instance can send a usage report to the default endpoint.

This occurs when the pod is marked by the Kubernetes cluster for deletion, Kubernetes then deletes the MGMT `ConfigMap`.  NIC applies the default ConfigMap values and restarts NGINX.  Eventually the pod is garbage collected and NIC/NGINX are stopped.

Discussion:
@pdabelf5: running helm with `--cascade foreground ` makes sure the Pod is garbage collected before the ConfigMap is removed and mitigates the issue@pdabelf5: Using `OwnerReferences` on the `ConfigMap` will also likely address the problem.@pdabelf5: Possible solution using [helm hooks](https://stackoverflow.com/questions/76040061/how-can-i-dynamically-parse-uid-in-the-ownerreference-helm-chart/77918374#77918374)@tstraley: Helm Hooks are managed differently from other resources, and often lead to headaches over object ownership and lifecycle. I'd push for avoiding adding helm hooks to the chart if it can be avoided https://helm.sh/docs/topics/charts_hooks/#hook-resources-are-not-managed-with-corresponding-releases@AlexFenlon: Useful links in relation to `OwnerReferences` :
-  https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/
- https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/
- https://kubernetes.io/blog/2021/05/14/using-finalizers-to-control-deletion/
@AlexFenlon: some investiagtion - 
tried adding OwnerReference to the mgmtConfigMap for Pod, Replicaset or Configmap and nothing semes to work, it still gets reset to the default mgmt block just before the deployment is uninstalled via helm. @shaun-nx: @tstraley the team managed to get a fix in for this. Plan is to either v5.1.0 or v5.0.1 if we do a patch release before then.


Solution patch:
diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go
index 09b156fe7..95323dfd1 100644
--- a/cmd/nginx-ingress/main.go
+++ b/cmd/nginx-ingress/main.go
@@ -323,6 +323,7 @@ func main() {
 		NICVersion:                   version,
 		DynamicWeightChangesReload:   *enableDynamicWeightChangesReload,
 		InstallationFlags:            parsedFlags,
+		ShuttingDown:                 false,
 	}
 
 	lbc := k8s.NewLoadBalancerController(lbcInput)
@@ -816,6 +817,7 @@ func handleTermination(lbc *k8s.LoadBalancerController, nginxManager nginx.Manag
 		nl.Fatalf(lbc.Logger, "AppProtectDosAgent command exited unexpectedly with status: %v", err)
 	case <-signalChan:
 		nl.Info(lbc.Logger, "Received SIGTERM, shutting down")
+		lbc.ShuttingDown = true
 		lbc.Stop()
 		nginxManager.Quit()
 		<-cpcfg.nginxDone
diff --git a/internal/k8s/configmap.go b/internal/k8s/configmap.go
index 39c0b0878..92bf241e9 100644
--- a/internal/k8s/configmap.go
+++ b/internal/k8s/configmap.go
@@ -82,6 +82,11 @@ func (lbc *LoadBalancerController) syncConfigMap(task task) {
 	key := task.Key
 	nl.Debugf(lbc.Logger, "Syncing configmap %v", key)
 
+	if key == lbc.mgmtConfigMapName && lbc.isPodMarkedForDeletion() {
+		nl.Debugf(lbc.Logger, "Pod is shutting down, skipping management ConfigMap sync")
+		return
+	}
+
 	switch key {
 	case lbc.nginxConfigMapName:
 		obj, configExists, err := lbc.configMapLister.GetByKey(key)
@@ -120,6 +125,9 @@ func (lbc *LoadBalancerController) syncConfigMap(task task) {
 		nl.Debugf(lbc.Logger, "Skipping ConfigMap update because batch sync is on")
 		return
 	}
-
+	if err := lbc.ctx.Err(); err != nil {
+		nl.Debugf(lbc.Logger, "Context canceled, skipping ConfigMap sync for %v: %v", task.Key, err)
+		return
+	}
 	lbc.updateAllConfigs()
 }
diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go
index f84139403..28a7ff989 100644
--- a/internal/k8s/controller.go
+++ b/internal/k8s/controller.go
@@ -185,6 +185,7 @@ type LoadBalancerController struct {
 	weightChangesDynamicReload    bool
 	nginxConfigMapName            string
 	mgmtConfigMapName             string
+	ShuttingDown                  bool
 }
 
 var keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
@@ -241,6 +242,7 @@ type NewLoadBalancerControllerInput struct {
 	NICVersion                   string
 	DynamicWeightChangesReload   bool
 	InstallationFlags            []string
+	ShuttingDown                 bool
 }
 
 // NewLoadBalancerController creates a controller
@@ -286,6 +288,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
 		weightChangesDynamicReload:   input.DynamicWeightChangesReload,
 		nginxConfigMapName:           input.ConfigMaps,
 		mgmtConfigMapName:            input.MGMTConfigMap,
+		ShuttingDown:                 input.ShuttingDown,
 	}
 
 	lbc.syncQueue = newTaskQueue(lbc.Logger, lbc.sync)
@@ -3649,3 +3652,20 @@ func (lbc *LoadBalancerController) createCombinedDeploymentHeadlessServiceName()
 	combinedDeployment := fmt.Sprintf("%s-%s", name, strings.ToLower(owner.Kind))
 	return combinedDeployment
 }
+
+func (lbc *LoadBalancerController) isPodMarkedForDeletion() bool {
+	// Check if the controller is shutting down
+	if lbc.ShuttingDown {
+		nl.Debugf(lbc.Logger, "SIGTERM already received, controller is shutting down")
+		return true
+	}
+	podName := lbc.metadata.pod.Name
+	podNamespace := lbc.metadata.pod.Namespace
+	pod, err := lbc.client.CoreV1().Pods(podNamespace).Get(context.Background(), podName, meta_v1.GetOptions{})
+	if err == nil && pod.DeletionTimestamp != nil {
+		nl.Debugf(lbc.Logger, "Pod %s/%s is marked for deletion", podNamespace, podName)
+		return true
+	}
+
+	return false
+}



Test patch:
diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go
index 84efe85a7..e23443bd2 100644
--- a/internal/k8s/controller_test.go
+++ b/internal/k8s/controller_test.go
@@ -3639,3 +3639,86 @@ func TestCreateIngressExWithZoneSync(t *testing.T) {
 		}
 	}
 }
+
+func TestIsPodMarkedForDeletion(t *testing.T) {
+	t.Parallel()
+
+	logger := nl.LoggerFromContext(context.Background())
+
+	tests := []struct {
+		name            string
+		shutdownFlag    bool
+		envPodName      string
+		envPodNamespace string
+		podExists       bool
+		podHasTimestamp bool
+		expectedResult  bool
+	}{
+		{
+			name:           "controller is shutting down",
+			shutdownFlag:   true,
+			expectedResult: true,
+		},
+		{
+			name:            "pod exists with deletion timestamp",
+			envPodName:      "test-pod",
+			envPodNamespace: "test-namespace",
+			podExists:       true,
+			podHasTimestamp: true,
+			expectedResult:  true,
+		},
+		{
+			name:            "pod exists without deletion timestamp",
+			envPodName:      "test-pod",
+			envPodNamespace: "test-namespace",
+			podExists:       true,
+			podHasTimestamp: false,
+			expectedResult:  false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			client := fake.NewSimpleClientset()
+			if test.podExists {
+				pod := &api_v1.Pod{
+					ObjectMeta: meta_v1.ObjectMeta{
+						Name:      test.envPodName,
+						Namespace: test.envPodNamespace,
+					},
+				}
+
+				if test.podHasTimestamp {
+					now := meta_v1.Now()
+					pod.DeletionTimestamp = &now
+				}
+
+				_, err := client.CoreV1().Pods(test.envPodNamespace).Create(context.Background(), pod, meta_v1.CreateOptions{})
+				if err != nil {
+					t.Fatalf("Error creating pod: %v", err)
+				}
+			}
+
+			lbc := &LoadBalancerController{
+				client: client,
+				metadata: controllerMetadata{
+					pod: &api_v1.Pod{
+						ObjectMeta: meta_v1.ObjectMeta{
+							Name:      test.envPodName,
+							Namespace: test.envPodNamespace,
+						},
+					},
+				},
+				ShuttingDown: test.shutdownFlag,
+				Logger:       logger,
+			}
+
+			// Call the function and verify result
+			result := lbc.isPodMarkedForDeletion()
+			if result != test.expectedResult {
+				t.Errorf("Returned %v but expected %v", result, test.expectedResult)
+			}
+		})
+	}
+}