From c2c4dc9b584cc505ed3ec3e20900ac69fbf7b931 Mon Sep 17 00:00:00 2001 From: Daniel Tomcej Date: Mon, 19 Jul 2021 12:06:07 -0600 Subject: [PATCH 1/2] Don't remove ingress config on API call failure --- pkg/provider/kubernetes/ingress/client.go | 24 ++++++++++--------- .../kubernetes/ingress/client_mock_test.go | 4 ++-- .../kubernetes/ingress/client_test.go | 4 +++- pkg/provider/kubernetes/ingress/kubernetes.go | 6 +---- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/provider/kubernetes/ingress/client.go b/pkg/provider/kubernetes/ingress/client.go index a37fc1d4d..dd62e6131 100644 --- a/pkg/provider/kubernetes/ingress/client.go +++ b/pkg/provider/kubernetes/ingress/client.go @@ -60,7 +60,7 @@ type Client interface { GetSecret(namespace, name string) (*corev1.Secret, bool, error) GetEndpoints(namespace, name string) (*corev1.Endpoints, bool, error) UpdateIngressStatus(ing *networkingv1beta1.Ingress, ingStatus []corev1.LoadBalancerIngress) error - GetServerVersion() (*version.Version, error) + GetServerVersion() *version.Version } type clientWrapper struct { @@ -72,6 +72,7 @@ type clientWrapper struct { ingressLabelSelector string isNamespaceAll bool watchedNamespaces []string + serverVersion *version.Version } // newInClusterClient returns a new Provider client that is expected to run @@ -208,12 +209,18 @@ func (c *clientWrapper) WatchAll(namespaces []string, stopCh <-chan struct{}) (< } } - serverVersion, err := c.GetServerVersion() + // Get and store the serverVersion for future use. + serverVersionInfo, err := c.clientset.Discovery().ServerVersion() if err != nil { - log.WithoutContext().Errorf("Failed to get server version: %v", err) - return eventCh, nil + return eventCh, fmt.Errorf("could not retrieve server version: %w", err) } + serverVersion, err := version.NewVersion(serverVersionInfo.GitVersion) + if err != nil { + return eventCh, fmt.Errorf("could not parse server version: %w", err) + } + + c.serverVersion = serverVersion if supportsIngressClass(serverVersion) { c.clusterFactory = informers.NewSharedInformerFactoryWithOptions(c.clientset, resyncPeriod) c.clusterFactory.Networking().V1beta1().IngressClasses().Informer().AddEventHandler(eventHandler) @@ -426,13 +433,8 @@ func (c *clientWrapper) lookupNamespace(ns string) string { } // GetServerVersion returns the cluster server version, or an error. -func (c *clientWrapper) GetServerVersion() (*version.Version, error) { - serverVersion, err := c.clientset.Discovery().ServerVersion() - if err != nil { - return nil, fmt.Errorf("could not retrieve server version: %w", err) - } - - return version.NewVersion(serverVersion.GitVersion) +func (c *clientWrapper) GetServerVersion() *version.Version { + return c.serverVersion } // eventHandlerFunc will pass the obj on to the events channel or drop it. diff --git a/pkg/provider/kubernetes/ingress/client_mock_test.go b/pkg/provider/kubernetes/ingress/client_mock_test.go index 94a282990..41614e396 100644 --- a/pkg/provider/kubernetes/ingress/client_mock_test.go +++ b/pkg/provider/kubernetes/ingress/client_mock_test.go @@ -73,8 +73,8 @@ func (c clientMock) GetIngresses() []*networkingv1beta1.Ingress { return c.ingresses } -func (c clientMock) GetServerVersion() (*version.Version, error) { - return c.serverVersion, nil +func (c clientMock) GetServerVersion() *version.Version { + return c.serverVersion } func (c clientMock) GetService(namespace, name string) (*corev1.Service, bool, error) { diff --git a/pkg/provider/kubernetes/ingress/client_test.go b/pkg/provider/kubernetes/ingress/client_test.go index d65484219..0c8ebc299 100644 --- a/pkg/provider/kubernetes/ingress/client_test.go +++ b/pkg/provider/kubernetes/ingress/client_test.go @@ -1,6 +1,7 @@ package ingress import ( + "errors" "fmt" "testing" "time" @@ -154,7 +155,8 @@ func TestClientIgnoresHelmOwnedSecrets(t *testing.T) { stopCh := make(chan struct{}) eventCh, err := client.WatchAll(nil, stopCh) - require.NoError(t, err) + // Fake clientset always returns this exact serverVersion that fails our validation. + require.Error(t, errors.New(`could not parse server version: Malformed version: v0.0.0-master+$Format:%h$`), err) select { case event := <-eventCh: diff --git a/pkg/provider/kubernetes/ingress/kubernetes.go b/pkg/provider/kubernetes/ingress/kubernetes.go index 3a4e03d0b..8fe7e3397 100644 --- a/pkg/provider/kubernetes/ingress/kubernetes.go +++ b/pkg/provider/kubernetes/ingress/kubernetes.go @@ -189,11 +189,7 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl TCP: &dynamic.TCPConfiguration{}, } - serverVersion, err := client.GetServerVersion() - if err != nil { - log.FromContext(ctx).Errorf("Failed to get server version: %v", err) - return conf - } + serverVersion := client.GetServerVersion() var ingressClasses []*networkingv1beta1.IngressClass From bc5e62168301e91f7f25e04c8f3852ca632c4378 Mon Sep 17 00:00:00 2001 From: Romain Date: Tue, 20 Jul 2021 13:02:10 +0200 Subject: [PATCH 2/2] Get Kubernetes server version early --- pkg/provider/kubernetes/ingress/client.go | 25 ++++++++++--------- .../kubernetes/ingress/client_test.go | 15 ++++++++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/pkg/provider/kubernetes/ingress/client.go b/pkg/provider/kubernetes/ingress/client.go index dd62e6131..8cfd1a6e6 100644 --- a/pkg/provider/kubernetes/ingress/client.go +++ b/pkg/provider/kubernetes/ingress/client.go @@ -150,6 +150,19 @@ func newClientImpl(clientset kubernetes.Interface) *clientWrapper { // WatchAll starts namespace-specific controllers for all relevant kinds. func (c *clientWrapper) WatchAll(namespaces []string, stopCh <-chan struct{}) (<-chan interface{}, error) { + // Get and store the serverVersion for future use. + serverVersionInfo, err := c.clientset.Discovery().ServerVersion() + if err != nil { + return nil, fmt.Errorf("could not retrieve server version: %w", err) + } + + serverVersion, err := version.NewVersion(serverVersionInfo.GitVersion) + if err != nil { + return nil, fmt.Errorf("could not parse server version: %w", err) + } + + c.serverVersion = serverVersion + eventCh := make(chan interface{}, 1) eventHandler := &resourceEventHandler{eventCh} @@ -209,18 +222,6 @@ func (c *clientWrapper) WatchAll(namespaces []string, stopCh <-chan struct{}) (< } } - // Get and store the serverVersion for future use. - serverVersionInfo, err := c.clientset.Discovery().ServerVersion() - if err != nil { - return eventCh, fmt.Errorf("could not retrieve server version: %w", err) - } - - serverVersion, err := version.NewVersion(serverVersionInfo.GitVersion) - if err != nil { - return eventCh, fmt.Errorf("could not parse server version: %w", err) - } - - c.serverVersion = serverVersion if supportsIngressClass(serverVersion) { c.clusterFactory = informers.NewSharedInformerFactoryWithOptions(c.clientset, resyncPeriod) c.clusterFactory.Networking().V1beta1().IngressClasses().Informer().AddEventHandler(eventHandler) diff --git a/pkg/provider/kubernetes/ingress/client_test.go b/pkg/provider/kubernetes/ingress/client_test.go index 0c8ebc299..28d0431ce 100644 --- a/pkg/provider/kubernetes/ingress/client_test.go +++ b/pkg/provider/kubernetes/ingress/client_test.go @@ -1,7 +1,6 @@ package ingress import ( - "errors" "fmt" "testing" "time" @@ -12,6 +11,8 @@ import ( kubeerror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/version" + fakediscovery "k8s.io/client-go/discovery/fake" kubefake "k8s.io/client-go/kubernetes/fake" ) @@ -150,13 +151,21 @@ func TestClientIgnoresHelmOwnedSecrets(t *testing.T) { kubeClient := kubefake.NewSimpleClientset(helmSecret, secret) + fakeDiscovery, ok := kubeClient.Discovery().(*fakediscovery.FakeDiscovery) + if !ok { + t.Fatalf("couldn't convert Discovery() to *FakeDiscovery") + } + + fakeDiscovery.FakedServerVersion = &version.Info{ + GitVersion: "1.17.0", + } + client := newClientImpl(kubeClient) stopCh := make(chan struct{}) eventCh, err := client.WatchAll(nil, stopCh) - // Fake clientset always returns this exact serverVersion that fails our validation. - require.Error(t, errors.New(`could not parse server version: Malformed version: v0.0.0-master+$Format:%h$`), err) + require.NoError(t, err) select { case event := <-eventCh: