From aad671cbe87bf39140651db598e4a3e1b59f7901 Mon Sep 17 00:00:00 2001
From: Marcus Rodan <marcus.rodan@sinch.com>
Date: Mon, 1 Feb 2021 17:30:26 +0100
Subject: [PATCH] Added checks for API server errors

---
 pkg/canary/config_tracker.go          |  7 +++----
 pkg/canary/config_tracker_test.go     | 29 +++++++++++++++++++++++++++
 pkg/canary/deployment_fixture_test.go |  7 ++++++-
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/pkg/canary/config_tracker.go b/pkg/canary/config_tracker.go
index 23e80feb1..4bbc07656 100644
--- a/pkg/canary/config_tracker.go
+++ b/pkg/canary/config_tracker.go
@@ -208,18 +208,17 @@ func (ct *ConfigTracker) GetTargetConfigs(cd *flaggerv1.Canary) (map[string]Conf
 	for configMapName := range configMapNames {
 		config, err := ct.getRefFromConfigMap(configMapName, cd.Namespace)
 		if err != nil {
-			ct.Logger.Errorf("getRefFromConfigMap failed: %v", err)
-			continue
+			return nil, fmt.Errorf("configmap %s.%s get query error: %w", configMapName, cd.Namespace, err)
 		}
 		if config != nil {
 			res[config.GetName()] = *config
 		}
 	}
+
 	for secretName := range secretNames {
 		secret, err := ct.getRefFromSecret(secretName, cd.Namespace)
 		if err != nil {
-			ct.Logger.Errorf("getRefFromSecret failed: %v", err)
-			continue
+			return nil, fmt.Errorf("secret %s.%s get query error: %w", secretName, cd.Namespace, err)
 		}
 		if secret != nil {
 			res[secret.GetName()] = *secret
diff --git a/pkg/canary/config_tracker_test.go b/pkg/canary/config_tracker_test.go
index 3399db95b..f229a5afb 100644
--- a/pkg/canary/config_tracker_test.go
+++ b/pkg/canary/config_tracker_test.go
@@ -18,12 +18,15 @@ package canary
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	k8sTesting "k8s.io/client-go/testing"
 )
 
 func TestConfigIsDisabled(t *testing.T) {
@@ -303,3 +306,29 @@ func TestConfigTracker_Secrets(t *testing.T) {
 		assert.True(t, originalVolPresent, "Volume for original secret with disabled tracking should be present")
 	})
 }
+
+func TestConfigTracker_HasConfigChanged_ShouldReturnErrorWhenAPIServerIsDown(t *testing.T) {
+	t.Run("secret", func(t *testing.T) {
+		dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
+		mocks, kubeClient := newCustomizableFixture(dc)
+
+		kubeClient.PrependReactor("get", "secrets", func(action k8sTesting.Action) (bool, runtime.Object, error) {
+			return true, nil, errors.New("server error")
+		})
+
+		_, err := mocks.controller.configTracker.HasConfigChanged(mocks.canary)
+		assert.Error(t, err)
+	})
+
+	t.Run("configmap", func(t *testing.T) {
+		dc := deploymentConfigs{name: "podinfo", label: "name", labelValue: "podinfo"}
+		mocks, kubeClient := newCustomizableFixture(dc)
+
+		kubeClient.PrependReactor("get", "configmaps", func(action k8sTesting.Action) (bool, runtime.Object, error) {
+			return true, nil, errors.New("server error")
+		})
+
+		_, err := mocks.controller.configTracker.HasConfigChanged(mocks.canary)
+		assert.Error(t, err)
+	})
+}
diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go
index 05bc19590..7877bd1ba 100644
--- a/pkg/canary/deployment_fixture_test.go
+++ b/pkg/canary/deployment_fixture_test.go
@@ -78,6 +78,11 @@ func (d deploymentControllerFixture) initializeCanary(t *testing.T) {
 }
 
 func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture {
+	fixture, _ := newCustomizableFixture(dc)
+	return fixture
+}
+
+func newCustomizableFixture(dc deploymentConfigs) (deploymentControllerFixture, *fake.Clientset) {
 	// init canary
 	cc := canaryConfigs{targetName: dc.name}
 	canary := newDeploymentControllerTestCanary(cc)
@@ -121,7 +126,7 @@ func newDeploymentFixture(dc deploymentConfigs) deploymentControllerFixture {
 		logger:        logger,
 		flaggerClient: flaggerClient,
 		kubeClient:    kubeClient,
-	}
+	}, kubeClient
 }
 
 func newDeploymentControllerTestConfigMap() *corev1.ConfigMap {