From 8fb7dbd364afee6597591306547918a65b16e4e8 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 23 Apr 2025 11:38:14 +0100 Subject: [PATCH 1/6] add signal based fix --- cmd/nginx-ingress/main.go | 2 ++ internal/k8s/configmap.go | 5 +++++ internal/k8s/controller.go | 22 ++++++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 75c4be0319..754f3463aa 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 39c0b0878f..4a62eeeb53 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) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index beca13b334..9fd2d22f09 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) @@ -3645,3 +3648,22 @@ 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 recieved, controller is shutting down") + return true + } + podName := os.Getenv("POD_NAME") + podNamespace := os.Getenv("POD_NAMESPACE") + + if podName != "" && podNamespace != "" { + 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 +} From efce0c7b16297e2bbe6383731ca16f1982734460 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 23 Apr 2025 12:45:52 +0100 Subject: [PATCH 2/6] simplify logic and add tests --- internal/k8s/controller.go | 14 +++--- internal/k8s/controller_test.go | 76 +++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 9fd2d22f09..d9b2a6fefd 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -3652,18 +3652,16 @@ func (lbc *LoadBalancerController) createCombinedDeploymentHeadlessServiceName() func (lbc *LoadBalancerController) isPodMarkedForDeletion() bool { // Check if the controller is shutting down if lbc.ShuttingDown { - nl.Debugf(lbc.Logger, "SIGTERM already recieved, controller is shutting down") + nl.Debugf(lbc.Logger, "SIGTERM already received, controller is shutting down") return true } podName := os.Getenv("POD_NAME") podNamespace := os.Getenv("POD_NAMESPACE") - - if podName != "" && podNamespace != "" { - 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 - } + 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 } diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 84efe85a70..064dc6ffd7 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -3639,3 +3639,79 @@ 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, + 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) + } + }) + } +} From 8c4e95258e46ab4fa81cf1e1603f2c727f87769b Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 23 Apr 2025 12:51:52 +0100 Subject: [PATCH 3/6] fix formatting --- internal/k8s/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 064dc6ffd7..feb0286186 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -3639,6 +3639,7 @@ func TestCreateIngressExWithZoneSync(t *testing.T) { } } } + func TestIsPodMarkedForDeletion(t *testing.T) { t.Parallel() @@ -3679,7 +3680,6 @@ func TestIsPodMarkedForDeletion(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - client := fake.NewSimpleClientset() if test.podExists { From ddbea2995f919da0665fc874e401200fb89991e5 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 23 Apr 2025 13:14:55 +0100 Subject: [PATCH 4/6] fix test-case --- internal/k8s/controller_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index feb0286186..4e02df9f71 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "reflect" "sort" "strings" @@ -3681,7 +3682,8 @@ func TestIsPodMarkedForDeletion(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { client := fake.NewSimpleClientset() - + os.Setenv("POD_NAME", test.envPodName) + os.Setenv("POD_NAMESPACE", test.envPodNamespace) if test.podExists { pod := &api_v1.Pod{ ObjectMeta: meta_v1.ObjectMeta{ From 314e251db137b66f42652d6aac3c697375e7ff7e Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 23 Apr 2025 13:53:47 +0100 Subject: [PATCH 5/6] fix linting issues and add context cancellation check --- internal/k8s/configmap.go | 5 ++++- internal/k8s/controller_test.go | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/internal/k8s/configmap.go b/internal/k8s/configmap.go index 4a62eeeb53..92bf241e9b 100644 --- a/internal/k8s/configmap.go +++ b/internal/k8s/configmap.go @@ -125,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_test.go b/internal/k8s/controller_test.go index 4e02df9f71..b0b8e2bf8e 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -3682,8 +3682,15 @@ func TestIsPodMarkedForDeletion(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { client := fake.NewSimpleClientset() - os.Setenv("POD_NAME", test.envPodName) - os.Setenv("POD_NAMESPACE", test.envPodNamespace) + + err := os.Setenv("POD_NAME", test.envPodName) + if err != nil { + t.Fatalf("Failed to set POD_NAME environment variable: %v", err) + } + err = os.Setenv("POD_NAMESPACE", test.envPodNamespace) + if err != nil { + t.Fatalf("Failed to set POD_NAMESPACE environment variable: %v", err) + } if test.podExists { pod := &api_v1.Pod{ ObjectMeta: meta_v1.ObjectMeta{ From 4bb3f81ed3657dd056dbaef3145ae5f03b427dbd Mon Sep 17 00:00:00 2001 From: Venktesh Date: Mon, 28 Apr 2025 14:29:55 +0100 Subject: [PATCH 6/6] update method to use lbc metadata --- internal/k8s/controller.go | 4 ++-- internal/k8s/controller_test.go | 20 +++++++++----------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index a71e17155f..28a7ff9899 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -3659,8 +3659,8 @@ func (lbc *LoadBalancerController) isPodMarkedForDeletion() bool { nl.Debugf(lbc.Logger, "SIGTERM already received, controller is shutting down") return true } - podName := os.Getenv("POD_NAME") - podNamespace := os.Getenv("POD_NAMESPACE") + 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) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index b0b8e2bf8e..e23443bd2d 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" "reflect" "sort" "strings" @@ -3682,15 +3681,6 @@ func TestIsPodMarkedForDeletion(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { client := fake.NewSimpleClientset() - - err := os.Setenv("POD_NAME", test.envPodName) - if err != nil { - t.Fatalf("Failed to set POD_NAME environment variable: %v", err) - } - err = os.Setenv("POD_NAMESPACE", test.envPodNamespace) - if err != nil { - t.Fatalf("Failed to set POD_NAMESPACE environment variable: %v", err) - } if test.podExists { pod := &api_v1.Pod{ ObjectMeta: meta_v1.ObjectMeta{ @@ -3711,7 +3701,15 @@ func TestIsPodMarkedForDeletion(t *testing.T) { } lbc := &LoadBalancerController{ - client: client, + client: client, + metadata: controllerMetadata{ + pod: &api_v1.Pod{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: test.envPodName, + Namespace: test.envPodNamespace, + }, + }, + }, ShuttingDown: test.shutdownFlag, Logger: logger, }