From 67436aee925cdbbd7ad28e1ebbc34599f34833fa Mon Sep 17 00:00:00 2001 From: Tamas Molnar Date: Fri, 23 Feb 2018 11:22:59 +0200 Subject: [PATCH 1/6] use circleci 2 --- .circleci/config.yml | 58 ++++++++++++++++++++++++++++++++++++++++++++ circle.yml | 30 ----------------------- 2 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 .circleci/config.yml delete mode 100644 circle.yml diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 0000000..60f8de4 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,58 @@ +version: 2 +jobs: + build: + working_directory: /go/src/github.com/Financial-Times/upp-aggregate-healthcheck + docker: + - image: golang:1.8.3 + environment: + GOPATH: /go + CIRCLE_TEST_REPORTS: /tmp/test-results + CIRCLE_COVERAGE_REPORT: /tmp/coverage-results + steps: + - checkout + - run: + name: External Dependencies + command: | + go get -u github.com/mattn/goveralls + go get -u github.com/jstemmer/go-junit-report + go get -u github.com/kardianos/govendor + go get -u github.com/haya14busa/goverage + - run: + name: Test Results + command: | + mkdir -p ${CIRCLE_TEST_REPORTS} + mkdir -p ${CIRCLE_COVERAGE_REPORT} + - run: + name: Govendor Sync + command: govendor sync -v + - run: + name: Go Build + command: go build -v + - run: + name: Run Tests + command: | + govendor test -race -v +local | /go/bin/go-junit-report > ${CIRCLE_TEST_REPORTS}/main.xml + goverage -covermode=atomic -race -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out ./... + - run: + name: Upload Coverage + command: /go/bin/goveralls -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out -service=circle-ci -repotoken=$COVERALLS_TOKEN + - store_test_results: + path: /tmp/test-results + dockerfile: + working_directory: /upp-aggregate-healthcheck + docker: + - image: docker:1.12.6-git + steps: + - checkout + - setup_docker_engine + - run: + name: Build Dockerfile + command: docker build . +workflows: + version: 2 + test-and-build-docker: + jobs: + - build + - dockerfile: + requires: + - build diff --git a/circle.yml b/circle.yml deleted file mode 100644 index 91cc0a3..0000000 --- a/circle.yml +++ /dev/null @@ -1,30 +0,0 @@ -machine: - environment: - PROJECT_GOPATH: "${HOME}/.go_project" - PROJECT_PARENT_PATH: "${PROJECT_GOPATH}/src/github.com/${CIRCLE_PROJECT_USERNAME}" - PROJECT_PATH: "${PROJECT_PARENT_PATH}/${CIRCLE_PROJECT_REPONAME}" - GOPATH: "${HOME}/.go_workspace:${PROJECT_GOPATH}" - -checkout: - post: - - mkdir -p "${PROJECT_PARENT_PATH}" - - rsync -avC "${HOME}/${CIRCLE_PROJECT_REPONAME}/" "${PROJECT_PATH}" - -dependencies: - pre: - - go get -u github.com/kardianos/govendor - override: - - cd $PROJECT_PATH && govendor sync - - cd $PROJECT_PATH && go build -v - -test: - pre: - - go get -u github.com/jstemmer/go-junit-report - - go get -u github.com/mattn/goveralls - - cd $PROJECT_PATH && wget https://raw.githubusercontent.com/Financial-Times/cookiecutter-upp-golang/master/coverage.sh && chmod +x coverage.sh - override: - - mkdir -p $CIRCLE_TEST_REPORTS/golang - - cd $PROJECT_PATH && govendor test -race -v +local | go-junit-report > $CIRCLE_TEST_REPORTS/golang/junit.xml - - cd $PROJECT_PATH && ./coverage.sh - post: - - goveralls -coverprofile=$CIRCLE_ARTIFACTS/coverage.out -service=circle-ci -repotoken=$COVERALLS_TOKEN From aa2318e17fed1a875d97bd06b9480b63bda84f83 Mon Sep 17 00:00:00 2001 From: Tamas Molnar Date: Mon, 26 Feb 2018 16:17:50 +0200 Subject: [PATCH 2/6] Take resiliency into consideration when computing service severity --- checkerService.go | 8 +-- controller_test.go | 142 +++++++++++++++++++++++++++++++----------- service.go | 2 +- service_test.go | 4 +- severityController.go | 19 +++++- 5 files changed, 131 insertions(+), 44 deletions(-) diff --git a/checkerService.go b/checkerService.go index 77703dd..609a88d 100644 --- a/checkerService.go +++ b/checkerService.go @@ -72,23 +72,23 @@ func (hs *k8sHealthcheckService) checkPodHealth(pod pod, appPort int32) error { return nil } -func (hs *k8sHealthcheckService) getIndividualPodSeverity(pod pod, appPort int32) (uint8, error) { +func (hs *k8sHealthcheckService) getIndividualPodSeverity(pod pod, appPort int32) (uint8, bool, error) { health, err := hs.getHealthChecksForPod(pod, appPort) if err != nil { - return defaultSeverity, fmt.Errorf("cannot get severity for pod with name %s: %s", pod.name, err.Error()) + return defaultSeverity, false, fmt.Errorf("cannot get severity for pod with name %s: %s", pod.name, err.Error()) } finalSeverity := uint8(2) for _, check := range health.Checks { if !check.OK { if check.Severity < finalSeverity { - return check.Severity, nil + return check.Severity, true, nil } } } - return finalSeverity, nil + return finalSeverity, false, nil } func (hs *k8sHealthcheckService) getHealthChecksForPod(pod pod, appPort int32) (healthcheckResponse, error) { diff --git a/controller_test.go b/controller_test.go index 9a89d0c..b51150d 100644 --- a/controller_test.go +++ b/controller_test.go @@ -100,39 +100,87 @@ func (m *MockService) getServicesMapByNames(serviceNames []string) map[string]se } func (m *MockService) getPodsForService(serviceName string) ([]pod, error) { - if serviceName == invalidNameForService { + switch serviceName { + case "invalidNameForService": return []pod{}, errors.New("Invalid pod name") + case "resilient-notok-sev1": + return []pod{ + { + name: "notok-pod-1", + ip: "10.2.51.2", + }, + { + name: "notok-pod-1", + ip: "10.2.51.2", + }, + }, nil + case "resilient-notok-sev2": + return []pod{ + { + name: "notok-pod-2", + ip: "10.2.51.2", + }, + { + name: "notok-pod-2", + ip: "10.2.51.2", + }, + }, nil + case "resilient-halfok-sev1": + return []pod{ + { + name: "notok-pod-1", + ip: "10.2.51.2", + }, + { + name: "ok-pod-2", + ip: "10.2.51.2", + }, + }, nil + case "resilient-halfok-sev2": + return []pod{ + { + name: "notok-pod-2", + ip: "10.2.51.2", + }, + { + name: "ok-pod-2", + ip: "10.2.51.2", + }, + }, nil + default: + return []pod{ + { + name: "test-pod-name2-8425234-9hdfg ", + ip: "10.2.51.2", + }, + { + name: "test-pod-name1-8425234-9hdfg ", + ip: "10.2.51.2", + }, + }, nil } - - return []pod{ - { - name: "test-pod-name2-8425234-9hdfg ", - ip: "10.2.51.2", - }, - { - name: "test-pod-name1-8425234-9hdfg ", - ip: "10.2.51.2", - }, - }, nil } func (m *MockService) getPodByName(podName string) (pod, error) { - if podName == nonExistingPodName { - return pod{}, errors.New("Pod not found") - } - - if podName == podWithBrokenService { + switch podName { + case nonExistingPodName: + { + return pod{}, errors.New("Pod not found") + } + case podWithBrokenService: + { + return pod{ + name: "test-pod-name-8425234-9hdfg ", + ip: "10.2.51.2", + serviceName: nonExistingServiceName, + }, nil + } + default: return pod{ - name: "test-pod-name-8425234-9hdfg ", - ip: "10.2.51.2", - serviceName: nonExistingServiceName, + name: "test-pod-name-8425234-9hdfg ", + ip: "10.2.51.2", }, nil } - - return pod{ - name: "test-pod-name-8425234-9hdfg ", - ip: "10.2.51.2", - }, nil } func (m *MockService) checkServiceHealth(service service, deployments map[string]deployment) (string, error) { @@ -143,16 +191,19 @@ func (m *MockService) checkPodHealth(pod, int32) error { return errors.New("Error reading healthcheck response: ") } -func (m *MockService) getIndividualPodSeverity(pod pod, port int32) (uint8, error) { - if pod.name == failingPod { - return 1, errors.New("Test") - } - - if pod.name == podWithCriticalSeverity { - return 1, nil +func (m *MockService) getIndividualPodSeverity(pod pod, port int32) (uint8, bool, error) { + switch pod.name { + case failingPod: + return 1, false, errors.New("Test") + case podWithCriticalSeverity: + return 1, true, nil + case "notok-pod-1": + return 1, true, nil + case "notok-pod-2": + return 2, true, nil + default: + return defaultSeverity, false, nil } - - return defaultSeverity, nil } func (m *MockService) getHealthChecksForPod(pod pod, port int32) (healthcheckResponse, error) { @@ -310,7 +361,28 @@ func TestComputeSeverityForPodWithCriticalSeverity(t *testing.T) { assert.Equal(t, uint8(1), severity) } -func TestGetSeverityForService(t *testing.T) { +func TestGetSeverityForServiceInvalidServiceName(t *testing.T) { + controller := initializeMockController("test", nil) + severity := controller.getSeverityForService(invalidNameForService, 8080) + assert.Equal(t, defaultSeverity, severity) +} + +func TestGetSeverityForResilientService(t *testing.T) { + controller := initializeMockController("test", nil) + severity := controller.getSeverityForService("resilient-notok-sev1", 8080) + assert.Equal(t, uint8(1), severity) + + severity = controller.getSeverityForService("resilient-notok-sev2", 8080) + assert.Equal(t, defaultSeverity, severity) + + severity = controller.getSeverityForService("resilient-halfok-sev1", 8080) + assert.Equal(t, defaultSeverity, severity) + + severity = controller.getSeverityForService("resilient-halfok-sev2", 8080) + assert.Equal(t, defaultSeverity, severity) +} + +func TestGetSeverityForNonResilientService(t *testing.T) { controller := initializeMockController("test", nil) severity := controller.getSeverityForService(invalidNameForService, 8080) assert.Equal(t, defaultSeverity, severity) diff --git a/service.go b/service.go index 07803f9..972b6bd 100644 --- a/service.go +++ b/service.go @@ -32,7 +32,7 @@ type healthcheckService interface { getPodByName(string) (pod, error) checkServiceHealth(service, map[string]deployment) (string, error) checkPodHealth(pod, int32) error - getIndividualPodSeverity(pod, int32) (uint8, error) + getIndividualPodSeverity(pod, int32) (uint8, bool, error) getHealthChecksForPod(pod, int32) (healthcheckResponse, error) addAck(string, string) error removeAck(string) error diff --git a/service_test.go b/service_test.go index 9d0f5e7..6fe023e 100644 --- a/service_test.go +++ b/service_test.go @@ -133,14 +133,14 @@ func TestGetHealthChecksForPodHealthAvailable(t *testing.T) { func TestGetIndividualPodSeverityErrorWhilePodHealthCheck(t *testing.T) { service := initializeMockService(initializeMockHTTPClient(http.StatusInternalServerError, "")) - severity, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) + severity, _, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) assert.NotNil(t, err) assert.Equal(t, defaultSeverity, severity) } func TestGetIndividualPodSeverityValidPodHealth(t *testing.T) { service := initializeMockService(initializeMockHTTPClient(http.StatusOK, validFailingHealthCheckResponseBody)) - severity, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) + severity, _, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) assert.Nil(t, err) assert.Equal(t, validSeverity, severity) } diff --git a/severityController.go b/severityController.go index 70611fc..3a6e038 100644 --- a/severityController.go +++ b/severityController.go @@ -8,7 +8,22 @@ func (c *healthCheckController) getSeverityForService(serviceName string, appPor return defaultSeverity } - return c.computeSeverityByPods(pods, appPort) + finalSeverity := defaultSeverity + for _, pod := range pods { + individualPodSeverity, checkFailed, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) + + if err != nil { + warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) + continue + } + if !checkFailed { + return defaultSeverity + } + if individualPodSeverity < finalSeverity { + finalSeverity = individualPodSeverity + } + } + return finalSeverity } func (c *healthCheckController) getSeverityForPod(podName string, appPort int32) uint8 { @@ -25,7 +40,7 @@ func (c *healthCheckController) getSeverityForPod(podName string, appPort int32) func (c *healthCheckController) computeSeverityByPods(pods []pod, appPort int32) uint8 { finalSeverity := defaultSeverity for _, pod := range pods { - individualPodSeverity, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) + individualPodSeverity, _, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) if err != nil { warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) From 0aaac706612afd19406e3034c318835ec9cc1a4d Mon Sep 17 00:00:00 2001 From: Tamas Molnar Date: Tue, 27 Feb 2018 13:50:20 +0200 Subject: [PATCH 3/6] Compute severity for non-resilient services too, added more tests --- controller_test.go | 105 ++++++++++++++++++++---------------------- service.go | 1 + severityController.go | 39 ++++++++++------ 3 files changed, 76 insertions(+), 69 deletions(-) diff --git a/controller_test.go b/controller_test.go index b51150d..8959728 100644 --- a/controller_test.go +++ b/controller_test.go @@ -10,6 +10,8 @@ import ( fthealth "github.com/Financial-Times/go-fthealth/v1_1" "github.com/stretchr/testify/assert" + "strconv" + "strings" ) const ( @@ -24,8 +26,21 @@ const ( validCat = "validCat" validService = "validService" validEnvName = "valid-env-name" + ip = "10.2.51.2" + severity1 = uint8(1) ) +var defaultPods = []pod{ + { + name: "test-pod-name2-8425234-9hdfg ", + ip: "10.2.51.2", + }, + { + name: "test-pod-name1-8425234-9hdfg ", + ip: "10.2.51.2", + }, +} + type MockService struct { httpClient *http.Client } @@ -75,10 +90,14 @@ func (m *MockService) getServiceByName(serviceName string) (service, error) { if serviceName == nonExistingServiceName { return service{}, fmt.Errorf("Cannot find service with name %s", serviceName) } - + resiliency := false + if strings.HasPrefix(serviceName, "resilient") { + resiliency = true + } return service{ - name: "test-service-name", - ack: "test ack", + name: "test-service-name", + ack: "test ack", + isResilient: resiliency, }, nil } @@ -99,65 +118,35 @@ func (m *MockService) getServicesMapByNames(serviceNames []string) map[string]se return services } +func createPod(goodCount int, notOkSeverities []int) []pod { + var pods []pod + for i := 0; i < goodCount; i++ { + pods = append(pods, pod{name: "ok-pod-" + strconv.Itoa(i), ip: ip}) + } + for _, sev := range notOkSeverities { + pods = append(pods, pod{name: "notok-pod-" + strconv.Itoa(sev), ip: ip}) + } + fmt.Println(pods) + return pods +} func (m *MockService) getPodsForService(serviceName string) ([]pod, error) { switch serviceName { case "invalidNameForService": - return []pod{}, errors.New("Invalid pod name") + return []pod{}, errors.New("invalid pod name") case "resilient-notok-sev1": - return []pod{ - { - name: "notok-pod-1", - ip: "10.2.51.2", - }, - { - name: "notok-pod-1", - ip: "10.2.51.2", - }, - }, nil + return createPod(0, []int{1, 1}), nil case "resilient-notok-sev2": - return []pod{ - { - name: "notok-pod-2", - ip: "10.2.51.2", - }, - { - name: "notok-pod-2", - ip: "10.2.51.2", - }, - }, nil + return createPod(0, []int{2, 2}), nil case "resilient-halfok-sev1": - return []pod{ - { - name: "notok-pod-1", - ip: "10.2.51.2", - }, - { - name: "ok-pod-2", - ip: "10.2.51.2", - }, - }, nil + return createPod(1, []int{1}), nil case "resilient-halfok-sev2": - return []pod{ - { - name: "notok-pod-2", - ip: "10.2.51.2", - }, - { - name: "ok-pod-2", - ip: "10.2.51.2", - }, - }, nil + return createPod(1, []int{2}), nil + case "non-resilient-halfok-sev1": + return createPod(1, []int{1}), nil + case "non-resilient-halfok-sev2": + return createPod(1, []int{2}), nil default: - return []pod{ - { - name: "test-pod-name2-8425234-9hdfg ", - ip: "10.2.51.2", - }, - { - name: "test-pod-name1-8425234-9hdfg ", - ip: "10.2.51.2", - }, - }, nil + return defaultPods, nil } } @@ -370,7 +359,7 @@ func TestGetSeverityForServiceInvalidServiceName(t *testing.T) { func TestGetSeverityForResilientService(t *testing.T) { controller := initializeMockController("test", nil) severity := controller.getSeverityForService("resilient-notok-sev1", 8080) - assert.Equal(t, uint8(1), severity) + assert.Equal(t, severity1, severity) severity = controller.getSeverityForService("resilient-notok-sev2", 8080) assert.Equal(t, defaultSeverity, severity) @@ -380,6 +369,12 @@ func TestGetSeverityForResilientService(t *testing.T) { severity = controller.getSeverityForService("resilient-halfok-sev2", 8080) assert.Equal(t, defaultSeverity, severity) + + severity = controller.getSeverityForService("non-resilient-halfok-sev1", 8080) + assert.Equal(t, severity1, severity) + + severity = controller.getSeverityForService("non-resilient-halfok-sev2", 8080) + assert.Equal(t, defaultSeverity, severity) } func TestGetSeverityForNonResilientService(t *testing.T) { diff --git a/service.go b/service.go index 972b6bd..af23bfd 100644 --- a/service.go +++ b/service.go @@ -42,6 +42,7 @@ type healthcheckService interface { const ( defaultRefreshRate = 60 defaultSeverity = uint8(2) + defaultResiliency = true ackMessagesConfigMapName = "healthcheck.ack.messages" ackMessagesConfigMapLabelSelector = "healthcheck-acknowledgements-for=aggregate-healthcheck" defaultAppPort = int32(8080) diff --git a/severityController.go b/severityController.go index 3a6e038..652ce24 100644 --- a/severityController.go +++ b/severityController.go @@ -2,28 +2,39 @@ package main func (c *healthCheckController) getSeverityForService(serviceName string, appPort int32) uint8 { pods, err := c.healthCheckService.getPodsForService(serviceName) - if err != nil { warnLogger.Printf("Cannot get pods for service with name %s in order to get severity level, using default severity: %d. Problem was: %s", serviceName, defaultSeverity, err.Error()) return defaultSeverity } - finalSeverity := defaultSeverity - for _, pod := range pods { - individualPodSeverity, checkFailed, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) + var isResilient bool + service, err := c.healthCheckService.getServiceByName(serviceName) + if err != nil { + warnLogger.Printf("Cannot get service with name %s in order to get resiliency, using default resiliency: %d. Problem was: %s", serviceName, defaultResiliency, err.Error()) + isResilient = defaultResiliency + } else { + isResilient = service.isResilient + } - if err != nil { - warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) - continue - } - if !checkFailed { - return defaultSeverity - } - if individualPodSeverity < finalSeverity { - finalSeverity = individualPodSeverity + if isResilient { + finalSeverity := defaultSeverity + for _, pod := range pods { + individualPodSeverity, checkFailed, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) + + if err != nil { + warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) + continue + } + if !checkFailed { + return defaultSeverity + } + if individualPodSeverity < finalSeverity { + finalSeverity = individualPodSeverity + } + return finalSeverity } } - return finalSeverity + return c.computeSeverityByPods(pods, appPort) } func (c *healthCheckController) getSeverityForPod(podName string, appPort int32) uint8 { From de24c46d13e5aea2286db708f56abeab9265312f Mon Sep 17 00:00:00 2001 From: Tamas Molnar Date: Tue, 27 Feb 2018 14:35:46 +0200 Subject: [PATCH 4/6] Refactored severity tests, small adjustments to mocks --- controller_test.go | 89 ++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/controller_test.go b/controller_test.go index 8959728..428e338 100644 --- a/controller_test.go +++ b/controller_test.go @@ -79,25 +79,17 @@ func (m *MockService) getDeployments() (map[string]deployment, error) { } func (m *MockService) isServicePresent(serviceName string) bool { - if serviceName == nonExistingServiceName { - return false - } - - return true + return serviceName != nonExistingServiceName } func (m *MockService) getServiceByName(serviceName string) (service, error) { if serviceName == nonExistingServiceName { return service{}, fmt.Errorf("Cannot find service with name %s", serviceName) } - resiliency := false - if strings.HasPrefix(serviceName, "resilient") { - resiliency = true - } return service{ name: "test-service-name", ack: "test ack", - isResilient: resiliency, + isResilient: strings.HasPrefix(serviceName, "resilient"), }, nil } @@ -105,7 +97,6 @@ func (m *MockService) getServicesMapByNames(serviceNames []string) map[string]se if len(serviceNames) != 0 && serviceNames[0] == nonExistingServiceName { return map[string]service{} } - services := make(map[string]service) services["test-service-name"] = service{ name: "test-service-name", @@ -114,11 +105,10 @@ func (m *MockService) getServicesMapByNames(serviceNames []string) map[string]se services["test-service-name-2"] = service{ name: "test-service-name-2", } - return services } -func createPod(goodCount int, notOkSeverities []int) []pod { +func createPods(goodCount int, notOkSeverities []int) []pod { var pods []pod for i := 0; i < goodCount; i++ { pods = append(pods, pod{name: "ok-pod-" + strconv.Itoa(i), ip: ip}) @@ -126,25 +116,25 @@ func createPod(goodCount int, notOkSeverities []int) []pod { for _, sev := range notOkSeverities { pods = append(pods, pod{name: "notok-pod-" + strconv.Itoa(sev), ip: ip}) } - fmt.Println(pods) return pods } + func (m *MockService) getPodsForService(serviceName string) ([]pod, error) { switch serviceName { case "invalidNameForService": return []pod{}, errors.New("invalid pod name") case "resilient-notok-sev1": - return createPod(0, []int{1, 1}), nil + return createPods(0, []int{1, 1}), nil case "resilient-notok-sev2": - return createPod(0, []int{2, 2}), nil + return createPods(0, []int{2, 2}), nil case "resilient-halfok-sev1": - return createPod(1, []int{1}), nil + return createPods(1, []int{1}), nil case "resilient-halfok-sev2": - return createPod(1, []int{2}), nil + return createPods(1, []int{2}), nil case "non-resilient-halfok-sev1": - return createPod(1, []int{1}), nil + return createPods(1, []int{1}), nil case "non-resilient-halfok-sev2": - return createPod(1, []int{2}), nil + return createPods(1, []int{2}), nil default: return defaultPods, nil } @@ -199,7 +189,6 @@ func (m *MockService) getHealthChecksForPod(pod pod, port int32) (healthcheckRes if pod.name == nonExistingPodName { return healthcheckResponse{}, errors.New("Cannot find pod") } - return healthcheckResponse{}, nil } @@ -207,14 +196,12 @@ func (m *MockService) addAck(serviceName string, ackMessage string) error { if serviceName == serviceNameForAckErr { return errors.New("Error") } - return nil } func (m *MockService) removeAck(serviceName string) error { if serviceName == serviceNameForAckErr { return errors.New("Cannot remove ack") } - return nil } func (m *MockService) getHTTPClient() *http.Client { @@ -227,7 +214,6 @@ func initializeMockedHTTPClient(responseStatusCode int, responseBody string) *ht responseStatusCode: responseStatusCode, responseBody: responseBody, } - return client } @@ -358,23 +344,48 @@ func TestGetSeverityForServiceInvalidServiceName(t *testing.T) { func TestGetSeverityForResilientService(t *testing.T) { controller := initializeMockController("test", nil) - severity := controller.getSeverityForService("resilient-notok-sev1", 8080) - assert.Equal(t, severity1, severity) - - severity = controller.getSeverityForService("resilient-notok-sev2", 8080) - assert.Equal(t, defaultSeverity, severity) - - severity = controller.getSeverityForService("resilient-halfok-sev1", 8080) - assert.Equal(t, defaultSeverity, severity) - - severity = controller.getSeverityForService("resilient-halfok-sev2", 8080) - assert.Equal(t, defaultSeverity, severity) - severity = controller.getSeverityForService("non-resilient-halfok-sev1", 8080) - assert.Equal(t, severity1, severity) + var testCases = []struct { + serviceName string + expectedSeverity uint8 + description string + }{ + { + serviceName: "resilient-notok-sev1", + expectedSeverity: severity1, + description: "resilient service with all pods failing a severity 1 check", + }, + { + serviceName: "resilient-notok-sev2", + expectedSeverity: defaultSeverity, + description: "resilient service with all pods failing a severity 2 check", + }, + { + serviceName: "resilient-halfok-sev1", + expectedSeverity: defaultSeverity, + description: "resilient service with one of two pods failing a severity 1 check", + }, + { + serviceName: "resilient-halfok-sev2", + expectedSeverity: defaultSeverity, + description: "resilient service with one of two pods failing a severity 2 check", + }, + { + serviceName: "non-resilient-halfok-sev1", + expectedSeverity: severity1, + description: "non-resilient service with one of two pods failing a severity 1 check", + }, + { + serviceName: "non-resilient-halfok-sev2", + expectedSeverity: defaultSeverity, + description: "non-resilient service with one of two pods failing a severity 2 check", + }, + } + for _, tc := range testCases { + actualSeverity := controller.getSeverityForService(tc.serviceName, 8080) + assert.Equal(t, tc.expectedSeverity, actualSeverity, tc.description) + } - severity = controller.getSeverityForService("non-resilient-halfok-sev2", 8080) - assert.Equal(t, defaultSeverity, severity) } func TestGetSeverityForNonResilientService(t *testing.T) { From 3dfda66e8ac7eb60f3283b60c67b9c6f1d89ed8f Mon Sep 17 00:00:00 2001 From: Tamas Molnar Date: Tue, 27 Feb 2018 14:43:15 +0200 Subject: [PATCH 5/6] Simplify the severity compute code a bit --- severityController.go | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/severityController.go b/severityController.go index 652ce24..875cd24 100644 --- a/severityController.go +++ b/severityController.go @@ -16,25 +16,28 @@ func (c *healthCheckController) getSeverityForService(serviceName string, appPor isResilient = service.isResilient } - if isResilient { - finalSeverity := defaultSeverity - for _, pod := range pods { - individualPodSeverity, checkFailed, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) + if !isResilient { + return c.computeSeverityByPods(pods, appPort) + } + + finalSeverity := defaultSeverity + for _, pod := range pods { + individualPodSeverity, checkFailed, err := c.healthCheckService.getIndividualPodSeverity(pod, appPort) - if err != nil { - warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) - continue - } - if !checkFailed { - return defaultSeverity - } - if individualPodSeverity < finalSeverity { - finalSeverity = individualPodSeverity - } - return finalSeverity + if err != nil { + warnLogger.Printf("Cannot get individual pod severity, skipping pod. Problem was: %s", err.Error()) + continue + } + // if at least one pod of the resilient service is healthy, we return the default severity, + // regardless of the status of other pods + if !checkFailed { + return defaultSeverity + } + if individualPodSeverity < finalSeverity { + finalSeverity = individualPodSeverity } } - return c.computeSeverityByPods(pods, appPort) + return finalSeverity } func (c *healthCheckController) getSeverityForPod(podName string, appPort int32) uint8 { From 4feb9eee2036b6eb14ce612725796300ae007149 Mon Sep 17 00:00:00 2001 From: Sorin Buliarca Date: Fri, 2 Mar 2018 12:39:59 +0200 Subject: [PATCH 6/6] Fixed getting the individual pod severity. --- checkerService.go | 6 ++++-- controller_test.go | 2 +- service_test.go | 28 +++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/checkerService.go b/checkerService.go index 609a88d..22e40dc 100644 --- a/checkerService.go +++ b/checkerService.go @@ -80,15 +80,17 @@ func (hs *k8sHealthcheckService) getIndividualPodSeverity(pod pod, appPort int32 } finalSeverity := uint8(2) + checkFailed := bool(false) for _, check := range health.Checks { if !check.OK { + checkFailed = true if check.Severity < finalSeverity { - return check.Severity, true, nil + return check.Severity, checkFailed, nil } } } - return finalSeverity, false, nil + return finalSeverity, checkFailed, nil } func (hs *k8sHealthcheckService) getHealthChecksForPod(pod pod, appPort int32) (healthcheckResponse, error) { diff --git a/controller_test.go b/controller_test.go index 428e338..7d660b3 100644 --- a/controller_test.go +++ b/controller_test.go @@ -124,7 +124,7 @@ func (m *MockService) getPodsForService(serviceName string) ([]pod, error) { case "invalidNameForService": return []pod{}, errors.New("invalid pod name") case "resilient-notok-sev1": - return createPods(0, []int{1, 1}), nil + return createPods(0, []int{2, 1}), nil case "resilient-notok-sev2": return createPods(0, []int{2, 2}), nil case "resilient-halfok-sev1": diff --git a/service_test.go b/service_test.go index 6fe023e..66512dc 100644 --- a/service_test.go +++ b/service_test.go @@ -50,6 +50,23 @@ const ( "name": "" } ] +}` + validFailingHealthCheckResponseBodyWithSeverity2 = `{ + "schemaVersion": 1, + "name": "CMSNotifierApplication", + "description": "CMSNotifierApplication", + "checks": [ + { + "checkOutput": "", + "panicGuide": "", + "lastUpdated": "", + "ok": false, + "severity": 2, + "businessImpact": "", + "technicalSummary": "", + "name": "" + } + ] }` validPassingHealthCheckResponseBody = `{ "schemaVersion": 1, @@ -140,11 +157,20 @@ func TestGetIndividualPodSeverityErrorWhilePodHealthCheck(t *testing.T) { func TestGetIndividualPodSeverityValidPodHealth(t *testing.T) { service := initializeMockService(initializeMockHTTPClient(http.StatusOK, validFailingHealthCheckResponseBody)) - severity, _, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) + severity, checkFailed, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) assert.Nil(t, err) + assert.True(t, checkFailed) assert.Equal(t, validSeverity, severity) } +func TestGetIndividualPodSeverityValidPodHealth_Severity2(t *testing.T) { + service := initializeMockService(initializeMockHTTPClient(http.StatusOK, validFailingHealthCheckResponseBodyWithSeverity2)) + severity, checkFailed, err := service.getIndividualPodSeverity(pod{name: "test", ip: validIP}, 8080) + assert.Nil(t, err) + assert.True(t, checkFailed) + assert.Equal(t, uint8(2), severity) +} + func TestCheckPodHealthFailingChecks(t *testing.T) { service := initializeMockService(initializeMockHTTPClient(http.StatusOK, validFailingHealthCheckResponseBody)) err := service.checkPodHealth(pod{name: "test", ip: validIP}, 8080)