diff --git a/cmd/dbg/main.go b/cmd/dbg/main.go index aacc2b54d..7e652abcc 100644 --- a/cmd/dbg/main.go +++ b/cmd/dbg/main.go @@ -211,18 +211,21 @@ func certGet(host string) { } func general() { - statusCode, body, requestErr := nginx.NewGetStatusRequest(generalPath) - if requestErr != nil { - fmt.Println(requestErr) - return - } - if statusCode != 200 { - fmt.Printf("Nginx returned code %v\n", statusCode) - return - } + //TODO: refactor to obtain ingress-nginx pod count from the api server + /* + statusCode, body, requestErr := nginx.NewGetStatusRequest(generalPath) + if requestErr != nil { + fmt.Println(requestErr) + return + } + if statusCode != 200 { + fmt.Printf("Nginx returned code %v\n", statusCode) + return + } + */ var prettyBuffer bytes.Buffer - indentErr := json.Indent(&prettyBuffer, body, "", " ") + indentErr := json.Indent(&prettyBuffer, []byte("{}"), "", " ") if indentErr != nil { fmt.Println(indentErr) return diff --git a/docs/kubectl-plugin.md b/docs/kubectl-plugin.md index 5350597d3..801a187b7 100644 --- a/docs/kubectl-plugin.md +++ b/docs/kubectl-plugin.md @@ -212,17 +212,6 @@ owasp-modsecurity-crs template ``` -### general - -`kubectl ingress-nginx general` dumps miscellaneous controller state as a JSON object. Currently it just shows the number of controller pods known to a particular controller pod. - -```console -$ kubectl ingress-nginx general -n ingress-nginx -{ - "controllerPodsCount": 1 -} -``` - ### info Shows the internal and external IP/CNAMES for an `ingress-nginx` service. diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 22d36a4bf..a6ffe4e1c 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -467,7 +467,6 @@ func (n *NGINXController) getConfiguration(ingresses []*ingress.Ingress) (sets.S UDPEndpoints: n.getStreamServices(n.cfg.UDPConfigMapName, apiv1.ProtocolUDP), PassthroughBackends: passUpstreams, BackendConfigChecksum: n.store.GetBackendConfiguration().Checksum, - ControllerPodsCount: n.store.GetRunningControllerPodsCount(), } } diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 467f38c05..847096ae6 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -86,10 +86,6 @@ func (fis fakeIngressStore) FilterIngresses(ingresses []*ingress.Ingress, filter return ingresses } -func (fakeIngressStore) GetRunningControllerPodsCount() int { - return 0 -} - func (fakeIngressStore) GetLocalSSLCert(name string) (*ingress.SSLCert, error) { return nil, fmt.Errorf("test error") } diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 586132a9a..194278030 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -828,9 +828,6 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati clearL4serviceEndpoints(©OfRunningConfig) clearL4serviceEndpoints(©OfPcfg) - copyOfRunningConfig.ControllerPodsCount = 0 - copyOfPcfg.ControllerPodsCount = 0 - clearCertificates(©OfRunningConfig) clearCertificates(©OfPcfg) @@ -856,18 +853,6 @@ func (n *NGINXController) configureDynamically(pcfg *ingress.Configuration) erro } } - if n.runningConfig.ControllerPodsCount != pcfg.ControllerPodsCount { - statusCode, _, err := nginx.NewPostStatusRequest("/configuration/general", "application/json", ingress.GeneralConfig{ - ControllerPodsCount: pcfg.ControllerPodsCount, - }) - if err != nil { - return err - } - if statusCode != http.StatusCreated { - return fmt.Errorf("unexpected error code: %d", statusCode) - } - } - serversChanged := !reflect.DeepEqual(n.runningConfig.Servers, pcfg.Servers) if serversChanged { err := configureCertificates(pcfg.Servers) diff --git a/internal/ingress/controller/nginx_test.go b/internal/ingress/controller/nginx_test.go index 88a75791a..cf6463f55 100644 --- a/internal/ingress/controller/nginx_test.go +++ b/internal/ingress/controller/nginx_test.go @@ -199,14 +199,11 @@ func TestConfigureDynamically(t *testing.T) { } case "/configuration/general": { - if !strings.Contains(body, "controllerPodsCount") { - t.Errorf("controllerPodsCount should be present in JSON content: %v", body) - } } case "/configuration/servers": { if !strings.Contains(body, `{"certificates":{},"servers":{"myapp.fake":"-1"}}`) { - t.Errorf("controllerPodsCount should be present in JSON content: %v", body) + t.Errorf("should be present in JSON content: %v", body) } } default: @@ -249,9 +246,8 @@ func TestConfigureDynamically(t *testing.T) { }} commonConfig := &ingress.Configuration{ - Backends: backends, - Servers: servers, - ControllerPodsCount: 2, + Backends: backends, + Servers: servers, } n := &NGINXController{ @@ -266,11 +262,6 @@ func TestConfigureDynamically(t *testing.T) { if commonConfig.Backends[0].Endpoints[0].Target != target { t.Errorf("unexpected change in the configuration object after configureDynamically invocation") } - for endpoint, count := range endpointStats { - if count != 1 { - t.Errorf("Expected %v to receive %d requests but received %d.", endpoint, 1, count) - } - } resetEndpointStats() n.runningConfig.Backends = backends @@ -283,8 +274,6 @@ func TestConfigureDynamically(t *testing.T) { if count != 0 { t.Errorf("Expected %v to receive %d requests but received %d.", endpoint, 0, count) } - } else if count != 1 { - t.Errorf("Expected %v to receive %d requests but received %d.", endpoint, 1, count) } } @@ -300,12 +289,8 @@ func TestConfigureDynamically(t *testing.T) { if count := endpointStats["/configuration/servers"]; count != 0 { t.Errorf("Expected %v to receive %d requests but received %d.", "/configuration/servers", 0, count) } - if count := endpointStats["/configuration/general"]; count != 1 { - t.Errorf("Expected %v to receive %d requests but received %d.", "/configuration/general", 0, count) - } resetEndpointStats() - n.runningConfig.ControllerPodsCount = commonConfig.ControllerPodsCount err = n.configureDynamically(commonConfig) if err != nil { t.Errorf("unexpected error posting dynamic configuration: %v", err) diff --git a/internal/ingress/controller/store/pod.go b/internal/ingress/controller/store/pod.go deleted file mode 100644 index 1a437d769..000000000 --- a/internal/ingress/controller/store/pod.go +++ /dev/null @@ -1,26 +0,0 @@ -/* -Copyright 2018 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. -*/ - -package store - -import ( - "k8s.io/client-go/tools/cache" -) - -// PodLister makes a Store that lists Pods. -type PodLister struct { - cache.Store -} diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index a64a09ed5..f285783e0 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -30,11 +30,9 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -81,9 +79,6 @@ type Storer interface { // ListIngresses returns a list of all Ingresses in the store. ListIngresses() []*ingress.Ingress - // GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods. - GetRunningControllerPodsCount() int - // GetLocalSSLCert returns the local copy of a SSLCert GetLocalSSLCert(name string) (*ingress.SSLCert, error) @@ -129,7 +124,6 @@ type Informer struct { Service cache.SharedIndexInformer Secret cache.SharedIndexInformer ConfigMap cache.SharedIndexInformer - Pod cache.SharedIndexInformer } // Lister contains object listers (stores). @@ -140,7 +134,6 @@ type Lister struct { Secret SecretLister ConfigMap ConfigMapLister IngressWithAnnotation IngressWithAnnotationsLister - Pod PodLister } // NotExistsError is returned when an object does not exist in a local store. @@ -157,7 +150,6 @@ func (i *Informer) Run(stopCh chan struct{}) { go i.Endpoint.Run(stopCh) go i.Service.Run(stopCh) go i.ConfigMap.Run(stopCh) - go i.Pod.Run(stopCh) // wait for all involved caches to be synced before processing items // from the queue @@ -166,7 +158,6 @@ func (i *Informer) Run(stopCh chan struct{}) { i.Service.HasSynced, i.Secret.HasSynced, i.ConfigMap.HasSynced, - i.Pod.HasSynced, ) { runtime.HandleError(fmt.Errorf("timed out waiting for caches to sync")) } @@ -290,25 +281,6 @@ func New( store.informers.Service = infFactory.Core().V1().Services().Informer() store.listers.Service.Store = store.informers.Service.GetStore() - ingressPodInfo, _ := k8s.GetPodDetails() - labelSelector := labels.SelectorFromSet(ingressPodInfo.Labels) - store.informers.Pod = cache.NewSharedIndexInformer( - &cache.ListWatch{ - ListFunc: func(options metav1.ListOptions) (k8sruntime.Object, error) { - options.LabelSelector = labelSelector.String() - return client.CoreV1().Pods(ingressPodInfo.Namespace).List(context.TODO(), options) - }, - WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { - options.LabelSelector = labelSelector.String() - return client.CoreV1().Pods(ingressPodInfo.Namespace).Watch(context.TODO(), options) - }, - }, - &corev1.Pod{}, - resyncPeriod, - cache.Indexers{}, - ) - store.listers.Pod.Store = store.informers.Pod.GetStore() - ingDeleteHandler := func(obj interface{}) { ing, ok := toIngress(obj) if !ok { @@ -592,34 +564,6 @@ func New( }, } - podEventHandler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { - updateCh.In() <- Event{ - Type: CreateEvent, - Obj: obj, - } - }, - UpdateFunc: func(old, cur interface{}) { - oldPod := old.(*corev1.Pod) - curPod := cur.(*corev1.Pod) - - if oldPod.Status.Phase == curPod.Status.Phase { - return - } - - updateCh.In() <- Event{ - Type: UpdateEvent, - Obj: cur, - } - }, - DeleteFunc: func(obj interface{}) { - updateCh.In() <- Event{ - Type: DeleteEvent, - Obj: obj, - } - }, - } - serviceHandler := cache.ResourceEventHandlerFuncs{ UpdateFunc: func(old, cur interface{}) { oldSvc := old.(*corev1.Service) @@ -641,7 +585,6 @@ func New( store.informers.Secret.AddEventHandler(secrEventHandler) store.informers.ConfigMap.AddEventHandler(cmEventHandler) store.informers.Service.AddEventHandler(serviceHandler) - store.informers.Pod.AddEventHandler(podEventHandler) // do not wait for informers to read the configmap configuration ns, name, _ := k8s.ParseNameNS(configmap) @@ -930,23 +873,6 @@ func (s *k8sStore) Run(stopCh chan struct{}) { s.informers.Run(stopCh) } -// GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods -func (s k8sStore) GetRunningControllerPodsCount() int { - count := 0 - - for _, i := range s.listers.Pod.List() { - pod := i.(*corev1.Pod) - - if pod.Status.Phase != corev1.PodRunning { - continue - } - - count++ - } - - return count -} - var runtimeScheme = k8sruntime.NewScheme() func init() { diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index e2ef62684..60c231df8 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -47,16 +47,6 @@ import ( func TestStore(t *testing.T) { k8s.IsNetworkingIngressAvailable = true - k8s.IngressNGINXPod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testpod", - Namespace: v1.NamespaceDefault, - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - } - //TODO: move env definition to docker image? os.Setenv("KUBEBUILDER_ASSETS", "/usr/local/bin") @@ -773,22 +763,11 @@ func deleteIngress(ingress *networking.Ingress, clientSet kubernetes.Interface, // newStore creates a new mock object store for tests which do not require the // use of Informers. func newStore(t *testing.T) *k8sStore { - k8s.IngressNGINXPod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-1", - Namespace: v1.NamespaceDefault, - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - } - return &k8sStore{ listers: &Lister{ // add more listers if needed Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, IngressWithAnnotation: IngressWithAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)}, - Pod: PodLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, }, sslStore: NewSSLCertTracker(), updateCh: channels.NewRingChannel(10), @@ -1003,67 +982,3 @@ func TestWriteSSLSessionTicketKey(t *testing.T) { } } } - -func TestGetRunningControllerPodsCount(t *testing.T) { - os.Setenv("POD_NAMESPACE", "testns") - os.Setenv("POD_NAME", "ingress-1") - - k8s.IngressNGINXPod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-1", - Namespace: "testns", - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - } - - s := newStore(t) - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-1", - Namespace: "testns", - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - Status: v1.PodStatus{ - Phase: v1.PodRunning, - }, - } - s.listers.Pod.Add(pod) - - pod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-2", - Namespace: "testns", - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - Status: v1.PodStatus{ - Phase: v1.PodRunning, - }, - } - s.listers.Pod.Add(pod) - - pod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-3", - Namespace: "testns", - Labels: map[string]string{ - "pod-template-hash": "1234", - }, - }, - Status: v1.PodStatus{ - Phase: v1.PodFailed, - }, - } - s.listers.Pod.Add(pod) - - podsCount := s.GetRunningControllerPodsCount() - if podsCount != 2 { - t.Errorf("Expected 1 controller Pods but got %v", s) - } -} diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index 700668eaf..16fff3122 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -177,7 +177,7 @@ func (s *statusSync) runningAddresses() ([]string, error) { ingressPod, err := k8s.GetPodDetails() if err != nil { - return nil, err + return []string{}, err } // get information about all the pods running the ingress controller diff --git a/internal/ingress/types.go b/internal/ingress/types.go index c922f2cc3..35b574f8c 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -73,9 +73,6 @@ type Configuration struct { // ConfigurationChecksum contains the particular checksum of a Configuration object ConfigurationChecksum string `json:"configurationChecksum,omitempty"` - - // ControllerPodsCount contains the list of running ingress controller Pod(s) - ControllerPodsCount int `json:"controllerPodsCount,omitempty"` } // Backend describes one or more remote server/s (endpoints) associated with a service @@ -390,5 +387,4 @@ type Ingress struct { // GeneralConfig holds the definition of lua general configuration data type GeneralConfig struct { - ControllerPodsCount int `json:"controllerPodsCount"` } diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 358bdd248..c179426b4 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -76,10 +76,6 @@ func (c1 *Configuration) Equal(c2 *Configuration) bool { return false } - if c1.ControllerPodsCount != c2.ControllerPodsCount { - return false - } - return true } diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index 6cdda619e..f97d87e69 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -29,7 +29,6 @@ import ( networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/nginx" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -198,22 +197,6 @@ var _ = framework.IngressNginxDescribe("[Lua] dynamic configuration", func() { assert.Equal(ginkgo.GinkgoT(), nginxConfig, newNginxConfig) }) }) - - ginkgo.It("sets controllerPodsCount in Lua general configuration", func() { - // https://github.com/curl/curl/issues/936 - curlCmd := fmt.Sprintf("curl --fail --silent http://localhost:%v/configuration/general", nginx.StatusPort) - - output, err := f.ExecIngressPod(curlCmd) - assert.Nil(ginkgo.GinkgoT(), err) - assert.Equal(ginkgo.GinkgoT(), output, `{"controllerPodsCount":1}`) - - err = f.UpdateIngressControllerDeployment(nil) - assert.Nil(ginkgo.GinkgoT(), err) - - output, err = f.ExecIngressPod(curlCmd) - assert.Nil(ginkgo.GinkgoT(), err) - assert.Equal(ginkgo.GinkgoT(), output, `{"controllerPodsCount":3}`) - }) }) func ensureIngress(f *framework.Framework, host string, deploymentName string) *networking.Ingress {