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,

internal/ingress/controller/controller.go

@@ -154,7 +154,7 @@ 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
 		}
 
@@ -162,7 +162,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
 		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{
+	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(ingressPod.Labels).String(),
+		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()
+	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
-	if err != nil {
+		LabelSelector: labels.SelectorFromSet(k8s.IngressPodDetails.Labels).String(),
-		return false
-	}
-
-	pods, err := s.Client.CoreV1().Pods(ingressPod.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(ingressPod.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 (
-var IngressNGINXPod *apiv1.Pod
+	// 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
+	metav1.TypeMeta
-	Namespace string
+	metav1.ObjectMeta
-	// Labels selectors of the running pod
-	// This is used to search for other Ingress controller pods
-	Labels map[string]string
 }
 
 // GetIngressPod load the ingress-nginx pod
@@ -96,28 +102,31 @@ 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{})
+	pod, err := kubeClient.CoreV1().Pods(podNs).Get(context.TODO(), podName, metav1.GetOptions{})
-	if IngressNGINXPod == nil {
+	if err != nil {
-		return fmt.Errorf("unable to get POD information")
+		return fmt.Errorf("unable to get POD information: %v", err)
 	}
 
+	labels := map[string]string{}
+	for _, key := range selectorLabelKeys {
+		value, ok := pod.GetLabels()[key]
+		if !ok {
+			return fmt.Errorf("label %v is missing. Please do not remove", key)
+		}
+
+		labels[key] = value
+	}
+
+	IngressPodDetails = &PodInfo{
+		TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"},
+	}
+
+	pod.ObjectMeta.DeepCopyInto(&IngressPodDetails.ObjectMeta)
+	IngressPodDetails.SetLabels(labels)
+
 	return nil
 }
 
-// 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")
-	}
-
-	return &PodInfo{
-		Name:      IngressNGINXPod.Name,
-		Namespace: IngressNGINXPod.Namespace,
-		Labels:    IngressNGINXPod.GetLabels(),
-	}, nil
-}
-
 // MetaNamespaceKey knows how to make keys for API objects which implement meta.Interface.
 func MetaNamespaceKey(obj interface{}) string {
 	key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)

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())
+	err := GetIngressPod(testclient.NewSimpleClientset())
-	if err1 == nil {
+	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())
+	err = GetIngressPod(testclient.NewSimpleClientset())
-	if err2 == nil {
+	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())
+	err = GetIngressPod(testclient.NewSimpleClientset())
-	if err3 == nil {
+	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())
+	err = GetIngressPod(testclient.NewSimpleClientset())
-	if err4 == nil {
+	if err == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
@@ -256,6 +256,9 @@ func TestGetPodDetails(t *testing.T) {
 				Labels: map[string]string{
 					"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)
+	err = GetIngressPod(fkClient)
-	if err5 != nil {
+	if err != nil {
-		t.Errorf("expected a PodInfo but returned error")
+		t.Errorf("expected a PodInfo but returned error: %v", err)
 		return
 	}
 }

internal/k8s/zz_generated.deepcopy.go

@@ -0,0 +1,36 @@
+// +build !ignore_autogenerated
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Code generated by deepcopy-gen. DO NOT EDIT.
+
+package k8s
+
+import "k8s.io/apimachinery/pkg/runtime"
+
+func (in *PodInfo) DeepCopyInto(out *PodInfo) {
+	out.TypeMeta = in.TypeMeta
+	out.ObjectMeta = in.ObjectMeta
+}
+
+// DeepCopyObject returns a generically typed copy of an object
+func (in *PodInfo) DeepCopyObject() runtime.Object {
+	out := PodInfo{}
+	in.DeepCopyInto(&out)
+
+	return &out
+}