From 15c29115e96e1fffacc8f5b7adcf40606491779f Mon Sep 17 00:00:00 2001 From: Illia Khudoshyn Date: Mon, 27 Feb 2017 17:31:26 +0200 Subject: [PATCH] Add support for 'ignore-all' --- e2e/basic_test.go | 25 ++++++ examples/on-error-ignore-all/create.sh | 15 ++++ examples/on-error-ignore-all/delete.sh | 11 +++ examples/on-error-ignore-all/deps.yaml | 13 +++ examples/on-error-ignore-all/pod.yaml | 13 +++ examples/on-error-ignore-all/pod2.yaml | 13 +++ .../on-error-ignore-all/timedout-pod.yaml | 25 ++++++ pkg/mocks/resource.go | 11 ++- pkg/resources/common.go | 10 ++- pkg/resources/common_test.go | 81 +++++++++++++++++++ pkg/scheduler/scheduler.go | 23 ++++++ 11 files changed, 236 insertions(+), 4 deletions(-) create mode 100755 examples/on-error-ignore-all/create.sh create mode 100755 examples/on-error-ignore-all/delete.sh create mode 100644 examples/on-error-ignore-all/deps.yaml create mode 100644 examples/on-error-ignore-all/pod.yaml create mode 100644 examples/on-error-ignore-all/pod2.yaml create mode 100644 examples/on-error-ignore-all/timedout-pod.yaml create mode 100644 pkg/resources/common_test.go diff --git a/e2e/basic_test.go b/e2e/basic_test.go index 7f539f5..eaab3b7 100644 --- a/e2e/basic_test.go +++ b/e2e/basic_test.go @@ -164,6 +164,31 @@ var _ = Describe("Basic Suite", func() { }) }) }) + + Describe("Failure handling - ignore-all", func() { + var parentPod *v1.Pod + var childPod *v1.Pod + var grandChildPod *v1.Pod + + BeforeEach(func() { + parentPod = DelayedPod("parent-pod", 15) + childPod = PodPause("child-pod") + grandChildPod = PodPause("grand-child-pod") + }) + + Context("If failed parent is marked on-error:ignore-all", func() { + It("all children including transitive must not be created", func() { + parentResDef := framework.WrapWithMetaAndCreate(parentPod, map[string]interface{}{"timeout": 5, "on-error": "ignore"}) + childResDef := framework.WrapAndCreate(childPod) + grandChildResDef := framework.WrapAndCreate(grandChildPod) + framework.Connect(parentResDef, childResDef) + framework.Connect(childResDef, grandChildResDef) + framework.Run() + testutils.WaitForPodNotToBeCreated(framework.Clientset, framework.Namespace.Name, childPod.Name) + testutils.WaitForPodNotToBeCreated(framework.Clientset, framework.Namespace.Name, grandChildPod.Name) + }) + }) + }) }) func getKind(resdef *client.ResourceDefinition) string { diff --git a/examples/on-error-ignore-all/create.sh b/examples/on-error-ignore-all/create.sh new file mode 100755 index 0000000..402c250 --- /dev/null +++ b/examples/on-error-ignore-all/create.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +source ../common.sh + +$KUBECTL_NAME create -f ../../manifests/appcontroller.yaml +wait-appcontroller + +$KUBECTL_NAME create -f deps.yaml + +cat timedout-pod.yaml | $KUBECTL_NAME create -f - +cat pod.yaml | $KUBECTL_NAME exec -i k8s-appcontroller kubeac wrap | $KUBECTL_NAME create -f - +cat pod2.yaml | $KUBECTL_NAME exec -i k8s-appcontroller kubeac wrap | $KUBECTL_NAME create -f - + +$KUBECTL_NAME exec k8s-appcontroller ac-run +$KUBECTL_NAME logs -f k8s-appcontroller diff --git a/examples/on-error-ignore-all/delete.sh b/examples/on-error-ignore-all/delete.sh new file mode 100755 index 0000000..33e506a --- /dev/null +++ b/examples/on-error-ignore-all/delete.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +source ../common.sh + +$KUBECTL_NAME delete -f deps.yaml + +cat timedout-pod.yaml | $KUBECTL_NAME delete -f - +cat pod.yaml | $KUBECTL_NAME exec -i k8s-appcontroller kubeac wrap | $KUBECTL_NAME delete -f - +cat pod2.yaml | $KUBECTL_NAME exec -i k8s-appcontroller kubeac wrap | $KUBECTL_NAME delete -f - + +$KUBECTL_NAME delete -f ../../manifests/appcontroller.yaml diff --git a/examples/on-error-ignore-all/deps.yaml b/examples/on-error-ignore-all/deps.yaml new file mode 100644 index 0000000..bf2dfb5 --- /dev/null +++ b/examples/on-error-ignore-all/deps.yaml @@ -0,0 +1,13 @@ +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: ce1b11dc-2850-1dad-a7dd-302038af20af +parent: pod/timed-out-pod +child: pod/eventually-alive-pod +--- +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: 6d3a53be-6cec-425d-aec0-e188d739e9f6 +parent: pod/eventually-alive-pod +child: pod/eventually-alive-pod2 diff --git a/examples/on-error-ignore-all/pod.yaml b/examples/on-error-ignore-all/pod.yaml new file mode 100644 index 0000000..818a0cf --- /dev/null +++ b/examples/on-error-ignore-all/pod.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: eventually-alive-pod +spec: + containers: + - command: ["/bin/sh"] + args: + - -c + - echo ok > /tmp/health + image: gcr.io/google_containers/busybox + name: test-container + restartPolicy: Never diff --git a/examples/on-error-ignore-all/pod2.yaml b/examples/on-error-ignore-all/pod2.yaml new file mode 100644 index 0000000..6c6beaa --- /dev/null +++ b/examples/on-error-ignore-all/pod2.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: eventually-alive-pod2 +spec: + containers: + - command: ["/bin/sh"] + args: + - -c + - echo ok > /tmp/health + image: gcr.io/google_containers/busybox + name: test-container + restartPolicy: Never diff --git a/examples/on-error-ignore-all/timedout-pod.yaml b/examples/on-error-ignore-all/timedout-pod.yaml new file mode 100644 index 0000000..ca2093b --- /dev/null +++ b/examples/on-error-ignore-all/timedout-pod.yaml @@ -0,0 +1,25 @@ +apiVersion: appcontroller.k8s/v1alpha1 +kind: Definition +metadata: + name: pod-timed-out-pod +meta: + timeout: 5 + on-error: ignore-all +pod: + apiVersion: v1 + kind: Pod + metadata: + name: timed-out-pod + spec: + containers: + - command: ["/bin/sh"] + args: + - -c + - sleep 30; echo ok > /tmp/health; sleep 60 + image: gcr.io/google_containers/busybox + readinessProbe: + exec: + command: + - /bin/cat + - /tmp/health + name: test-container diff --git a/pkg/mocks/resource.go b/pkg/mocks/resource.go index 84e8ac9..aa2f234 100644 --- a/pkg/mocks/resource.go +++ b/pkg/mocks/resource.go @@ -24,6 +24,7 @@ import ( type Resource struct { key string status string + meta map[string]interface{} } // Key returns a key of the Resource @@ -47,8 +48,8 @@ func (c *Resource) Delete() error { } // Meta returns empty string -func (c *Resource) Meta(string) interface{} { - return nil +func (c *Resource) Meta(key string) interface{} { + return c.meta[key] } // NameMatches returns true @@ -73,8 +74,14 @@ func (c *Resource) StatusIsCacheable(meta map[string]string) bool { // NewResource creates new instance of Resource func NewResource(key string, status string) *Resource { + return NewResourceWithMeta(key, status, nil) +} + +// NewResourceWithMeta creates new instance of Resource +func NewResourceWithMeta(key string, status string, meta map[string]interface{}) *Resource { return &Resource{ key: key, status: status, + meta: meta, } } diff --git a/pkg/resources/common.go b/pkg/resources/common.go index a6fc913..b8d2a62 100644 --- a/pkg/resources/common.go +++ b/pkg/resources/common.go @@ -170,13 +170,19 @@ func GetIntMeta(r interfaces.BaseResource, paramName string, defaultValue int) i return defaultValue } - intVal, ok := value.(float64) + intVal, ok := value.(int) + if ok { + return intVal + } + + floatVal, ok := value.(float64) + if !ok { log.Printf("Metadata parameter '%s' for resource '%s' is set to '%v' but it does not seem to be a number, using default value %d", paramName, r.Key(), value, defaultValue) return defaultValue } - return int(intVal) + return int(floatVal) } // GetStringMeta returns metadata value for parameter 'paramName', or 'defaultValue' diff --git a/pkg/resources/common_test.go b/pkg/resources/common_test.go new file mode 100644 index 0000000..4efd049 --- /dev/null +++ b/pkg/resources/common_test.go @@ -0,0 +1,81 @@ +// Copyright 2016 Mirantis +// +// 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 resources + +import ( + "testing" + + "github.com/Mirantis/k8s-AppController/pkg/mocks" +) + +// TestGetStringMeta checks metadata retrieval from a resource +func TestGetStringMeta(t *testing.T) { + r := mocks.NewResource("fake", "not ready") + + if GetStringMeta(r, "non-existing key", "default") != "default" { + t.Error("GetStringMeta for non-existing key returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": "value"}) + + if GetStringMeta(r, "non-existing key", "default") != "default" { + t.Error("GetStringMeta for non-existing key returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": 1}) + + if GetStringMeta(r, "key", "default") != "default" { + t.Error("GetStringMeta for non-string value returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": "value"}) + + if GetStringMeta(r, "key", "default") != "value" { + t.Error("GetStringMeta returned not an actual value") + } +} + +// TestGetIntMeta checks metadata retrieval from a resource +func TestGetIntMeta(t *testing.T) { + r := mocks.NewResource("fake", "not ready") + + if GetIntMeta(r, "non-existing key", -1) != -1 { + t.Error("GetIntMeta for non-existing key returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": "value"}) + + if GetIntMeta(r, "non-existing key", -1) != -1 { + t.Error("GetIntMeta for non-existing key returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": "value"}) + + if GetIntMeta(r, "key", -1) != -1 { + t.Error("GetIntMeta for non-int value returned not a default value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": 42}) + + if GetIntMeta(r, "key", -1) != 42 { + t.Error("GetIntMeta returned not an actual value") + } + + r = mocks.NewResourceWithMeta("fake", "not ready", map[string]interface{}{"key": 42.}) + + if GetIntMeta(r, "key", -1) != 42 { + t.Error("GetIntMeta returned not an actual value") + } +} diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 4a9f9a7..95f1924 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -388,6 +388,15 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL } + r.RLock() + ignored := r.Ignored + r.RUnlock() + + if ignored { + log.Printf("Skipping creation of resource %s as being ignored", r.Key()) + break + } + log.Printf("Creating resource %s, attempt %d of %d", r.Key(), attemptNo, attempts) err = r.Create() if err != nil { @@ -412,6 +421,8 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL r.Ignored = true log.Printf("Resource %s failure ignored -- prooceeding as normal", r.Key()) r.Unlock() + } else if onError == "ignore-all" { + ignoreAll(r) } } } @@ -422,6 +433,18 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL } } +func ignoreAll(top *ScheduledResource) { + top.Lock() + top.Ignored = true + top.Unlock() + + log.Printf("Marking resource %s as ignored", top.Key()) + + for _, child := range top.RequiredBy { + ignoreAll(child) + } +} + // Create starts the deployment of a DependencyGraph func Create(depGraph DependencyGraph, concurrency int) {