Refactor extraction of ingress pod details

cmd/nginx/main_test.go

@@ -26,7 +26,6 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
@@ -58,7 +57,7 @@ func TestHandleSigterm(t *testing.T) {
 		namespace = "test"
 	)
 
-	k8s.IngressNGINXPod = &v1.Pod{
+	k8s.IngressPodDetails = &k8s.PodInfo{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      podName,
 			Namespace: namespace,

ing

@@ -0,0 +1,43 @@
+apiVersion: networking.k8s.io/v1beta1
+kind: Ingress
+metadata:
+  name: simple-grpc
+  annotations:
+    nginx.ingress.kubernetes.io/backend-protocol: GRPC
+spec:
+  tls:
+    - hosts:
+      - simple-grpc.127.0.0.1.xip.io
+      secretName: xip-tls
+  rules:
+  - host: simple-grpc.127.0.0.1.xip.io
+    http:
+      paths:
+      - path: /
+        backend:
+          serviceName: simple-grpc
+          servicePort: 50051
+
+---
+kind: Pod
+apiVersion: v1
+metadata:
+  name: simple-grpc
+  labels:
+    app: simple-grpc
+spec:
+  containers:
+  - name: simple-grpc
+    image: dhf0820/simple_grpc:200725.4
+
+---
+kind: Service
+apiVersion: v1
+metadata:
+  name: simple-grpc
+spec:
+  selector:
+    app: simple-grpc
+  ports:
+  - name: plain
+    port: 50051

internal/ingress/controller/controller.go

@@ -154,15 +154,15 @@ func (n *NGINXController) syncIngress(interface{}) error {
 			n.metricCollector.IncReloadErrorCount()
 			n.metricCollector.ConfigSuccess(hash, false)
 			klog.Errorf("Unexpected failure reloading the backend:\n%v", err)
-			n.recorder.Eventf(k8s.IngressNGINXPod, apiv1.EventTypeWarning, "RELOAD", fmt.Sprintf("Error reloading NGINX: %v", err))
+			n.recorder.Eventf(k8s.IngressPodDetails, apiv1.EventTypeWarning, "RELOAD", fmt.Sprintf("Error reloading NGINX: %v", err))
 			return err
 		}
 
 		klog.InfoS("Backend successfully reloaded")
 		n.metricCollector.ConfigSuccess(hash, true)
 		n.metricCollector.IncReloadCount()
 
-		n.recorder.Eventf(k8s.IngressNGINXPod, apiv1.EventTypeNormal, "RELOAD", "NGINX reload triggered due to a change in configuration")
+		n.recorder.Eventf(k8s.IngressPodDetails, apiv1.EventTypeNormal, "RELOAD", "NGINX reload triggered due to a change in configuration")
 	}
 
 	isFirstSync := n.runningConfig.Equal(&ingress.Configuration{})

internal/ingress/controller/controller_test.go

@@ -1671,7 +1671,7 @@ func newNGINXController(t *testing.T) *NGINXController {
 		t.Fatalf("error creating the configuration map: %v", err)
 	}
 
-	k8s.IngressNGINXPod = &v1.Pod{
+	k8s.IngressPodDetails = &k8s.PodInfo{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "testpod",
 			Namespace: ns,
@@ -1729,7 +1729,7 @@ func newDynamicNginxController(t *testing.T, setConfigMap func(string) *v1.Confi
 		t.Fatalf("error creating the configuration map: %v", err)
 	}
 
-	k8s.IngressNGINXPod = &v1.Pod{
+	k8s.IngressPodDetails = &k8s.PodInfo{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "testpod",
 			Namespace: ns,

internal/ingress/controller/status.go

@@ -93,23 +93,18 @@ func setupLeaderElection(config *leaderElectionConfig) {
 		Host:      hostname,
 	})
 
-	ingressPod, err := k8s.GetPodDetails()
-	if err != nil {
-		klog.Fatalf("unexpected error starting leader election: %v", err)
-	}
-
 	lock := resourcelock.ConfigMapLock{
-		ConfigMapMeta: metav1.ObjectMeta{Namespace: ingressPod.Namespace, Name: config.ElectionID},
+		ConfigMapMeta: metav1.ObjectMeta{Namespace: k8s.IngressPodDetails.Namespace, Name: config.ElectionID},
 		Client:        config.Client.CoreV1(),
 		LockConfig: resourcelock.ResourceLockConfig{
-			Identity:      ingressPod.Name,
+			Identity:      k8s.IngressPodDetails.Name,
 			EventRecorder: recorder,
 		},
 	}
 
 	ttl := 30 * time.Second
 
-	elector, err = leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
+	elector, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
 		Lock:          &lock,
 		LeaseDuration: ttl,
 		RenewDeadline: ttl / 2,

internal/ingress/status/status.go

@@ -172,14 +172,9 @@ func (s *statusSync) runningAddresses() ([]string, error) {
 		return statusAddressFromService(s.PublishService, s.Client)
 	}
 
-	ingressPod, err := k8s.GetPodDetails()
-	if err != nil {
-		return []string{}, err
-	}
-
 	// get information about all the pods running the ingress controller
-	pods, err := s.Client.CoreV1().Pods(ingressPod.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(ingressPod.Labels).String(),
+	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
+		LabelSelector: labels.SelectorFromSet(k8s.IngressPodDetails.Labels).String(),
 	})
 	if err != nil {
 		return nil, err
@@ -216,13 +211,8 @@ func (s *statusSync) runningAddresses() ([]string, error) {
 }
 
 func (s *statusSync) isRunningMultiplePods() bool {
-	ingressPod, err := k8s.GetPodDetails()
-	if err != nil {
-		return false
-	}
-
-	pods, err := s.Client.CoreV1().Pods(ingressPod.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(ingressPod.Labels).String(),
+	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
+		LabelSelector: labels.SelectorFromSet(k8s.IngressPodDetails.Labels).String(),
 	})
 	if err != nil {
 		return false

internal/ingress/status/status_test.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	apiv1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	testclient "k8s.io/client-go/kubernetes/fake"
@@ -297,7 +296,7 @@ func TestStatusActions(t *testing.T) {
 		UpdateStatusOnShutdown: true,
 	}
 
-	k8s.IngressNGINXPod = &v1.Pod{
+	k8s.IngressPodDetails = &k8s.PodInfo{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "foo_base_pod",
 			Namespace: apiv1.NamespaceDefault,

internal/k8s/main.go

@@ -75,16 +75,22 @@ func GetNodeIPOrName(kubeClient clientset.Interface, name string, useInternalIP
 	return defaultOrInternalIP
 }
 
-// IngressNGINXPod hold information about the ingress-nginx pod
-var IngressNGINXPod *apiv1.Pod
+var (
+	// IngressPodDetails hold information about the ingress-nginx pod
+	IngressPodDetails *PodInfo
+
+	selectorLabelKeys = []string{
+		"app.kubernetes.io/component",
+		"app.kubernetes.io/instance",
+		"app.kubernetes.io/name",
+	}
+)
 
 // PodInfo contains runtime information about the pod running the Ingres controller
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 type PodInfo struct {
-	Name      string
-	Namespace string
-	// Labels selectors of the running pod
-	// This is used to search for other Ingress controller pods
-	Labels map[string]string
+	metav1.TypeMeta
+	metav1.ObjectMeta
 }
 
 // GetIngressPod load the ingress-nginx pod
@@ -96,26 +102,29 @@ func GetIngressPod(kubeClient clientset.Interface) error {
 		return fmt.Errorf("unable to get POD information (missing POD_NAME or POD_NAMESPACE environment variable")
 	}
 
-	IngressNGINXPod, _ = kubeClient.CoreV1().Pods(podNs).Get(context.TODO(), podName, metav1.GetOptions{})
-	if IngressNGINXPod == nil {
-		return fmt.Errorf("unable to get POD information")
+	pod, err := kubeClient.CoreV1().Pods(podNs).Get(context.TODO(), podName, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("unable to get POD information: %v", err)
 	}
 
-	return nil
-}
+	labels := map[string]string{}
+	for _, key := range selectorLabelKeys {
+		value, ok := pod.GetLabels()[key]
+		if !ok {
+			return fmt.Errorf("label %v is missing. Please check", key)
+		}
+
+		labels[key] = value
+	}
 
-// GetPodDetails returns runtime information about the pod:
-// name, namespace and IP of the node where it is running
-func GetPodDetails() (*PodInfo, error) {
-	if IngressNGINXPod == nil {
-		return nil, fmt.Errorf("no ingress-nginx pod details available")
+	IngressPodDetails = &PodInfo{
+		TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"},
 	}
 
-	return &PodInfo{
-		Name:      IngressNGINXPod.Name,
-		Namespace: IngressNGINXPod.Namespace,
-		Labels:    IngressNGINXPod.GetLabels(),
-	}, nil
+	pod.ObjectMeta.DeepCopyInto(&IngressPodDetails.ObjectMeta)
+	IngressPodDetails.SetLabels(labels)
+
+	return nil
 }
 
 // MetaNamespaceKey knows how to make keys for API objects which implement meta.Interface.

internal/k8s/main_test.go

@@ -214,36 +214,36 @@ func TestGetNodeIP(t *testing.T) {
 	}
 }
 
-func TestGetPodDetails(t *testing.T) {
+func TestGetIngressPod(t *testing.T) {
 	// POD_NAME & POD_NAMESPACE not exist
 	os.Setenv("POD_NAME", "")
 	os.Setenv("POD_NAMESPACE", "")
-	err1 := GetIngressPod(testclient.NewSimpleClientset())
-	if err1 == nil {
+	err := GetIngressPod(testclient.NewSimpleClientset())
+	if err == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD_NAME not exist
 	os.Setenv("POD_NAME", "")
 	os.Setenv("POD_NAMESPACE", apiv1.NamespaceDefault)
-	err2 := GetIngressPod(testclient.NewSimpleClientset())
-	if err2 == nil {
+	err = GetIngressPod(testclient.NewSimpleClientset())
+	if err == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD_NAMESPACE not exist
 	os.Setenv("POD_NAME", "testpod")
 	os.Setenv("POD_NAMESPACE", "")
-	err3 := GetIngressPod(testclient.NewSimpleClientset())
-	if err3 == nil {
+	err = GetIngressPod(testclient.NewSimpleClientset())
+	if err == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD not exist
 	os.Setenv("POD_NAME", "testpod")
 	os.Setenv("POD_NAMESPACE", apiv1.NamespaceDefault)
-	err4 := GetIngressPod(testclient.NewSimpleClientset())
-	if err4 == nil {
+	err = GetIngressPod(testclient.NewSimpleClientset())
+	if err == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
@@ -254,8 +254,11 @@ func TestGetPodDetails(t *testing.T) {
 				Name:      "testpod",
 				Namespace: apiv1.NamespaceDefault,
 				Labels: map[string]string{
-					"first":  "first_label",
-					"second": "second_label",
+					"first":                       "first_label",
+					"second":                      "second_label",
+					"app.kubernetes.io/component": "controller",
+					"app.kubernetes.io/instance":  "ingress-nginx",
+					"app.kubernetes.io/name":      "ingress-nginx",
 				},
 			},
 		}}},
@@ -273,9 +276,9 @@ func TestGetPodDetails(t *testing.T) {
 			},
 		}}})
 
-	err5 := GetIngressPod(fkClient)
-	if err5 != nil {
-		t.Errorf("expected a PodInfo but returned error")
+	err = GetIngressPod(fkClient)
+	if err != nil {
+		t.Errorf("expected a PodInfo but returned error: %v", err)
 		return
 	}
 }