From 9ed8be686f3440636a125ad5d77bb498c05198fe Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Sat, 20 May 2017 18:22:22 -0700 Subject: [PATCH 1/6] Dependency replication This change adds ability to replicate dependency with index parameters iterated over arbitrary number of lists. For each dependency it is now possible to specify map of indexVariableName -> listExpression listExpression := range|item + [, listExpression] range := number '..' number item := STRING for example, if for "i: 1..3" the dependency will be replicated into 3 clones, each one of them having argument i set to value in range [1, 3] This also allows to consume N flow replicas by replicating the dependency that leads to the consumed flow --- docs/flows.md | 92 +++++++++++++++- pkg/client/dependencies.go | 3 + pkg/interfaces/interfaces.go | 1 - pkg/resources/flow.go | 12 +-- pkg/scheduler/dependency_graph.go | 144 +++++++++++++++++++++---- pkg/scheduler/dependency_graph_test.go | 74 +++++++++++++ pkg/scheduler/flows_test.go | 99 +++++++++++++++++ 7 files changed, 394 insertions(+), 31 deletions(-) diff --git a/docs/flows.md b/docs/flows.md index 9d42583..36d82d1 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -195,7 +195,7 @@ my-flow -> -> pod/pod-$arg ``` will create two pods: `pod-a` and `pod-b`. -## Replication of flows +## Replication Flow replication is an AppController feature that makes specified number of flow graph copies, each one with a unique name and then merges them into a single graph. Because each replica name may be used in some of resource @@ -234,6 +234,96 @@ If there were 7 of them, 4 replicas would be deleted.\ `kubeac run my-flow` if there are no replicas exist, create one, otherwise validate status of resources of existing replicas. +### Replication of dependencies + +With commandline parameters one can create number of flow replicas. But sometimes there is a need to have flow +that creates several replicas of another flow, or just several resources with the same specification that differ +only in name. + +One possible solution is to utilize technique shown above: make parameter value be part of resource name and +then duplicate the dependency that leads to this resource and pass different parameter value along each of +dependencies. This works well for small and fixed number of replicas. But if the number goes big, it becomes hard +to manage such number of dependency objects. Moreover if the number itself is not fixed but rather passed as a +parameter replicating resource by manual replication of dependencies becomes impossible. + +Luckily, the dependencies can be automatically replicated. This is done through the `generateFor` field of the +`Dependency` object. `generateFor` is a map where keys are argument names and values are list expressions. Each list +expression is comma-separated list of values. If the value has a form of `number..number`, it is expended into a +list of integers in the given range. For example `"1..3, 10..11, abc"` will turn into `["1", "2", "3", "10", "11", "abc"]`. +Then the dependency is going to be replicated automatically with each replica getting on of the list values as an +additional argument. There can be several `generateFor` arguments. In this case there is going to be one dependency +for each combination of the list values. For example, + +```YAML +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency +parent: pod/podName +child: flow/flowName-$x-$y +generateFor: + x: 1..2 + y: a, b +``` + +has the same effect as + +```YAML +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency1 +parent: pod/podName +child: flow/flowName-$x-$y +args: + x: 1 + y: a +--- +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency2 +parent: pod/podName +child: flow/flowName-$x-$y +args: + x: 2 + y: a +--- +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency3 +parent: pod/podName +child: flow/flowName-$x-$y +args: + x: 1 + y: b +--- +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency4 +parent: pod/podName +child: flow/flowName-$x-$y +args: + x: 2 + y: b +``` + +Besides simplifying the dependency graph, dependency replication makes possible to have dynamic number of replicas +by using parameter value right inside the list expressions: + +```YAML +apiVersion: appcontroller.k8s/v1alpha1 +kind: Dependency +metadata: + name: dependency +parent: pod/podName +child: flow/flowName-$index +generateFor: + index: 1..$replicaCount +``` + ### Replica-spaces and contexts Replica-space, is a tag that all replicas of the flow share. When new `Replica` object for the flow is created, diff --git a/pkg/client/dependencies.go b/pkg/client/dependencies.go index 075b48d..5b72158 100644 --- a/pkg/client/dependencies.go +++ b/pkg/client/dependencies.go @@ -41,6 +41,9 @@ type Dependency struct { // Arguments passed to dependent resource Args map[string]string `json:"args,omitempty"` + + // map of variable name -> list expression. New dependencies are generated by replication and iteration over those lists + GenerateFor map[string]string `json:"generateFor,omitempty"` } // DependencyList is a k8s object representing list of dependencies diff --git a/pkg/interfaces/interfaces.go b/pkg/interfaces/interfaces.go index 384c010..929d627 100644 --- a/pkg/interfaces/interfaces.go +++ b/pkg/interfaces/interfaces.go @@ -82,7 +82,6 @@ type GraphContext interface { Scheduler() Scheduler GetArg(string) string Graph() DependencyGraph - Dependency() *client.Dependency } // DependencyGraphOptions contains all the input required to build a dependency graph diff --git a/pkg/resources/flow.go b/pkg/resources/flow.go index 564801f..4f7c0ce 100644 --- a/pkg/resources/flow.go +++ b/pkg/resources/flow.go @@ -15,9 +15,7 @@ package resources import ( - "fmt" "log" - "strings" "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" @@ -29,7 +27,6 @@ type flow struct { flow *client.Flow context interfaces.GraphContext originalName string - instanceName string currentGraph interfaces.DependencyGraph } @@ -52,19 +49,12 @@ func (flowTemplateFactory) Kind() string { func (flowTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { newFlow := parametrizeResource(def.Flow, gc, []string{"*"}).(*client.Flow) - dep := gc.Dependency() - var depName string - if dep != nil { - depName = strings.Replace(dep.Name, dep.GenerateName, "", 1) - } - return report.SimpleReporter{ BaseResource: &flow{ Base: Base{def.Meta}, flow: newFlow, context: gc, originalName: def.Flow.Name, - instanceName: fmt.Sprintf("%s%s", depName, gc.GetArg("AC_NAME")), }} } @@ -98,7 +88,7 @@ func (f *flow) buildDependencyGraph(replicaCount int, silent bool) (interfaces.D options := interfaces.DependencyGraphOptions{ FlowName: f.originalName, Args: args, - FlowInstanceName: f.instanceName, + FlowInstanceName: f.context.GetArg("AC_ID"), ReplicaCount: replicaCount, Silent: silent, FixedNumberOfReplicas: fixedNumberOfReplicas, diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index 8dab597..737840d 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -19,6 +19,7 @@ import ( "fmt" "log" "sort" + "strconv" "strings" "time" @@ -40,12 +41,12 @@ type dependencyGraph struct { } type graphContext struct { - args map[string]string - graph *dependencyGraph - scheduler *scheduler - flow *client.Flow - dependency *client.Dependency - replica string + args map[string]string + graph *dependencyGraph + scheduler *scheduler + flow *client.Flow + id string + replica string } var _ interfaces.GraphContext = &graphContext{} @@ -62,6 +63,8 @@ func (gc graphContext) GetArg(name string) string { return gc.replica case "AC_FLOW_NAME": return gc.flow.Name + case "AC_ID": + return gc.id default: val, ok := gc.args[name] if ok { @@ -84,11 +87,6 @@ func (gc graphContext) Graph() interfaces.DependencyGraph { return gc.graph } -// Dependency returns Dependency for which child is the resource being created with this context -func (gc graphContext) Dependency() *client.Dependency { - return gc.dependency -} - // newScheduledResourceFor returns new scheduled resource for given resource in init state func newScheduledResourceFor(r interfaces.Resource, suffix string, context *graphContext, existing bool) *ScheduledResource { return &ScheduledResource{ @@ -159,7 +157,9 @@ func groupDependencies(dependencies []client.Dependency, defaultFlow = []client.Dependency{} addResource := func(name string) { if !strings.HasPrefix(name, "flow/") && !isDependant[name] { - defaultFlow = append(defaultFlow, client.Dependency{Parent: defaultFlowName, Child: name}) + dep := client.Dependency{Parent: defaultFlowName, Child: name} + dep.Name = name + defaultFlow = append(defaultFlow, dep) isDependant[name] = true } } @@ -328,11 +328,11 @@ func getArgFunc(gc interfaces.GraphContext) func(string) string { func (sched *scheduler) prepareContext(parentContext *graphContext, dependency *client.Dependency, replica string) *graphContext { context := &graphContext{ - scheduler: sched, - graph: parentContext.graph, - flow: parentContext.flow, - replica: replica, - dependency: dependency, + scheduler: sched, + graph: parentContext.graph, + flow: parentContext.flow, + replica: replica, + id: getVertexID(dependency, replica), } context.args = make(map[string]string) @@ -344,6 +344,15 @@ func (sched *scheduler) prepareContext(parentContext *graphContext, dependency * return context } +func getVertexID(dependency *client.Dependency, replica string) string { + var depName string + if dependency != nil { + depName = strings.Replace(dependency.Name, dependency.GenerateName, "", 1) + } + depName += replica + return depName +} + func (sched *scheduler) updateContext(context, parentContext *graphContext, dependency client.Dependency) { for key, value := range dependency.Args { context.args[key] = copier.EvaluateString(value, parentContext.GetArg) @@ -661,6 +670,105 @@ func (sched *scheduler) BuildDependencyGraph(options interfaces.DependencyGraphO return depGraph, nil } +func listDependencies(dependencies map[string][]client.Dependency, parent string, flow *client.Flow, + useDestructionSelector bool, context *graphContext) []client.Dependency { + + deps := filterDependencies(dependencies, parent, flow, useDestructionSelector) + var result []client.Dependency + for _, dep := range deps { + if len(dep.GenerateFor) == 0 { + result = append(result, dep) + continue + } + + var keys []string + for k := range dep.GenerateFor { + keys = append(keys, k) + } + sort.Strings(keys) + lists := make([][]string, len(dep.GenerateFor)) + for i, key := range keys { + lists[i] = expandListExpression(copier.EvaluateString(dep.GenerateFor[key], getArgFunc(context))) + } + for n, combination := range permute(lists) { + newArgs := make(map[string]string, len(dep.Args)+len(keys)) + for k, v := range dep.Args { + newArgs[k] = v + } + for i, key := range keys { + newArgs[key] = combination[i] + } + depCopy := dep + depCopy.Args = newArgs + depCopy.Name += strconv.Itoa(n + 1) + result = append(result, depCopy) + } + } + return result +} + +func permute(variants [][]string) [][]string { + switch len(variants) { + case 0: + return variants + case 1: + var result [][]string + for _, v := range variants[0] { + result = append(result, []string{v}) + } + return result + default: + var result [][]string + for _, tail := range variants[len(variants)-1] { + for _, p := range permute(variants[:len(variants)-1]) { + result = append(result, append(p, tail)) + } + } + return result + } +} + +func expandListExpression(expr string) []string { + var result []string + for _, part := range strings.Split(expr, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + isRange := true + var from, to int + + rangeParts := strings.SplitN(part, "..", 2) + if len(rangeParts) != 2 { + isRange = false + } + + var err error + if isRange { + from, err = strconv.Atoi(rangeParts[0]) + if err != nil { + isRange = false + } + } + if isRange { + to, err = strconv.Atoi(rangeParts[1]) + if err != nil { + isRange = false + } + } + + if isRange { + for i := from; i <= to; i++ { + result = append(result, strconv.Itoa(i)) + } + } else { + result = append(result, part) + } + } + return result +} + func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, resDefs map[string]client.ResourceDefinition, dependencies map[string][]client.Dependency, @@ -683,7 +791,7 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, for e := queue.Front(); e != nil; e = e.Next() { parent := e.Value.(*Block) - deps := filterDependencies(dependencies, parent.dependency.Child, flow, useDestructionSelector) + deps := listDependencies(dependencies, parent.dependency.Child, flow, useDestructionSelector, replicaContext) for _, dep := range deps { if parent.scheduledResource != nil && strings.HasPrefix(parent.scheduledResource.Key(), "flow/") { diff --git a/pkg/scheduler/dependency_graph_test.go b/pkg/scheduler/dependency_graph_test.go index 9cc7068..07ae319 100644 --- a/pkg/scheduler/dependency_graph_test.go +++ b/pkg/scheduler/dependency_graph_test.go @@ -15,6 +15,7 @@ package scheduler import ( + "strings" "testing" "github.com/Mirantis/k8s-AppController/pkg/client" @@ -319,3 +320,76 @@ func TestDependencyToFlowMatching(t *testing.T) { } } } + +// TestPermute tests permute function +func TestPermute(t *testing.T) { + alphabets := [][]string{ + {"1", "2", "3"}, + {"+", "-"}, + {"a", "b"}, + {"="}, + } + + expected := map[string]bool{ + "1+a=": true, + "1+b=": true, + "1-a=": true, + "1-b=": true, + "2+a=": true, + "2+b=": true, + "2-a=": true, + "2-b=": true, + "3+a=": true, + "3+b=": true, + "3-a=": true, + "3-b=": true, + } + permutations := permute(alphabets) + for _, combination := range permutations { + combinationStr := strings.Join(combination, "") + if !expected[combinationStr] { + t.Errorf("unexpected combination %s", combinationStr) + } else { + delete(expected, combinationStr) + } + } + if len(expected) != 0 { + t.Error("not all combinations were generated") + } + + alphabets = append(alphabets, make([]string, 0)) + if len(permute(alphabets)) != 0 { + t.Error("empty alphabet didin't result in empty permutation list") + } +} + +// TestExpendListExpression tests list expression translation to list of strings +func TestExpendListExpression(t *testing.T) { + table := map[string][]string{ + "1": {"1"}, + "1..5": {"1", "2", "3", "4", "5"}, + "2..-1": {}, + "a, b": {"a", "b"}, + "a, b, 2..4": {"a", "b", "2", "3", "4"}, + "-1..1, 2..4, x": {"-1", "0", "1", "2", "3", "4", "x"}, + "a..b": {"a..b"}, + "..": {".."}, + "1...3": {"1...3"}, + "1..b": {"1..b"}, + "a..b, 1..3": {"a..b", "1", "2", "3"}, + "a..b, c..d": {"a..b", "c..d"}, + "": {}, + } + for expr, expected := range table { + result := expandListExpression(expr) + if len(result) != len(expected) { + t.Errorf("unexpected result length for expression %s: %d != %d", expr, len(result), len(expected)) + } else { + for i := range expected { + if expected[i] != result[i] { + t.Errorf("invalid entry %d for expression %s: %s != %s", i, expr, expected[i], result[i]) + } + } + } + } +} diff --git a/pkg/scheduler/flows_test.go b/pkg/scheduler/flows_test.go index 464865c..700e7e0 100644 --- a/pkg/scheduler/flows_test.go +++ b/pkg/scheduler/flows_test.go @@ -1367,3 +1367,102 @@ func TestSyncOnVoidResource(t *testing.T) { depGraph.Deploy(stopChan) ensureReplicas(c, t, replicaCount, replicaCount) } + +// TestConsumeReplicatedFlow tests case, where each replica of the outer flow consumes N replicas of another flow +// by replicating dependency which leads to the consumed flow +func TestConsumeReplicatedFlow(t *testing.T) { + dep := mocks.MakeDependency("flow/outer", "flow/inner/$AC_NAME-$i", "flow=outer") + dep.GenerateFor = map[string]string{"i": "1..3"} + + c := mocks.NewClient( + mocks.MakeFlow("inner"), + mocks.MakeFlow("outer"), + mocks.MakeResourceDefinition("job/ready-$AC_NAME"), + dep, + mocks.MakeDependency("flow/inner", "job/ready-$AC_NAME", "flow=inner"), + ) + depGraph, err := New(c, nil, 0).BuildDependencyGraph( + interfaces.DependencyGraphOptions{ReplicaCount: 2, FlowName: "outer"}) + if err != nil { + t.Fatal(err) + } + stopChan := make(chan struct{}) + depGraph.Deploy(stopChan) + + ensureReplicas(c, t, 2*3, 3*2+2) +} + +// TestComplexDependencyReplication tests complex dependency generation over two list expressions +func TestComplexDependencyReplication(t *testing.T) { + dep := mocks.MakeDependency("flow/test", "job/ready-$x-$y", "flow=test") + dep.GenerateFor = map[string]string{ + "x": "1..3, 8..9", + "y": "a, b", + } + + c := mocks.NewClient( + mocks.MakeFlow("test"), + mocks.MakeResourceDefinition("job/ready-$x-$y"), + dep, + ) + depGraph, err := New(c, nil, 0).BuildDependencyGraph( + interfaces.DependencyGraphOptions{ReplicaCount: 1, FlowName: "test"}) + if err != nil { + t.Fatal(err) + } + stopChan := make(chan struct{}) + depGraph.Deploy(stopChan) + + expectedJobNames := map[string]bool{ + "ready-1-a": true, + "ready-2-a": true, + "ready-3-a": true, + "ready-8-a": true, + "ready-9-a": true, + "ready-1-b": true, + "ready-2-b": true, + "ready-3-b": true, + "ready-8-b": true, + "ready-9-b": true, + } + jobs := ensureReplicas(c, t, len(expectedJobNames), 1) + for _, j := range jobs { + if !expectedJobNames[j.Name] { + t.Errorf("unexpected job %s", j.Name) + } else { + delete(expectedJobNames, j.Name) + } + } + if len(expectedJobNames) != 0 { + t.Error("not all jobs were found") + } +} + +// TestDynamicDependencyReplication tests that variables can be used in list expressions used for dependency replication +func TestDynamicDependencyReplication(t *testing.T) { + flow := mocks.MakeFlow("test") + flow.Flow.Parameters = map[string]client.FlowParameter{ + "replicaCount": mocks.MakeFlowParameter("1"), + } + + dep := mocks.MakeDependency("flow/test", "job/ready-$index", "flow=test") + dep.GenerateFor = map[string]string{ + "index": "1..$replicaCount", + } + + c := mocks.NewClient( + flow, + mocks.MakeResourceDefinition("job/ready-$index"), + dep, + ) + depGraph, err := New(c, nil, 0).BuildDependencyGraph( + interfaces.DependencyGraphOptions{ReplicaCount: 1, FlowName: "test", + Args: map[string]string{"replicaCount": "7"}}) + if err != nil { + t.Fatal(err) + } + stopChan := make(chan struct{}) + depGraph.Deploy(stopChan) + + ensureReplicas(c, t, 7, 1) +} From 4a8d7b0e84bf148d6e32b3032c76177a7ae8cb9e Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Wed, 24 May 2017 22:27:27 -0700 Subject: [PATCH 2/6] Sequential flow replication For sequential flow, each next replica is attached to the leafs of previous one so that they will be deployed sequentially --- docs/flows.md | 9 ++ examples/etcd/README.md | 3 +- examples/etcd/resdefs/scale-flow.yaml | 2 + pkg/client/flows.go | 3 + pkg/resources/flow.go | 13 +- pkg/scheduler/dependency_graph.go | 167 +++++++++++++++++++++----- pkg/scheduler/flows_test.go | 82 +++++++++++++ 7 files changed, 237 insertions(+), 42 deletions(-) diff --git a/docs/flows.md b/docs/flows.md index 36d82d1..eb53890 100644 --- a/docs/flows.md +++ b/docs/flows.md @@ -63,6 +63,7 @@ destruction: replicaSpace: optional-name exported: true +sequential: true parameters: parameterName1: @@ -351,6 +352,14 @@ another flow will "see" only its own replicas so the `Flow` resource can always However, when the flow is run independently, it will not have any context and thus query replicas based on replica-space alone, which means it will get all the replicas from all contexts. +### Sequential flows + +By default, if flow has more than one replica, generated dependency graph would have each replica subgraph attached +to the graph root vertex (the `Flow` vertex). When deployed, resources of all replicas are going to be created in +parallel. However, in some cases it is desired that replicas be deployed sequentially, one by one. This can be achieved +by setting `sequential` attribute of the `Flow` to `true`. For sequential flows each replica roots get attached to the +leaf vertices of previous one. + ## Scheduling flow deployments When user runs `kubeac run something` the deployment does not happen immediately (unless there is also a `--deploy` diff --git a/examples/etcd/README.md b/examples/etcd/README.md index cd945de..22fe40c 100644 --- a/examples/etcd/README.md +++ b/examples/etcd/README.md @@ -26,8 +26,7 @@ If omitted, `etcd` name is used by default. `kubectl exec k8s-appcontroller kubeac run etcd-scale -n +1 --arg clusterName=my-cluster` `-n +1` - adds one node to the cluster. Use `-n -1` to scale the cluster down by one node. In this case the last -added node is going to be deleted. At the moment it is only possible to scale cluster up by one node at a time. -However, any number of nodes can be removed. Note, that this can also remove nodes created upon initial deployment. +added node is going to be deleted. This flow can also remove nodes created upon initial deployment. `--arg clusterName=my-cluster` - name of the cluster to scale (`etcd` if not specified). diff --git a/examples/etcd/resdefs/scale-flow.yaml b/examples/etcd/resdefs/scale-flow.yaml index 4394b4b..4fee091 100644 --- a/examples/etcd/resdefs/scale-flow.yaml +++ b/examples/etcd/resdefs/scale-flow.yaml @@ -4,6 +4,8 @@ metadata: name: etcd-scale exported: true +sequential: true + construction: flow: etcd-scale destruction: diff --git a/pkg/client/flows.go b/pkg/client/flows.go index 04b4251..55df049 100644 --- a/pkg/client/flows.go +++ b/pkg/client/flows.go @@ -46,6 +46,9 @@ type Flow struct { // can only be triggered by other flows (including DEFAULT flow which is exported by-default) Exported bool `json:"exported,omitempty"` + // Flow replicas must be deployed sequentially, one by one + Sequential bool `json:"sequential,omitempty"` + // Parameters that the flow can accept (i.e. valid inputs for the flow) Parameters map[string]FlowParameter `json:"parameters,omitempty"` diff --git a/pkg/resources/flow.go b/pkg/resources/flow.go index 4f7c0ce..9b11f84 100644 --- a/pkg/resources/flow.go +++ b/pkg/resources/flow.go @@ -78,20 +78,13 @@ func (f *flow) buildDependencyGraph(replicaCount int, silent bool) (interfaces.D args[arg] = val } } - fixedNumberOfReplicas := false - if replicaCount > 0 { - fixedNumberOfReplicas = f.context.Graph().Options().FixedNumberOfReplicas - } else if replicaCount == 0 { - fixedNumberOfReplicas = true - replicaCount = -1 - } options := interfaces.DependencyGraphOptions{ FlowName: f.originalName, Args: args, FlowInstanceName: f.context.GetArg("AC_ID"), ReplicaCount: replicaCount, Silent: silent, - FixedNumberOfReplicas: fixedNumberOfReplicas, + FixedNumberOfReplicas: true, } graph, err := f.context.Scheduler().BuildDependencyGraph(options) @@ -131,7 +124,7 @@ func (f *flow) Create() error { // Delete is called during dlow destruction which can happen only once while Create ensures that at least one flow // replica exists, and as such can be called any number of times func (f flow) Delete() error { - graph, err := f.buildDependencyGraph(-1, false) + graph, err := f.buildDependencyGraph(0, false) if err != nil { return err } @@ -145,7 +138,7 @@ func (f flow) Status(meta map[string]string) (interfaces.ResourceStatus, error) graph := f.currentGraph if graph == nil { var err error - graph, err = f.buildDependencyGraph(0, true) + graph, err = f.buildDependencyGraph(-1, true) if err != nil { return interfaces.ResourceError, err } diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index 737840d..30308bd 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -769,27 +769,29 @@ func expandListExpression(expr string) []string { return result } +type interimGraphVertex struct { + dependency client.Dependency + scheduledResource *ScheduledResource + parentContext *graphContext +} + func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, resDefs map[string]client.ResourceDefinition, dependencies map[string][]client.Dependency, flow *client.Flow, replicas []client.Replica, useDestructionSelector bool) error { - type Block struct { - dependency client.Dependency - scheduledResource *ScheduledResource - parentContext *graphContext - } - blocks := map[string][]*Block{} + var vertices [][]interimGraphVertex silent := rootContext.graph.Options().Silent for _, replica := range replicas { + var replicaVertices []interimGraphVertex replicaName := replica.ReplicaName() replicaContext := sched.prepareContext(rootContext, nil, replicaName) queue := list.New() - queue.PushFront(&Block{dependency: client.Dependency{Child: "flow/" + flow.Name}}) + queue.PushFront(interimGraphVertex{dependency: client.Dependency{Child: "flow/" + flow.Name}}) for e := queue.Front(); e != nil; e = e.Next() { - parent := e.Value.(*Block) + parent := e.Value.(interimGraphVertex) deps := listDependencies(dependencies, parent.dependency.Child, flow, useDestructionSelector, replicaContext) @@ -815,44 +817,149 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, } sr.usedInReplicas = []string{replicaName} - block := &Block{ + vertex := interimGraphVertex{ scheduledResource: sr, dependency: dep, parentContext: parentContext, } - - blocks[dep.Child] = append(blocks[dep.Child], block) + replicaVertices = append(replicaVertices, vertex) if parent.scheduledResource != nil { sr.Requires = append(sr.Requires, parent.scheduledResource.Key()) parent.scheduledResource.RequiredBy = append(parent.scheduledResource.RequiredBy, sr.Key()) sr.Meta[parent.dependency.Child] = dep.Meta } - queue.PushBack(block) + queue.PushBack(vertex) } } - for _, block := range blocks { - for _, entry := range block { - key := entry.scheduledResource.Key() - existingSr := rootContext.graph.graph[key] - if existingSr == nil { - if !silent { - log.Printf("Adding resource %s to the dependency graph flow %s", key, flow.Name) - } - rootContext.graph.graph[key] = entry.scheduledResource - } else { - sched.updateContext(existingSr.context, entry.parentContext, entry.dependency) - existingSr.Requires = append(existingSr.Requires, entry.scheduledResource.Requires...) - existingSr.RequiredBy = append(existingSr.RequiredBy, entry.scheduledResource.RequiredBy...) - existingSr.usedInReplicas = append(existingSr.usedInReplicas, entry.scheduledResource.usedInReplicas...) - for metaKey, metaValue := range entry.scheduledResource.Meta { - existingSr.Meta[metaKey] = metaValue - } + vertices = append(vertices, replicaVertices) + } + + if flow.Sequential { + sched.concatenateReplicas(vertices, rootContext, rootContext.graph.Options()) + } else { + sched.mergeReplicas(vertices, rootContext, rootContext.graph.Options()) + } + return nil +} + +func (sched *scheduler) mergeReplicas(vertices [][]interimGraphVertex, gc *graphContext, + options interfaces.DependencyGraphOptions) { + + for _, replicaVertices := range vertices { + sched.mergeInterimGraphVertices(replicaVertices, gc.graph.graph, options) + } +} + +func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc *graphContext, + options interfaces.DependencyGraphOptions) { + graph := gc.graph.graph + var previousReplicaGraph map[string]*ScheduledResource + for i, replicaVertices := range vertices { + replicaGraph := map[string]*ScheduledResource{} + sched.mergeInterimGraphVertices(replicaVertices, replicaGraph, options) + + if i > 0 { + correctDuplicateResources(graph, replicaGraph, i) + + for _, leafName := range getLeafs(previousReplicaGraph) { + for _, rootName := range getRoots(replicaGraph) { + root := replicaGraph[rootName] + leaf := previousReplicaGraph[leafName] + root.Requires = append(root.Requires, leafName) + leaf.RequiredBy = append(leaf.RequiredBy, rootName) } } } + previousReplicaGraph = replicaGraph + for key, value := range replicaGraph { + graph[key] = value + } + } +} + +func correctDuplicateResources(existingGraph, newGraph map[string]*ScheduledResource, index int) { + toReplace := map[string]*ScheduledResource{} + for key, sr := range newGraph { + if existingGraph[key] != nil { + toReplace[key] = sr + } + } + for key, sr := range toReplace { + sr.context.id = existingGraph[key].context.id + j := index + 1 + suffix := sr.suffix + for { + sr.suffix = fmt.Sprintf("%s #%d", suffix, j) + if existingGraph[sr.Key()] == nil { + break + } + j++ + } + for _, rKey := range sr.RequiredBy { + requires := newGraph[rKey].Requires + for i, rKey2 := range requires { + if rKey2 == key { + requires[i] = sr.Key() + break + } + } + } + for _, rKey := range sr.Requires { + requiredBy := newGraph[rKey].RequiredBy + for i, rKey2 := range requiredBy { + if rKey2 == key { + requiredBy[i] = sr.Key() + break + } + } + } + delete(newGraph, key) + newGraph[sr.Key()] = sr + } +} + +func getRoots(graph map[string]*ScheduledResource) []string { + var result []string + for key, sr := range graph { + if len(sr.Requires) == 0 { + result = append(result, key) + } + } + return result +} + +func getLeafs(graph map[string]*ScheduledResource) []string { + var result []string + for key, sr := range graph { + if len(sr.RequiredBy) == 0 { + result = append(result, key) + } + } + return result +} + +func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, graph map[string]*ScheduledResource, + options interfaces.DependencyGraphOptions) { + + for _, entry := range vertices { + key := entry.scheduledResource.Key() + existingSr := graph[key] + if existingSr == nil { + if !options.Silent { + log.Printf("Adding resource %s to the dependency graph flow %s", key, options.FlowName) + } + graph[key] = entry.scheduledResource + } else { + sched.updateContext(existingSr.context, entry.parentContext, entry.dependency) + existingSr.Requires = append(existingSr.Requires, entry.scheduledResource.Requires...) + existingSr.RequiredBy = append(existingSr.RequiredBy, entry.scheduledResource.RequiredBy...) + existingSr.usedInReplicas = append(existingSr.usedInReplicas, entry.scheduledResource.usedInReplicas...) + for metaKey, metaValue := range entry.scheduledResource.Meta { + existingSr.Meta[metaKey] = metaValue + } + } } - return nil } // getResourceDestructors builds a list of functions, each of them delete one of replica resources diff --git a/pkg/scheduler/flows_test.go b/pkg/scheduler/flows_test.go index 700e7e0..1bc48aa 100644 --- a/pkg/scheduler/flows_test.go +++ b/pkg/scheduler/flows_test.go @@ -1466,3 +1466,85 @@ func TestDynamicDependencyReplication(t *testing.T) { ensureReplicas(c, t, 7, 1) } + +// TestSequentialReplication tests that resources of sequentially replicated flows create in right order +func TestSequentialReplication(t *testing.T) { + replicaCount := 3 + flow := mocks.MakeFlow("test") + flow.Flow.Sequential = true + + c, fake := mocks.NewClientWithFake( + flow, + mocks.MakeResourceDefinition("pod/ready-$AC_NAME"), + mocks.MakeResourceDefinition("secret/secret"), + mocks.MakeResourceDefinition("job/ready-$AC_NAME"), + mocks.MakeDependency("flow/test", "pod/ready-$AC_NAME", "flow=test"), + mocks.MakeDependency("pod/ready-$AC_NAME", "secret/secret", "flow=test"), + mocks.MakeDependency("secret/secret", "job/ready-$AC_NAME", "flow=test"), + ) + + stopChan := make(chan struct{}) + var deployed []string + fake.PrependReactor("create", "*", + func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + resource := action.GetResource().Resource + if resource != "replica" { + deployed = append(deployed, resource) + } + + return false, nil, nil + }) + + depGraph, err := New(c, nil, 0).BuildDependencyGraph( + interfaces.DependencyGraphOptions{ReplicaCount: replicaCount, FlowName: "test"}) + if err != nil { + t.Fatal(err) + } + + graph := depGraph.(*dependencyGraph).graph + if len(graph) != 3*replicaCount { + t.Error("wrong dependency graph length") + } + + depGraph.Deploy(stopChan) + expected := []string{"pods", "secrets", "jobs", "pods", "jobs", "pods", "jobs"} + if len(deployed) != len(expected) { + t.Fatal("invalid resource sequence", deployed) + } + for i, r := range deployed { + if expected[i] != r { + t.Fatal("invalid resource sequence") + } + } + + ensureReplicas(c, t, replicaCount, replicaCount) +} + +// TestSequentialReplicationWithSharedFlow tests that flow consumed as a resource shared by replicas of +// sequentially replicated flow deployed only once +func TestSequentialReplicationWithSharedFlow(t *testing.T) { + replicaCount := 3 + flow := mocks.MakeFlow("outer") + flow.Flow.Sequential = true + + c := mocks.NewClient( + flow, + mocks.MakeFlow("inner"), + mocks.MakeResourceDefinition("job/ready-a$AC_NAME"), + mocks.MakeResourceDefinition("job/ready-b$AC_NAME"), + mocks.MakeDependency("flow/outer", "flow/inner", "flow=outer"), + mocks.MakeDependency("flow/inner", "job/ready-a$AC_NAME", "flow=outer"), + mocks.MakeDependency("flow/inner", "job/ready-b$AC_NAME", "flow=inner"), + ) + + stopChan := make(chan struct{}) + + depGraph, err := New(c, nil, 0).BuildDependencyGraph( + interfaces.DependencyGraphOptions{ReplicaCount: replicaCount, FlowName: "outer"}) + if err != nil { + t.Fatal(err) + } + + depGraph.Deploy(stopChan) + ensureReplicas(c, t, replicaCount+1, replicaCount+1) +} From 2a0771b1d722c17bfdb1fba44b1023c5a7d1e1dd Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Thu, 25 May 2017 17:50:49 -0700 Subject: [PATCH 3/6] Better handling of deployment cancellation * stopChan is now passed to the graph finalizers so that deployment can be canceled on the final stages * never write to stopChan. The only correct way to cancel deployment is to close the channel * pass nil instead of real chanel for unit tests that do not cancel deployment --- pkg/scheduler/controller.go | 1 - pkg/scheduler/dependency_graph.go | 41 +++++----- pkg/scheduler/flows_test.go | 122 +++++++++++------------------- pkg/scheduler/frontend.go | 9 +-- pkg/scheduler/scheduler.go | 22 ++++-- pkg/scheduler/scheduler_test.go | 10 +-- 6 files changed, 88 insertions(+), 117 deletions(-) diff --git a/pkg/scheduler/controller.go b/pkg/scheduler/controller.go index f92a733..264e398 100644 --- a/pkg/scheduler/controller.go +++ b/pkg/scheduler/controller.go @@ -140,7 +140,6 @@ func deployTasks(taskQueue *list.List, mutex *sync.Mutex, cond *sync.Cond, clien cond.L.Unlock() if processing != nil { if abortChan != nil { - abortChan <- struct{}{} close(abortChan) abortChan = nil processing = nil diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index 30308bd..634c8dc 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -37,7 +37,7 @@ type dependencyGraph struct { graph map[string]*ScheduledResource scheduler *scheduler graphOptions interfaces.DependencyGraphOptions - finalizer func() + finalizer func(stopChan <-chan struct{}) } type graphContext struct { @@ -963,9 +963,10 @@ func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, } // getResourceDestructors builds a list of functions, each of them delete one of replica resources -func getResourceDestructors(construction, destruction *dependencyGraph, replicaMap map[string]client.Replica, failed *chan *ScheduledResource) []func() bool { - var destructors []func() bool +func getResourceDestructors(construction, destruction *dependencyGraph, replicaMap map[string]client.Replica, + failed *chan *ScheduledResource) []func(<-chan struct{}) bool { + var destructors []func(<-chan struct{}) bool for _, depGraph := range [2]*dependencyGraph{construction, destruction} { for _, resource := range depGraph.graph { resourceCanBeDeleted := true @@ -983,8 +984,8 @@ func getResourceDestructors(construction, destruction *dependencyGraph, replicaM return destructors } -func getDestructorFunc(resource *ScheduledResource, failed *chan *ScheduledResource) func() bool { - return func() bool { +func getDestructorFunc(resource *ScheduledResource, failed *chan *ScheduledResource) func(<-chan struct{}) bool { + return func(<-chan struct{}) bool { res := deleteResource(resource) if res != nil { *failed <- resource @@ -995,10 +996,12 @@ func getDestructorFunc(resource *ScheduledResource, failed *chan *ScheduledResou } // deleteReplicaResources invokes resources destructors and deletes replicas for which 100% of resources were deleted -func deleteReplicaResources(sched *scheduler, destructors []func() bool, replicaMap map[string]client.Replica, failed *chan *ScheduledResource) { +func deleteReplicaResources(sched *scheduler, destructors []func(<-chan struct{}) bool, replicaMap map[string]client.Replica, + failed *chan *ScheduledResource, stopChan <-chan struct{}) { + *failed = make(chan *ScheduledResource, len(destructors)) defer close(*failed) - deleted := runConcurrently(destructors, sched.concurrency) + deleted := runConcurrently(destructors, sched.concurrency, stopChan) failedReplicas := map[string]bool{} if !deleted { log.Println("Some of resources were not deleted") @@ -1015,7 +1018,7 @@ readFailed: break readFailed } } - var deleteReplicaFuncs []func() bool + var deleteReplicaFuncs []func(<-chan struct{}) bool for replicaName, replicaObject := range replicaMap { if _, found := failedReplicas[replicaName]; found { @@ -1023,7 +1026,7 @@ readFailed: } replicaNameCopy := replicaName replicaObjectCopy := replicaObject - deleteReplicaFuncs = append(deleteReplicaFuncs, func() bool { + deleteReplicaFuncs = append(deleteReplicaFuncs, func(<-chan struct{}) bool { log.Printf("%s flow: Deleting replica %s", replicaObjectCopy.FlowName, replicaNameCopy) err := sched.client.Replicas().Delete(replicaObjectCopy.Name) if err != nil { @@ -1033,12 +1036,12 @@ readFailed: }) } - if deleteReplicaFuncs != nil && !runConcurrently(deleteReplicaFuncs, sched.concurrency) { + if deleteReplicaFuncs != nil && !runConcurrently(deleteReplicaFuncs, sched.concurrency, stopChan) { log.Println("Some of flow replicas were not deleted") } } -func (sched *scheduler) composeDeletingFinalizer(construction, destruction *dependencyGraph, replicas []client.Replica) func() { +func (sched *scheduler) composeDeletingFinalizer(construction, destruction *dependencyGraph, replicas []client.Replica) func(<-chan struct{}) { replicaMap := map[string]client.Replica{} for _, replica := range replicas { replicaMap[replica.ReplicaName()] = replica @@ -1047,14 +1050,14 @@ func (sched *scheduler) composeDeletingFinalizer(construction, destruction *depe var failed chan *ScheduledResource destructors := getResourceDestructors(construction, destruction, replicaMap, &failed) - return func() { + return func(stopChan <-chan struct{}) { log.Print("Performing resource cleanup") - deleteReplicaResources(sched, destructors, replicaMap, &failed) + deleteReplicaResources(sched, destructors, replicaMap, &failed, stopChan) } } -func makeAcknowledgeReplicaFunc(replica client.Replica, api client.ReplicasInterface) func() bool { - return func() bool { +func makeAcknowledgeReplicaFunc(replica client.Replica, api client.ReplicasInterface) func(<-chan struct{}) bool { + return func(<-chan struct{}) bool { replica.Deployed = true log.Printf("%s flow: Marking replica %s as deployed", replica.FlowName, replica.ReplicaName()) if err := api.Update(&replica); err != nil { @@ -1065,16 +1068,16 @@ func makeAcknowledgeReplicaFunc(replica client.Replica, api client.ReplicasInter } } -func (sched *scheduler) composeAcknowledgingFinalizer(replicas []client.Replica) func() { - var funcs []func() bool +func (sched *scheduler) composeAcknowledgingFinalizer(replicas []client.Replica) func(<-chan struct{}) { + var funcs []func(<-chan struct{}) bool for _, replica := range replicas { if !replica.Deployed { funcs = append(funcs, makeAcknowledgeReplicaFunc(replica, sched.client.Replicas())) } } - return func() { - if !runConcurrently(funcs, sched.concurrency) { + return func(stopChan <-chan struct{}) { + if !runConcurrently(funcs, sched.concurrency, stopChan) { log.Println("Some of the replicas were not updated!") } } diff --git a/pkg/scheduler/flows_test.go b/pkg/scheduler/flows_test.go index 1bc48aa..5713179 100644 --- a/pkg/scheduler/flows_test.go +++ b/pkg/scheduler/flows_test.go @@ -144,8 +144,7 @@ func TestTriggerFlowIndependently(t *testing.T) { t.Fatal("Job list is not empty") } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err = c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -217,8 +216,7 @@ func TestTriggerOneFlowFromAnother(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -262,8 +260,7 @@ func TestParameterPassing(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -306,8 +303,7 @@ func TestMultipathParameterPassing(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -354,8 +350,7 @@ func TestParametrizedFlow(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -396,8 +391,7 @@ func TestAcNameParameter(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) replicas, err := c.Replicas().List(api.ListOptions{}) if err != nil { @@ -444,8 +438,7 @@ func TestUseUndeclaredFlowParameter(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -538,8 +531,7 @@ func TestParameterPassingBetweenFlows(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -596,8 +588,7 @@ func TestReplication(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, 2*replicaCount, replicaCount) @@ -625,8 +616,7 @@ func TestReplicationWithSharedResources(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, replicaCount+1, replicaCount) @@ -664,8 +654,7 @@ func TestReplicationScaleUp(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, initialReplicaCount, initialReplicaCount) @@ -679,7 +668,7 @@ func TestReplicationScaleUp(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, adjustedReplicaCount, adjustedReplicaCount) @@ -693,7 +682,7 @@ func TestReplicationScaleUp(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, adjustedReplicaCount+replicaCountDelta, adjustedReplicaCount+replicaCountDelta) @@ -718,8 +707,7 @@ func TestNoOpReplication(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) } @@ -749,8 +737,7 @@ func TestCompositionFlowReplication(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, 4*replicaCount, 2*replicaCount) @@ -802,8 +789,7 @@ func TestDestruction(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 6, 5) @@ -812,7 +798,7 @@ func TestDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 4, 3) @@ -821,7 +807,7 @@ func TestDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, 2, 1) if jobs[0].Name != "ready-b" && jobs[1].Name != "ready-b" { @@ -833,7 +819,7 @@ func TestDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) } @@ -859,8 +845,7 @@ func TestCompositeFlowDestruction(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 15, 10) @@ -869,7 +854,7 @@ func TestCompositeFlowDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 9, 6) @@ -878,7 +863,7 @@ func TestCompositeFlowDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 3, 2) @@ -887,7 +872,7 @@ func TestCompositeFlowDestruction(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) } @@ -944,8 +929,7 @@ func TestCleanupResources(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 1) if !(aCreated && !aDeleted && !bCreated && !bDeleted && !cCreated && !cDeleted) { @@ -958,7 +942,7 @@ func TestCleanupResources(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) if !(aCreated && aDeleted && bCreated && bDeleted && cCreated && cDeleted) { @@ -991,8 +975,7 @@ func TestSharedReplicaSpace(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 2, 1) @@ -1002,7 +985,7 @@ func TestSharedReplicaSpace(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) @@ -1012,7 +995,7 @@ func TestSharedReplicaSpace(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 2, 1) @@ -1022,7 +1005,7 @@ func TestSharedReplicaSpace(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0) } @@ -1043,8 +1026,7 @@ func TestDeleteExistingResources(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 1) @@ -1053,7 +1035,7 @@ func TestDeleteExistingResources(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 0, "invalid attempt to delete external resources") @@ -1063,7 +1045,7 @@ func TestDeleteExistingResources(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 1) @@ -1072,7 +1054,7 @@ func TestDeleteExistingResources(t *testing.T) { if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 0, 0, "valid attempt to delete external resources") } @@ -1103,8 +1085,7 @@ func TestCleanupResourcesErrorHandling(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 2, 2) @@ -1114,7 +1095,7 @@ func TestCleanupResourcesErrorHandling(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 1) } @@ -1137,7 +1118,7 @@ func TestDeploymentRecoveryForRelativeReplicaCount(t *testing.T) { func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { if !prevented { prevented = true - stopChan <- struct{}{} + close(stopChan) return true, nil, errors.New("resource cannot be created") } return false, nil, nil @@ -1166,7 +1147,7 @@ func TestDeploymentRecoveryForRelativeReplicaCount(t *testing.T) { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 1, 2) replicas, _ = c.Replicas().List(api.ListOptions{}) for _, r := range replicas.Items { @@ -1194,9 +1175,7 @@ func TestMultiParentFlow(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) - + depGraph.Deploy(nil) ensureReplicas(c, t, 5, 3) } @@ -1219,8 +1198,7 @@ func TestMultiParentFlowWithSuffix(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 6, 4) } @@ -1254,8 +1232,7 @@ func TestMultipleFlowCalls(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs := ensureReplicas(c, t, 4, 3) jobNames := map[string]bool{ @@ -1304,8 +1281,7 @@ func TestMultipathParameterPassingWithSuffix(t *testing.T) { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) jobs, err := c.Jobs().List(api_v1.ListOptions{}) if err != nil { @@ -1386,8 +1362,7 @@ func TestConsumeReplicatedFlow(t *testing.T) { if err != nil { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 2*3, 3*2+2) } @@ -1410,8 +1385,7 @@ func TestComplexDependencyReplication(t *testing.T) { if err != nil { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) expectedJobNames := map[string]bool{ "ready-1-a": true, @@ -1461,8 +1435,7 @@ func TestDynamicDependencyReplication(t *testing.T) { if err != nil { t.Fatal(err) } - stopChan := make(chan struct{}) - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, 7, 1) } @@ -1483,7 +1456,6 @@ func TestSequentialReplication(t *testing.T) { mocks.MakeDependency("secret/secret", "job/ready-$AC_NAME", "flow=test"), ) - stopChan := make(chan struct{}) var deployed []string fake.PrependReactor("create", "*", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { @@ -1506,7 +1478,7 @@ func TestSequentialReplication(t *testing.T) { t.Error("wrong dependency graph length") } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) expected := []string{"pods", "secrets", "jobs", "pods", "jobs", "pods", "jobs"} if len(deployed) != len(expected) { t.Fatal("invalid resource sequence", deployed) @@ -1537,14 +1509,12 @@ func TestSequentialReplicationWithSharedFlow(t *testing.T) { mocks.MakeDependency("flow/inner", "job/ready-b$AC_NAME", "flow=inner"), ) - stopChan := make(chan struct{}) - depGraph, err := New(c, nil, 0).BuildDependencyGraph( interfaces.DependencyGraphOptions{ReplicaCount: replicaCount, FlowName: "outer"}) if err != nil { t.Fatal(err) } - depGraph.Deploy(stopChan) + depGraph.Deploy(nil) ensureReplicas(c, t, replicaCount+1, replicaCount+1) } diff --git a/pkg/scheduler/frontend.go b/pkg/scheduler/frontend.go index dffcd92..d49973b 100644 --- a/pkg/scheduler/frontend.go +++ b/pkg/scheduler/frontend.go @@ -161,14 +161,7 @@ func Deploy(sched interfaces.Scheduler, options interfaces.DependencyGraphOption if err != nil { return "", err } - var ch <-chan struct{} - if stopChan == nil { - ch := make(chan struct{}) - defer close(ch) - } else { - ch = stopChan - } - depGraph.Deploy(ch) + depGraph.Deploy(stopChan) } else { log.Printf("Scheduling deployment of %s flow", options.FlowName) var err error diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 43e0031..2970aee 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -353,7 +353,7 @@ func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) { } } if depGraph.finalizer != nil { - depGraph.finalizer() + depGraph.finalizer(stopChan) } // TODO Make sure every KO gets created eventually } @@ -418,7 +418,7 @@ func (depGraph dependencyGraph) GetStatus() (interfaces.DeploymentStatus, interf return status, deploymentReport } -func runConcurrently(funcs []func() bool, concurrency int) bool { +func runConcurrently(funcs []func(<-chan struct{}) bool, concurrency int, stopChan <-chan struct{}) bool { if concurrency < 1 { concurrency = len(funcs) } @@ -430,12 +430,18 @@ func runConcurrently(funcs []func() bool, concurrency int) bool { for _, f := range funcs { sem <- true - go func(foo func() bool) { - defer func() { <-sem }() - if !foo() { - result = false - } - }(f) + select { + case <-stopChan: + result = false + <-sem + default: + go func(foo func(<-chan struct{}) bool) { + defer func() { <-sem }() + if !foo(stopChan) { + result = false + } + }(f) + } } for i := 0; i < cap(sem); i++ { diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 2176b3f..f4117ad 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -546,18 +546,18 @@ func TestGraph(t *testing.T) { } } -func makeTaskFunc(i int32, res bool, acc *int32) func() bool { - return func() bool { +func makeTaskFunc(i int32, res bool, acc *int32) func(<-chan struct{}) bool { + return func(<-chan struct{}) bool { time.Sleep(time.Second / 10) atomic.AddInt32(acc, i) return res } } -func runTaskFuncs(t *testing.T, funcs []func() bool, concurrency int, expectedSum int32, expectedResult bool, threshold float64, acc *int32) { +func runTaskFuncs(t *testing.T, funcs []func(<-chan struct{}) bool, concurrency int, expectedSum int32, expectedResult bool, threshold float64, acc *int32) { *acc = 0 before := time.Now() - res := runConcurrently(funcs, concurrency) + res := runConcurrently(funcs, concurrency, nil) after := time.Now() if *acc != expectedSum { t.Errorf("runConcurrently failed: %d != %d", expectedSum, acc) @@ -576,7 +576,7 @@ func runTaskFuncs(t *testing.T, funcs []func() bool, concurrency int, expectedSu // TestRunConcurrently tests runConcurrently function which runs list of concurrent tasks func TestRunConcurrently(t *testing.T) { var acc int32 - var funcs []func() bool + var funcs []func(<-chan struct{}) bool for i := int32(1); i <= 50; i++ { funcs = append(funcs, makeTaskFunc(i, true, &acc)) } From 06a4167df5ea4fe6bc7874ac670cff8b585a2859 Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Sun, 28 May 2017 16:38:41 -0700 Subject: [PATCH 4/6] Deployment error handling was reworked In some cases deployment could hang forever: * if graph vertex depends on vertex which will never be created (because of timeout or permanent error) * if graph vertex was set to be created only if parent fails, but it didn't Because deployment algorithm waits for all vertexes to be created if any one of them remained blocked, Deploy() is going to run forever blocking AC process from handling other deployment tasks. Also on-error processing could be triggered by intermediate resource status. For example it could happen, if resource status was obtained prior to resource.Create() call. Another case if resource was set to have several deployment attempts. If the first attempt fails on-error dependency becomes activated, but on the second attempt the deployment may succeed. This commit reworks error handling: * Resources which cannot be created or time out, marked with error. * Resources, that depend on failed resources also fail * Thus all graph vertexes eventually become unblocked and deployment finishes * on-error handling is done based on the final resource status/error * Deploy() now returns true if deployment succeeded. Deployment fails if any of resources (and their dependents) went into failed state except for cases, where they were skipped because all dependencies had on-error meta and parent resource didn't fail Also: * e2e tests were updated so that most of them wait for deployment to finish rather than just waiting for resource status. Thus now they also test that deployment doesn't hang * Graph vertex type (ScheduledResource) and it fields are not exported anymore. The same goes for some of dependency graph methods. * wait() method doesn't create unnecessary goroutines and channels --- e2e/basic_test.go | 6 +- e2e/example_runner.go | 10 +- e2e/flows_test.go | 4 +- e2e/utils/appcmanager.go | 29 +++- pkg/interfaces/interfaces.go | 2 +- pkg/scheduler/dependency_graph.go | 80 ++++----- pkg/scheduler/frontend.go | 7 +- pkg/scheduler/scheduler.go | 275 ++++++++++++++++-------------- pkg/scheduler/scheduler_test.go | 83 +++++---- 9 files changed, 263 insertions(+), 233 deletions(-) diff --git a/e2e/basic_test.go b/e2e/basic_test.go index c039163..2b3cfc1 100644 --- a/e2e/basic_test.go +++ b/e2e/basic_test.go @@ -56,7 +56,9 @@ var _ = Describe("Basic Suite", func() { }, } childPod := PodPause("child-pod") - framework.Connect(framework.WrapAndCreate(parentPod), framework.WrapAndCreate(childPod)) + framework.Connect( + framework.WrapWithMetaAndCreate(parentPod, map[string]interface{}{"timeout": 30}), + framework.WrapAndCreate(childPod)) framework.Run() testutils.WaitForPod(framework.Clientset, framework.Namespace.Name, parentPod.Name, "") time.Sleep(time.Second) @@ -104,7 +106,7 @@ var _ = Describe("Basic Suite", func() { By("Creating resource definition with single pod") pod1 := PodPause("pod1") framework.WrapAndCreate(pod1) - framework.Run() + framework.RunAsynchronously() framework.DeleteAppControllerPod() By("Verify that pod is consistently not found") Consistently(func() bool { diff --git a/e2e/example_runner.go b/e2e/example_runner.go index 06001a5..8f98139 100644 --- a/e2e/example_runner.go +++ b/e2e/example_runner.go @@ -104,14 +104,10 @@ func (f *examplesFramework) handleListCreation(ustList *runtime.UnstructuredList } } -func (f *examplesFramework) VerifyStatus(task string, options interfaces.DependencyGraphOptions) { +func (f *examplesFramework) VerifyStatus(options interfaces.DependencyGraphOptions) { var depReport report.DeploymentReport Eventually( func() bool { - _, err := f.Client.ConfigMaps().Get(task) - if err == nil { - return false - } status, r, err := scheduler.GetStatus(f.Client, nil, options) if err != nil { return false @@ -127,7 +123,7 @@ func (f *examplesFramework) CreateRunAndVerify(exampleName string, options inter By("Creating example " + exampleName) f.CreateExample(exampleName) By("Running appcontroller scheduler") - task := f.RunWithOptions(options) + f.RunWithOptions(options) By("Verifying status of deployment for example " + exampleName) - f.VerifyStatus(task, options) + f.VerifyStatus(options) } diff --git a/e2e/flows_test.go b/e2e/flows_test.go index 72ab376..d0f84fd 100644 --- a/e2e/flows_test.go +++ b/e2e/flows_test.go @@ -64,9 +64,9 @@ var _ = Describe("Flows Suite", func() { deleteOptions := interfaces.DependencyGraphOptions{ReplicaCount: 0, FixedNumberOfReplicas: true} By("Running appcontroller scheduler") - task := framework.RunWithOptions(deleteOptions) + framework.RunWithOptions(deleteOptions) By("Verifying status of deployment") - framework.VerifyStatus(task, deleteOptions) + framework.VerifyStatus(deleteOptions) framework.validateResourceCounts([]resourceCount{ {"replicas", "", false, 0}, diff --git a/e2e/utils/appcmanager.go b/e2e/utils/appcmanager.go index 266faf1..1e82e80 100644 --- a/e2e/utils/appcmanager.go +++ b/e2e/utils/appcmanager.go @@ -44,17 +44,38 @@ type AppControllerManager struct { } // Run runs dependency graph deployment with default settings -func (a *AppControllerManager) Run() string { - return a.RunWithOptions(interfaces.DependencyGraphOptions{MinReplicaCount: 1}) +func (a *AppControllerManager) Run() { + a.runDeployment(false, interfaces.DependencyGraphOptions{MinReplicaCount: 1}) +} + +// RunAsynchronously runs dependency graph deployment with default settings without waiting for deployment to complete +func (a *AppControllerManager) RunAsynchronously() { + a.runDeployment(true, interfaces.DependencyGraphOptions{MinReplicaCount: 1}) } // RunWithOptions runs dependency graph deployment with given settings -func (a *AppControllerManager) RunWithOptions(options interfaces.DependencyGraphOptions) string { +func (a *AppControllerManager) RunWithOptions(options interfaces.DependencyGraphOptions) { + a.runDeployment(false, options) +} + +// RunAsynchronouslyWithOptions runs dependency graph deployment with given settings without waiting for deployment to complete +func (a *AppControllerManager) RunAsynchronouslyWithOptions(options interfaces.DependencyGraphOptions) { + a.runDeployment(true, options) +} + +func (a *AppControllerManager) runDeployment(runAsync bool, options interfaces.DependencyGraphOptions) { sched := scheduler.New(a.Client, nil, 0) task, err := scheduler.Deploy(sched, options, false, nil) Expect(err).NotTo(HaveOccurred()) - return task + if !runAsync { + Eventually( + func() error { + _, err := a.Client.ConfigMaps().Get(task) + return err + }, + 300*time.Second, 5*time.Second).Should(HaveOccurred(), "Deployment job wasn't completed") + } } // DeleteAppControllerPod deletes pod, where AppController is running diff --git a/pkg/interfaces/interfaces.go b/pkg/interfaces/interfaces.go index 929d627..c470831 100644 --- a/pkg/interfaces/interfaces.go +++ b/pkg/interfaces/interfaces.go @@ -72,7 +72,7 @@ type DeploymentReport interface { // DependencyGraph represents operations on dependency graph type DependencyGraph interface { GetStatus() (DeploymentStatus, DeploymentReport) - Deploy(<-chan struct{}) + Deploy(<-chan struct{}) bool Options() DependencyGraphOptions } diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index 634c8dc..24bc045 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -34,7 +34,7 @@ import ( ) type dependencyGraph struct { - graph map[string]*ScheduledResource + graph map[string]*scheduledResource scheduler *scheduler graphOptions interfaces.DependencyGraphOptions finalizer func(stopChan <-chan struct{}) @@ -88,15 +88,15 @@ func (gc graphContext) Graph() interfaces.DependencyGraph { } // newScheduledResourceFor returns new scheduled resource for given resource in init state -func newScheduledResourceFor(r interfaces.Resource, suffix string, context *graphContext, existing bool) *ScheduledResource { - return &ScheduledResource{ - Started: false, - Ignored: false, - Error: nil, +func newScheduledResourceFor(r interfaces.Resource, suffix string, context *graphContext, existing bool) *scheduledResource { + return &scheduledResource{ + started: false, + ignored: false, + error: nil, Resource: r, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, - Existing: existing, + existing: existing, suffix: copier.EvaluateString(suffix, getArgFunc(context)), } } @@ -254,7 +254,7 @@ func isMapContainedIn(contained, containing map[string]string) bool { // newScheduledResource is a constructor for ScheduledResource func (sched scheduler) newScheduledResource(kind, name, suffix string, resDefs map[string]client.ResourceDefinition, - gc *graphContext, silent bool) (*ScheduledResource, error) { + gc *graphContext, silent bool) (*scheduledResource, error) { var r interfaces.Resource resourceTemplate, ok := resources.KindToResourceTemplate[kind] @@ -310,7 +310,7 @@ func keyParts(key string) (kind, name, suffix string, err error) { func newDependencyGraph(sched *scheduler, options interfaces.DependencyGraphOptions) *dependencyGraph { return &dependencyGraph{ - graph: make(map[string]*ScheduledResource), + graph: make(map[string]*scheduledResource), scheduler: sched, graphOptions: options, } @@ -635,8 +635,8 @@ func (sched *scheduler) BuildDependencyGraph(options interfaces.DependencyGraphO } for _, value := range depGraph.graph { - value.RequiredBy = unique(value.RequiredBy) - value.Requires = unique(value.Requires) + value.requiredBy = unique(value.requiredBy) + value.requires = unique(value.requires) value.usedInReplicas = unique(value.usedInReplicas) } @@ -771,7 +771,7 @@ func expandListExpression(expr string) []string { type interimGraphVertex struct { dependency client.Dependency - scheduledResource *ScheduledResource + scheduledResource *scheduledResource parentContext *graphContext } @@ -825,9 +825,9 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, replicaVertices = append(replicaVertices, vertex) if parent.scheduledResource != nil { - sr.Requires = append(sr.Requires, parent.scheduledResource.Key()) - parent.scheduledResource.RequiredBy = append(parent.scheduledResource.RequiredBy, sr.Key()) - sr.Meta[parent.dependency.Child] = dep.Meta + sr.requires = append(sr.requires, parent.scheduledResource.Key()) + parent.scheduledResource.requiredBy = append(parent.scheduledResource.requiredBy, sr.Key()) + sr.meta[parent.dependency.Child] = dep.Meta } queue.PushBack(vertex) } @@ -854,9 +854,9 @@ func (sched *scheduler) mergeReplicas(vertices [][]interimGraphVertex, gc *graph func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc *graphContext, options interfaces.DependencyGraphOptions) { graph := gc.graph.graph - var previousReplicaGraph map[string]*ScheduledResource + var previousReplicaGraph map[string]*scheduledResource for i, replicaVertices := range vertices { - replicaGraph := map[string]*ScheduledResource{} + replicaGraph := map[string]*scheduledResource{} sched.mergeInterimGraphVertices(replicaVertices, replicaGraph, options) if i > 0 { @@ -866,8 +866,8 @@ func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc for _, rootName := range getRoots(replicaGraph) { root := replicaGraph[rootName] leaf := previousReplicaGraph[leafName] - root.Requires = append(root.Requires, leafName) - leaf.RequiredBy = append(leaf.RequiredBy, rootName) + root.requires = append(root.requires, leafName) + leaf.requiredBy = append(leaf.requiredBy, rootName) } } } @@ -878,8 +878,8 @@ func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc } } -func correctDuplicateResources(existingGraph, newGraph map[string]*ScheduledResource, index int) { - toReplace := map[string]*ScheduledResource{} +func correctDuplicateResources(existingGraph, newGraph map[string]*scheduledResource, index int) { + toReplace := map[string]*scheduledResource{} for key, sr := range newGraph { if existingGraph[key] != nil { toReplace[key] = sr @@ -896,8 +896,8 @@ func correctDuplicateResources(existingGraph, newGraph map[string]*ScheduledReso } j++ } - for _, rKey := range sr.RequiredBy { - requires := newGraph[rKey].Requires + for _, rKey := range sr.requiredBy { + requires := newGraph[rKey].requires for i, rKey2 := range requires { if rKey2 == key { requires[i] = sr.Key() @@ -905,8 +905,8 @@ func correctDuplicateResources(existingGraph, newGraph map[string]*ScheduledReso } } } - for _, rKey := range sr.Requires { - requiredBy := newGraph[rKey].RequiredBy + for _, rKey := range sr.requires { + requiredBy := newGraph[rKey].requiredBy for i, rKey2 := range requiredBy { if rKey2 == key { requiredBy[i] = sr.Key() @@ -919,27 +919,27 @@ func correctDuplicateResources(existingGraph, newGraph map[string]*ScheduledReso } } -func getRoots(graph map[string]*ScheduledResource) []string { +func getRoots(graph map[string]*scheduledResource) []string { var result []string for key, sr := range graph { - if len(sr.Requires) == 0 { + if len(sr.requires) == 0 { result = append(result, key) } } return result } -func getLeafs(graph map[string]*ScheduledResource) []string { +func getLeafs(graph map[string]*scheduledResource) []string { var result []string for key, sr := range graph { - if len(sr.RequiredBy) == 0 { + if len(sr.requiredBy) == 0 { result = append(result, key) } } return result } -func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, graph map[string]*ScheduledResource, +func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, graph map[string]*scheduledResource, options interfaces.DependencyGraphOptions) { for _, entry := range vertices { @@ -952,11 +952,11 @@ func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, graph[key] = entry.scheduledResource } else { sched.updateContext(existingSr.context, entry.parentContext, entry.dependency) - existingSr.Requires = append(existingSr.Requires, entry.scheduledResource.Requires...) - existingSr.RequiredBy = append(existingSr.RequiredBy, entry.scheduledResource.RequiredBy...) + existingSr.requires = append(existingSr.requires, entry.scheduledResource.requires...) + existingSr.requiredBy = append(existingSr.requiredBy, entry.scheduledResource.requiredBy...) existingSr.usedInReplicas = append(existingSr.usedInReplicas, entry.scheduledResource.usedInReplicas...) - for metaKey, metaValue := range entry.scheduledResource.Meta { - existingSr.Meta[metaKey] = metaValue + for metaKey, metaValue := range entry.scheduledResource.meta { + existingSr.meta[metaKey] = metaValue } } } @@ -964,7 +964,7 @@ func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, // getResourceDestructors builds a list of functions, each of them delete one of replica resources func getResourceDestructors(construction, destruction *dependencyGraph, replicaMap map[string]client.Replica, - failed *chan *ScheduledResource) []func(<-chan struct{}) bool { + failed *chan *scheduledResource) []func(<-chan struct{}) bool { var destructors []func(<-chan struct{}) bool for _, depGraph := range [2]*dependencyGraph{construction, destruction} { @@ -984,7 +984,7 @@ func getResourceDestructors(construction, destruction *dependencyGraph, replicaM return destructors } -func getDestructorFunc(resource *ScheduledResource, failed *chan *ScheduledResource) func(<-chan struct{}) bool { +func getDestructorFunc(resource *scheduledResource, failed *chan *scheduledResource) func(<-chan struct{}) bool { return func(<-chan struct{}) bool { res := deleteResource(resource) if res != nil { @@ -997,9 +997,9 @@ func getDestructorFunc(resource *ScheduledResource, failed *chan *ScheduledResou // deleteReplicaResources invokes resources destructors and deletes replicas for which 100% of resources were deleted func deleteReplicaResources(sched *scheduler, destructors []func(<-chan struct{}) bool, replicaMap map[string]client.Replica, - failed *chan *ScheduledResource, stopChan <-chan struct{}) { + failed *chan *scheduledResource, stopChan <-chan struct{}) { - *failed = make(chan *ScheduledResource, len(destructors)) + *failed = make(chan *scheduledResource, len(destructors)) defer close(*failed) deleted := runConcurrently(destructors, sched.concurrency, stopChan) failedReplicas := map[string]bool{} @@ -1047,7 +1047,7 @@ func (sched *scheduler) composeDeletingFinalizer(construction, destruction *depe replicaMap[replica.ReplicaName()] = replica } - var failed chan *ScheduledResource + var failed chan *scheduledResource destructors := getResourceDestructors(construction, destruction, replicaMap, &failed) return func(stopChan <-chan struct{}) { diff --git a/pkg/scheduler/frontend.go b/pkg/scheduler/frontend.go index d49973b..1f059ae 100644 --- a/pkg/scheduler/frontend.go +++ b/pkg/scheduler/frontend.go @@ -161,7 +161,11 @@ func Deploy(sched interfaces.Scheduler, options interfaces.DependencyGraphOption if err != nil { return "", err } - depGraph.Deploy(stopChan) + if depGraph.Deploy(stopChan) { + log.Println("Deployment finished sucessfully") + } else { + log.Println("Deployment failed") + } } else { log.Printf("Scheduling deployment of %s flow", options.FlowName) var err error @@ -171,7 +175,6 @@ func Deploy(sched interfaces.Scheduler, options interfaces.DependencyGraphOption } log.Printf("Scheduled deployment task %s", task) } - log.Println("Done") return task, nil } diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index 2970aee..b477877 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -17,6 +17,7 @@ package scheduler import ( "fmt" "log" + "strings" "sync" "time" @@ -34,26 +35,27 @@ const ( WaitTimeout = time.Second * 600 ) -// ScheduledResource is a wrapper for Resource with attached relationship data -type ScheduledResource struct { - Requires []string - RequiredBy []string - Started bool - Ignored bool - Error error - Existing bool +// scheduledResource is a wrapper for Resource with attached relationship data +type scheduledResource struct { + requires []string + requiredBy []string + started bool + ignored bool + skipped bool + error error + existing bool context *graphContext usedInReplicas []string status interfaces.ResourceStatus suffix string interfaces.Resource // parentKey -> dependencyMetadata - Meta map[string]map[string]string + meta map[string]map[string]string sync.RWMutex } // Key returns resource identifier with optional suffix -func (sr *ScheduledResource) Key() string { +func (sr *scheduledResource) Key() string { baseKey := sr.Resource.Key() if sr.suffix == "" { return baseKey @@ -61,76 +63,66 @@ func (sr *ScheduledResource) Key() string { return baseKey + "/" + sr.suffix } -// RequestCreation does not create a scheduled resource immediately, but updates status +// requestCreation does not create a scheduled resource immediately, but updates status // and puts the scheduled resource to corresponding channel. Returns true if // scheduled resource creation was actually requested, false otherwise. -func (sr *ScheduledResource) RequestCreation(toCreate chan *ScheduledResource) bool { +func (sr *scheduledResource) requestCreation(toCreate chan<- *scheduledResource) bool { sr.RLock() // somebody already requested resource creation - if sr.Started { + if sr.started { sr.RUnlock() return true } sr.RUnlock() + isBlocked := sr.isBlocked() && sr.error == nil sr.Lock() defer sr.Unlock() - if !sr.Started && !sr.IsBlocked() { - sr.Started = true + if !sr.started && !isBlocked { + sr.started = true toCreate <- sr return true } return false } -func isResourceFinished(sr *ScheduledResource, ch chan error) bool { +func isResourceFinished(sr *scheduledResource) (bool, error) { status, err := sr.Status(nil) if err != nil { - ch <- err - return true + return status != interfaces.ResourceNotReady, err } if status == interfaces.ResourceReady { - ch <- nil - return true + return true, nil } - return false + return false, nil } -// Wait periodically checks resource status and returns if the resource processing is finished, +// wait periodically checks resource status and returns if the resource processing is finished, // regardless successful or not. The actual result of processing could be obtained from returned error. -func (sr *ScheduledResource) Wait(checkInterval time.Duration, timeout time.Duration, stopChan <-chan struct{}) (bool, error) { - ch := make(chan error, 1) - go func(ch chan error) { - log.Printf("%s flow: waiting for %v to be created", sr.context.graph.graphOptions.FlowName, sr.Key()) - if isResourceFinished(sr, ch) { - return - } - ticker := time.NewTicker(checkInterval) - for { - select { - case <-stopChan: - return - case <-ticker.C: - if isResourceFinished(sr, ch) { - return - } +func (sr *scheduledResource) wait(checkInterval time.Duration, timeout time.Duration, stopChan <-chan struct{}) (bool, error) { + log.Printf("%s flow: waiting for %v to be created", sr.context.graph.graphOptions.FlowName, sr.Key()) + var err error + var finished bool + if finished, err = isResourceFinished(sr); finished { + return false, err + } + ticker := time.NewTicker(checkInterval) + timeoutChan := time.After(timeout) + for { + select { + case <-stopChan: + return true, nil + case <-ticker.C: + if finished, err = isResourceFinished(sr); finished { + return false, err + } + case <-timeoutChan: + if err == nil { + err = fmt.Errorf("%s flow: timeout waiting for resource %s", sr.context.graph.graphOptions.FlowName, sr.Key()) } + return false, err } - - }(ch) - - select { - case <-stopChan: - return true, nil - case err := <-ch: - return false, err - case <-time.After(timeout): - e := fmt.Errorf("%s flow: timeout waiting for resource %s", sr.context.graph.graphOptions.FlowName, sr.Key()) - sr.Lock() - defer sr.Unlock() - sr.Error = e - return false, e } } @@ -139,11 +131,11 @@ func (sr *ScheduledResource) Wait(checkInterval time.Duration, timeout time.Dura // so that if resource becomes ready it stays in this status for the whole deployment duration. // Errors returned by the resource are never cached, however if AC sees permanent problem with resource it may set the // error field -func (sr *ScheduledResource) Status(meta map[string]string) (interfaces.ResourceStatus, error) { +func (sr *scheduledResource) Status(meta map[string]string) (interfaces.ResourceStatus, error) { sr.Lock() defer sr.Unlock() - if sr.status != "" || sr.Error != nil { - return sr.status, sr.Error + if sr.status != "" || sr.error != nil { + return sr.status, sr.error } status, err := sr.Resource.Status(meta) if err == nil && status == interfaces.ResourceReady { @@ -152,50 +144,64 @@ func (sr *ScheduledResource) Status(meta map[string]string) (interfaces.Resource return status, err } -// IsBlocked checks whether a scheduled resource can be created. It checks status of resources -// it depends on, via API -func (sr *ScheduledResource) IsBlocked() bool { - for _, reqKey := range sr.Requires { - meta := sr.Meta[reqKey] - _, onErrorSet := meta["on-error"] - req := sr.context.graph.graph[reqKey] +// isBlocked checks whether a scheduled resource can be created. It checks status of resources +// it depends on, via API. +func (sr *scheduledResource) isBlocked() bool { + isBlocked := false + var permanentlyBlockedKeys []string + skipped := true - status, err := req.Status(meta) + for _, reqKey := range sr.requires { + meta := sr.meta[reqKey] + onErrorValue, onErrorSet := meta["on-error"] + onErrorSet = onErrorSet && onErrorValue != "false" + req := sr.context.graph.graph[reqKey] req.RLock() - ignored := req.Ignored + ignored := req.ignored + permanentError := req.error req.RUnlock() - if err != nil && !onErrorSet && !ignored { - return true - } else if status == "ready" && onErrorSet { - return true - } else if err == nil && status != "ready" { - return true + if ignored { + continue + } + status, err := req.Status(meta) + if onErrorSet { + if permanentError == nil { + isBlocked = true + if err == nil && status == interfaces.ResourceReady { + permanentlyBlockedKeys = append(permanentlyBlockedKeys, reqKey) + } + } + } else { + skipped = false + if permanentError != nil || err != nil || status != interfaces.ResourceReady { + isBlocked = true + if permanentError != nil { + permanentlyBlockedKeys = append(permanentlyBlockedKeys, reqKey) + } + } } } - return false -} + if len(permanentlyBlockedKeys) > 0 { + sr.Lock() + sr.error = fmt.Errorf("permanently blocked because of (%s) dependencies", strings.Join(permanentlyBlockedKeys, ",")) + sr.skipped = skipped + sr.Unlock() + } -// ResetStatus resets cached status of scheduled resource -func (sr *ScheduledResource) ResetStatus() { - sr.Lock() - defer sr.Unlock() - sr.Error = nil - sr.status = "" + return isBlocked } -func createResources(toCreate chan *ScheduledResource, finished chan string, ccLimiter chan struct{}, stopChan <-chan struct{}) { +func createResources(toCreate chan *scheduledResource, finished chan<- *scheduledResource, ccLimiter chan struct{}, stopChan <-chan struct{}) { for r := range toCreate { - log.Printf("Requesting creation of %v", r.Key()) select { case <-stopChan: log.Println("Terminating creation of resources") return default: - log.Println("Deployment is not stopped, keep creating") } - go func(r *ScheduledResource, finished chan string, ccLimiter chan struct{}) { + go func(r *scheduledResource, finished chan<- *scheduledResource, ccLimiter chan struct{}) { // Acquire semaphore ccLimiter <- struct{}{} defer func() { @@ -212,18 +218,15 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL } for attemptNo := 1; attemptNo <= attempts; attemptNo++ { - - r.ResetStatus() - var err error // NOTE(gluke77): We start goroutines for dependencies // before the resource becomes ready, since dependencies // could have metadata defining their own readiness condition if attemptNo == 1 { - for _, reqKey := range r.RequiredBy { + for _, reqKey := range r.requiredBy { req := r.context.graph.graph[reqKey] - go func(req *ScheduledResource, toCreate chan *ScheduledResource) { + go func(req *scheduledResource, toCreate chan<- *scheduledResource) { ticker := time.NewTicker(CheckInterval) for { select { @@ -231,7 +234,7 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL log.Println("Terminating creation of dependencies") return case <-ticker.C: - if req.RequestCreation(toCreate) { + if req.requestCreation(toCreate) { return } } @@ -240,7 +243,7 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL } } - if attemptNo > 1 && (!r.Existing || r.context.graph.graphOptions.AllowDeleteExternalResources) { + if attemptNo > 1 && (!r.existing || r.context.graph.graphOptions.AllowDeleteExternalResources) { log.Printf("Trying to delete resource %s after previous unsuccessful attempt", r.Key()) err = r.Delete() if err != nil { @@ -249,11 +252,12 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL } r.RLock() - ignored := r.Ignored + ignored := r.ignored + srErr := r.error r.RUnlock() - if ignored { - log.Printf("Skipping creation of resource %s as being ignored", r.Key()) + if ignored || srErr != nil { + log.Printf("Skipping creation of resource %s", r.Key()) break } @@ -261,49 +265,53 @@ func createResources(toCreate chan *ScheduledResource, finished chan string, ccL err = r.Create() if err != nil { log.Printf("Error deploying resource %s: %v", r.Key(), err) - continue - } - - log.Printf("Checking status for %s", r.Key()) + } else { + log.Printf("Checking status for %s", r.Key()) - stoped, err := r.Wait(CheckInterval, waitTimeout, stopChan) + var stopped bool + stopped, err = r.wait(CheckInterval, waitTimeout, stopChan) - if stoped { - log.Printf("Received interrupt while waiting for %v. Exiting", r.Key()) - return - } + if stopped { + log.Printf("Received interrupt while waiting for %v. Exiting", r.Key()) + return + } - if err == nil { - log.Printf("Resource %s created", r.Key()) - break + if err == nil { + log.Printf("Resource %s created", r.Key()) + break + } + log.Printf("Resource %s was not created: %v", r.Key(), err) } - log.Printf("Resource %s was not created: %v", r.Key(), err) - - if attemptNo >= attempts { - if onError == "ignore" { + if attemptNo == attempts { + switch onError { + case "ignore": r.Lock() - r.Ignored = true + r.ignored = true log.Printf("Resource %s failure ignored -- prooceeding as normal", r.Key()) r.Unlock() - } else if onError == "ignore-all" { + case "ignore-all": ignoreAll(r) + default: + r.Lock() + r.error = err + r.Unlock() } } } - finished <- r.Key() + finished <- r }(r, finished, ccLimiter) } } -func ignoreAll(top *ScheduledResource) { +func ignoreAll(top *scheduledResource) { top.Lock() - top.Ignored = true + top.ignored = true top.Unlock() log.Printf("Marking resource %s as ignored", top.Key()) - for _, childKey := range top.RequiredBy { + for _, childKey := range top.requiredBy { child := top.context.graph.graph[childKey] ignoreAll(child) } @@ -315,8 +323,7 @@ func (depGraph dependencyGraph) Options() interfaces.DependencyGraphOptions { } // Deploy starts the deployment of a DependencyGraph -func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) { - +func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) bool { depCount := len(depGraph.graph) concurrency := depGraph.scheduler.concurrency @@ -326,8 +333,8 @@ func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) { } ccLimiter := make(chan struct{}, concurrencyLimiterLen) - toCreate := make(chan *ScheduledResource, depCount) - created := make(chan string, depCount) + toCreate := make(chan *scheduledResource, depCount) + created := make(chan *scheduledResource, depCount) defer func() { close(toCreate) close(created) @@ -336,18 +343,22 @@ func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) { go createResources(toCreate, created, ccLimiter, stopChan) for _, r := range depGraph.graph { - if len(r.Requires) == 0 { - r.RequestCreation(toCreate) + if len(r.requires) == 0 { + r.requestCreation(toCreate) } } log.Printf("%s flow: waiting for %d resource deployments", depGraph.graphOptions.FlowName, depCount) + result := true for i := 0; i < depCount; { select { case <-stopChan: log.Printf("Deployment of %s is stopped", depGraph.graphOptions.FlowName) - return - case <-created: + return false + case sr := <-created: + if sr.error != nil && !sr.skipped { + result = false + } i++ log.Printf("%s flow: %v out of %v were created", depGraph.graphOptions.FlowName, i, depCount) } @@ -355,25 +366,25 @@ func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) { if depGraph.finalizer != nil { depGraph.finalizer(stopChan) } - // TODO Make sure every KO gets created eventually + return result } -// GetNodeReport acts as a more verbose version of IsBlocked. It performs the -// same check as IsBlocked, but returns the DeploymentReport -func (sr *ScheduledResource) GetNodeReport(name string) report.NodeReport { +// getNodeReport acts as a more verbose version of isBlocked. It performs the +// same check as isBlocked, but returns the DeploymentReport +func (sr *scheduledResource) getNodeReport(name string) report.NodeReport { var ready bool isBlocked := false - dependencies := make([]interfaces.DependencyReport, 0, len(sr.Requires)) + dependencies := make([]interfaces.DependencyReport, 0, len(sr.requires)) status, err := sr.Status(nil) if err != nil { ready = false } else { ready = status == "ready" } - for _, rKey := range sr.Requires { + for _, rKey := range sr.requires { r := sr.context.graph.graph[rKey] r.RLock() - meta := r.Meta[sr.Key()] + meta := r.meta[sr.Key()] depReport := r.GetDependencyReport(meta) r.RUnlock() if depReport.Blocks { @@ -397,7 +408,7 @@ func (depGraph dependencyGraph) GetStatus() (interfaces.DeploymentStatus, interf var status interfaces.DeploymentStatus deploymentReport := make(report.DeploymentReport, 0, len(depGraph.graph)) for key, resource := range depGraph.graph { - depReport := resource.GetNodeReport(key) + depReport := resource.getNodeReport(key) deploymentReport = append(deploymentReport, depReport) if depReport.Ready { readyExist = true @@ -450,8 +461,8 @@ func runConcurrently(funcs []func(<-chan struct{}) bool, concurrency int, stopCh return result } -func deleteResource(resource *ScheduledResource) error { - if !resource.Existing || resource.context.graph.graphOptions.AllowDeleteExternalResources { +func deleteResource(resource *scheduledResource) error { + if !resource.existing || resource.context.graph.graphOptions.AllowDeleteExternalResources { log.Printf("%s flow: Deleting resource %s", resource.context.flow.Name, resource.Key()) err := resource.Delete() if err != nil { diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index f4117ad..484b56e 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -64,19 +64,19 @@ func TestBuildDependencyGraph(t *testing.T) { "pod/ready-1", sr.Key()) } - if len(sr.Requires) != 0 { + if len(sr.requires) != 0 { t.Errorf("wrong length of 'Requires' for scheduled resource '%s', expected %d, actual %d", - sr.Key(), 0, len(sr.Requires)) + sr.Key(), 0, len(sr.requires)) } - if len(sr.RequiredBy) != 1 { + if len(sr.requiredBy) != 1 { t.Errorf("wrong length of 'RequiredBy' for scheduled resource '%s', expected %d, actual %d", - sr.Key(), 1, len(sr.Requires)) + sr.Key(), 1, len(sr.requires)) } - if sr.RequiredBy[0] != "pod/ready-2" { + if sr.requiredBy[0] != "pod/ready-2" { t.Errorf("wrong value of 'RequiredBy' for scheduled resource '%s', expected '%s', actual '%s'", - sr.Key(), "pod/ready-2", sr.RequiredBy[0]) + sr.Key(), "pod/ready-2", sr.requiredBy[0]) return } @@ -92,19 +92,19 @@ func TestBuildDependencyGraph(t *testing.T) { "pod/ready-2", sr.Key()) } - if len(sr.Requires) != 1 { + if len(sr.requires) != 1 { t.Errorf("wrong length of 'Requires' for scheduled resource '%s', expected %d, actual %d", - sr.Key(), 1, len(sr.Requires)) + sr.Key(), 1, len(sr.requires)) } - if sr.Requires[0] != "pod/ready-1" { + if sr.requires[0] != "pod/ready-1" { t.Errorf("wrong value of 'Requires' for scheduled resource '%s', expected '%s', actual '%s'", - sr.Key(), "pod/ready-1", sr.Requires[0]) + sr.Key(), "pod/ready-1", sr.requires[0]) } - if len(sr.RequiredBy) != 0 { + if len(sr.requiredBy) != 0 { t.Errorf("wrong length of 'RequiredBy' for scheduled resource '%s', expected %d, actual %d", - sr.Key(), 0, len(sr.Requires)) + sr.Key(), 0, len(sr.requires)) } } @@ -112,47 +112,47 @@ func TestIsBlocked(t *testing.T) { depGraph := newDependencyGraph(nil, interfaces.DependencyGraphOptions{}) context := &graphContext{graph: depGraph} - one := &ScheduledResource{ + one := &scheduledResource{ Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, } depGraph.graph["fake1"] = one - if one.IsBlocked() { + if one.isBlocked() { t.Error("scheduled resource is blocked but it must not") } - two := &ScheduledResource{ + two := &scheduledResource{ Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake2", "ready")}, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, } depGraph.graph["fake2"] = two - one.Requires = []string{"fake2"} + one.requires = []string{"fake2"} - if one.IsBlocked() { + if one.isBlocked() { t.Error("scheduled resource is blocked but it must not") } - two.Error = errors.New("non-nil error") - if !one.IsBlocked() { + two.error = errors.New("non-nil error") + if !one.isBlocked() { t.Error("scheduled resource is not blocked but it must be") } - depGraph.graph["fake3"] = &ScheduledResource{ + depGraph.graph["fake3"] = &scheduledResource{ Resource: report.SimpleReporter{mocks.NewResource("fake3", "not ready")}, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, } - two.Error = nil - one.Requires = append(one.Requires, "fake3") + two.error = nil + one.requires = append(one.requires, "fake3") - if !one.IsBlocked() { + if !one.isBlocked() { t.Error("scheduled resource is not blocked but it must be") } } @@ -169,33 +169,33 @@ func TestIsBlockedWithOnErrorDependency(t *testing.T) { depGraph := newDependencyGraph(nil, interfaces.DependencyGraphOptions{}) context := &graphContext{graph: depGraph} - one := &ScheduledResource{ + one := &scheduledResource{ Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, } depGraph.graph["fake1"] = one - if one.IsBlocked() { + if one.isBlocked() { t.Error("scheduled resource is blocked but it must be not") } - two := &ScheduledResource{ + two := &scheduledResource{ Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake2", "not ready")}, - Meta: map[string]map[string]string{}, + meta: map[string]map[string]string{}, context: context, } depGraph.graph["fake2"] = two - one.Requires = []string{"fake2"} - one.Meta["fake2"] = map[string]string{"on-error": "true"} + one.requires = []string{"fake2"} + one.meta["fake2"] = map[string]string{"on-error": "true"} - if !one.IsBlocked() { + if !one.isBlocked() { t.Error("scheduled resource is not blocked but it must be") } - two.Error = errors.New("non-nil error") - if one.IsBlocked() { + two.error = errors.New("non-nil error") + if one.isBlocked() { t.Error("scheduled resource is blocked but it must be not") } } @@ -349,7 +349,7 @@ func TestLimitConcurrency(t *testing.T) { func TestStopBeforeDeploymentStarted(t *testing.T) { depGraph := newDependencyGraph(&scheduler{}, interfaces.DependencyGraphOptions{}) - sr := &ScheduledResource{ + sr := &scheduledResource{ Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, } depGraph.graph[sr.Key()] = sr @@ -604,9 +604,9 @@ func TestWaitWithZeroTimeout(t *testing.T) { defer close(stopChan) now := time.Now() - res, err := sr.Wait(CheckInterval, 0, stopChan) + res, err := sr.wait(CheckInterval, 0, stopChan) if res { - t.Error("Wait() succeded") + t.Error("wait() succeded") } if err == nil { t.Error("No error was returned") @@ -615,11 +615,8 @@ func TestWaitWithZeroTimeout(t *testing.T) { if err.Error() != expectedMessage { t.Error("Got unexpected error:", err) } - if sr.Error != err { - t.Error("ScheduledResource was not marked as permanently failed") - } } if time.Now().Sub(now) >= time.Second { - t.Error("Wait() was running for too long") + t.Error("wait() was running for too long") } } From bbf78fefb03484d9417f1776d880e8b4e610c997 Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Thu, 25 May 2017 18:21:29 -0700 Subject: [PATCH 5/6] Performance improvements * Removes one second delay before scheduler starts to check if dependent resources can be created. This drastically speeds up unit tests and subsequent deployments when resources already exist and thus resource is created instantly * Caches dependencies and definitions during deployment. When one flow is consumed from another, they were all fetched from k8s for each replica of the inner flow + once for the outer flow. Now it happens only once. It also makes deployment more consistent in case if definitions were modified in k8s in the middle of deployment * Check dependency graph for cycles only once per deployment --- pkg/scheduler/dependency_graph.go | 35 ++++++++++++++++++++----------- pkg/scheduler/frontend.go | 4 ++++ pkg/scheduler/scheduler.go | 3 +++ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index 24bc045..c093c0e 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -123,7 +123,13 @@ func (d *sortableDependencyList) Swap(i, j int) { d.Items[i], d.Items[j] = d.Items[j], d.Items[i] } -func (sched *scheduler) getDependencies() ([]client.Dependency, error) { +func (sched *scheduler) getDependencies(silent bool) ([]client.Dependency, error) { + if sched.dependencyCache != nil { + return sched.dependencyCache, nil + } + if !silent { + log.Println("Getting dependencies") + } depList, err := sched.client.Dependencies().List(api.ListOptions{LabelSelector: sched.selector}) if err != nil { return nil, err @@ -131,6 +137,7 @@ func (sched *scheduler) getDependencies() ([]client.Dependency, error) { sortableDepList := sortableDependencyList(*depList) sort.Stable(&sortableDepList) + sched.dependencyCache = sortableDepList.Items return sortableDepList.Items, nil } @@ -184,7 +191,13 @@ func getResourceName(resourceDefinition client.ResourceDefinition) (string, stri return "", "" } -func (sched *scheduler) getResourceDefinitions() (map[string]client.ResourceDefinition, error) { +func (sched *scheduler) getResourceDefinitions(silent bool) (map[string]client.ResourceDefinition, error) { + if sched.resDefsCache != nil { + return sched.resDefsCache, nil + } + if !silent { + log.Println("Getting resource definitions") + } resDefList, err := sched.client.ResourceDefinitions().List(api.ListOptions{LabelSelector: sched.selector}) if err != nil { return nil, err @@ -197,6 +210,7 @@ func (sched *scheduler) getResourceDefinitions() (map[string]client.ResourceDefi } result[kind+"/"+name] = resDef } + sched.resDefsCache = result return result, nil } @@ -567,10 +581,7 @@ func (sched *scheduler) BuildDependencyGraph(options interfaces.DependencyGraphO options.FlowName = interfaces.DefaultFlowName } - if !options.Silent { - log.Println("Getting resource definitions") - } - resDefs, err := sched.getResourceDefinitions() + resDefs, err := sched.getResourceDefinitions(options.Silent) if err != nil { return nil, err } @@ -595,10 +606,7 @@ func (sched *scheduler) BuildDependencyGraph(options interfaces.DependencyGraphO return nil, err } - if !options.Silent { - log.Println("Getting dependencies") - } - depList, err := sched.getDependencies() + depList, err := sched.getDependencies(options.Silent) if err != nil { return nil, err } @@ -606,8 +614,11 @@ func (sched *scheduler) BuildDependencyGraph(options interfaces.DependencyGraphO if !options.Silent { log.Println("Making sure there is no cycles in the dependency graph") } - if err = EnsureNoCycles(depList, resDefs); err != nil { - return nil, err + if !sched.graphHasNoCycles { + if err = EnsureNoCycles(depList, resDefs); err != nil { + return nil, err + } + sched.graphHasNoCycles = true } dependencies := groupDependencies(depList, resDefs) diff --git a/pkg/scheduler/frontend.go b/pkg/scheduler/frontend.go index 1f059ae..45174a4 100644 --- a/pkg/scheduler/frontend.go +++ b/pkg/scheduler/frontend.go @@ -30,6 +30,10 @@ type scheduler struct { client client.Interface selector labels.Selector concurrency int + + resDefsCache map[string]client.ResourceDefinition + dependencyCache []client.Dependency + graphHasNoCycles bool } var _ interfaces.Scheduler = &scheduler{} diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go index b477877..73ad75f 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -227,6 +227,9 @@ func createResources(toCreate chan *scheduledResource, finished chan<- *schedule for _, reqKey := range r.requiredBy { req := r.context.graph.graph[reqKey] go func(req *scheduledResource, toCreate chan<- *scheduledResource) { + if req.requestCreation(toCreate) { + return + } ticker := time.NewTicker(CheckInterval) for { select { From ab95c248e32eff64553d5ff5929fb3cb82b2b25c Mon Sep 17 00:00:00 2001 From: Stan Lagun Date: Tue, 30 May 2017 22:00:50 -0700 Subject: [PATCH 6/6] Improved status reporting This commit rewrites status reporting used to control deployment progress and get-status command * Old reporter machinery was removed * Resource classes were simplified: - they no longer wrap resources in SimpleReporter - they do not care about their metadata - Status method was replaced with GetProgress which reports deployment progress in float number 0..1. Most resources have either 0 or 1, but replicated resources can have all the variety - resources no longer dial with reporting - handling of success factor is now done in scheduler As a result resource classes are now much simpler On top of that new status methods were added that collect overall deployment statistics and detailed per-resource status table. Both can be retrieved using get-status CLI command --- cmd/get-status.go | 40 ++++-- e2e/example_runner.go | 11 +- glide.lock | 12 +- glide.yaml | 1 + pkg/interfaces/enums.go | 54 -------- pkg/interfaces/interfaces.go | 55 +++----- pkg/mocks/countingresource.go | 39 ++---- pkg/mocks/resource.go | 41 ++---- pkg/report/report.go | 147 -------------------- pkg/resources/common.go | 132 ++++-------------- pkg/resources/common_test.go | 81 ----------- pkg/resources/configmap.go | 52 ++++--- pkg/resources/configmap_test.go | 13 +- pkg/resources/daemonset.go | 88 ++++++------ pkg/resources/daemonset_test.go | 12 +- pkg/resources/deployment.go | 102 ++++++-------- pkg/resources/deployment_test.go | 19 +-- pkg/resources/flow.go | 38 ++--- pkg/resources/job.go | 64 +++++---- pkg/resources/persistentvolumeclaim.go | 70 +++++----- pkg/resources/petset.go | 87 ++++++------ pkg/resources/pod.go | 70 +++++----- pkg/resources/replicaset.go | 121 +++++----------- pkg/resources/replicaset_test.go | 13 +- pkg/resources/secrets.go | 53 ++++--- pkg/resources/secrets_test.go | 13 +- pkg/resources/service.go | 103 +++++++------- pkg/resources/service_test.go | 72 ++++------ pkg/resources/serviceaccount.go | 70 +++++----- pkg/resources/serviceaccount_test.go | 13 +- pkg/resources/statefulset.go | 85 ++++++------ pkg/resources/statefulset_test.go | 19 ++- pkg/resources/void.go | 14 +- pkg/scheduler/dependency_graph.go | 125 ++++++----------- pkg/scheduler/dependency_graph_test.go | 74 ---------- pkg/scheduler/frontend.go | 17 +-- pkg/scheduler/helpers.go | 138 +++++++++++++++++++ pkg/scheduler/helpers_test.go | 169 +++++++++++++++++++++++ pkg/scheduler/scheduler.go | 184 +++++++++++-------------- pkg/scheduler/scheduler_test.go | 118 +++++++--------- pkg/scheduler/status.go | 141 +++++++++++++++++++ 41 files changed, 1234 insertions(+), 1536 deletions(-) delete mode 100644 pkg/interfaces/enums.go delete mode 100644 pkg/report/report.go delete mode 100644 pkg/resources/common_test.go create mode 100644 pkg/scheduler/helpers.go create mode 100644 pkg/scheduler/helpers_test.go create mode 100644 pkg/scheduler/status.go diff --git a/cmd/get-status.go b/cmd/get-status.go index a027bad..9f5a227 100644 --- a/cmd/get-status.go +++ b/cmd/get-status.go @@ -23,8 +23,9 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/scheduler" - "github.com/spf13/cobra" + "github.com/olekukonko/tablewriter" + "github.com/spf13/cobra" "k8s.io/client-go/pkg/labels" ) @@ -93,25 +94,48 @@ func getStatus(cmd *cobra.Command, args []string) { AllowUndeclaredArgs: anyArgs, } - status, report, err := scheduler.GetStatus(c, sel, options) + graph, err := scheduler.GetStatusGraph(c, sel, options) if err != nil { log.Fatal(err) } + + //status := graph.GetDeploymentStatus() if getJSON { - data, err := json.Marshal(report) + data := graph.GetNodeStatuses() + out, err := json.Marshal(data) if err != nil { log.Fatal(err) } - fmt.Printf(string(data)) + fmt.Printf(string(out)) } else { - fmt.Printf("STATUS: %s\n", status) + status := graph.GetDeploymentStatus() + if status.Total == 0 { + fmt.Println("Dependency graph is empty") + return + } + + fmt.Println() + fmt.Printf("Total nodes: %d (%d groups)\n", status.Total, status.TotalGroups) + fmt.Printf(" Finished: %d\n", status.Finished) + fmt.Printf(" Progress: %d %%\n", int(status.Progress*100)) + if getReport { - data := report.AsText(0) - for _, line := range data { - fmt.Println(line) + fmt.Println() + table := tablewriter.NewWriter(os.Stdout) + table.SetHeader([]string{"Node", "Status", "Progress %"}) + table.SetColWidth(70) + table.SetAutoFormatHeaders(false) + for _, node := range graph.GetNodeStatuses() { + table.Append([]string{ + node.Name, + node.Status, + fmt.Sprintf("%d %%", node.Progress), + }) } + table.Render() } } + } // InitGetStatusCommand is an initialiser for get-status diff --git a/e2e/example_runner.go b/e2e/example_runner.go index 8f98139..b779e3e 100644 --- a/e2e/example_runner.go +++ b/e2e/example_runner.go @@ -25,7 +25,6 @@ import ( "github.com/Mirantis/k8s-AppController/e2e/utils" "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" "github.com/Mirantis/k8s-AppController/pkg/scheduler" . "github.com/onsi/ginkgo" @@ -105,18 +104,16 @@ func (f *examplesFramework) handleListCreation(ustList *runtime.UnstructuredList } func (f *examplesFramework) VerifyStatus(options interfaces.DependencyGraphOptions) { - var depReport report.DeploymentReport Eventually( func() bool { - status, r, err := scheduler.GetStatus(f.Client, nil, options) + graph, err := scheduler.GetStatusGraph(f.Client, nil, options) if err != nil { return false } - depReport = r.(report.DeploymentReport) - utils.Logf("STATUS: %s\n", status) - return status == interfaces.Finished || status == interfaces.Empty + status := graph.GetDeploymentStatus() + return status.Progress == 1 || status.Total == 0 }, - 300*time.Second, 5*time.Second).Should(BeTrue(), strings.Join(depReport.AsText(0), "\n")) + 300*time.Second, 5*time.Second).Should(BeTrue()) } func (f *examplesFramework) CreateRunAndVerify(exampleName string, options interfaces.DependencyGraphOptions) { diff --git a/glide.lock b/glide.lock index a951353..cd6ac2f 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 2f834ec155cd18ba87aa9e1d956eddb68e27fed79ce906a755642f746da88757 -updated: 2017-02-21T17:08:06.756552076+02:00 +hash: 3d9eb1f21cfc40c5821df13618f7cf523730afd27e75ae5c99f53354307dcec1 +updated: 2017-05-29T23:26:02.460000726-07:00 imports: - name: cloud.google.com/go version: 3b1ae45394a234c385be014e9a488f2bb6eef821 @@ -37,7 +37,7 @@ imports: - log - swagger - name: github.com/fatih/color - version: 7824417a424b65227be8ac9b14d545a7f3e6c3f0 + version: 570b54cabe6b8eb0bc2dfce68d964677d63b5260 - name: github.com/ghodss/yaml version: 73d445a93680fa1a78ae23a5839bad48f32ba1ee - name: github.com/go-openapi/jsonpointer @@ -75,6 +75,10 @@ imports: - buffer - jlexer - jwriter +- name: github.com/mattn/go-runewidth + version: 97311d9f7767e3d6f422ea06661bc2c7a19e8a5d +- name: github.com/olekukonko/tablewriter + version: febf2d34b54a69ce7530036c7503b1c9fbfdf0bb - name: github.com/onsi/ginkgo version: 74c678d97c305753605c338c6c78c49ec104b5e7 subpackages: @@ -271,6 +275,7 @@ imports: - pkg/util - pkg/util/cert - pkg/util/clock + - pkg/util/diff - pkg/util/errors - pkg/util/flowcontrol - pkg/util/framer @@ -300,6 +305,7 @@ imports: - rest - testing - tools/auth + - tools/cache - tools/clientcmd - tools/clientcmd/api - tools/clientcmd/api/latest diff --git a/glide.yaml b/glide.yaml index 8b26896..17466b2 100644 --- a/glide.yaml +++ b/glide.yaml @@ -6,3 +6,4 @@ import: - package: k8s.io/apimachinery - package: github.com/fatih/color version: ^1.1.0 +- package: github.com/olekukonko/tablewriter diff --git a/pkg/interfaces/enums.go b/pkg/interfaces/enums.go deleted file mode 100644 index 9757d93..0000000 --- a/pkg/interfaces/enums.go +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2017 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 interfaces - -// DeploymentStatus describes possible status of whole deployment process -type DeploymentStatus int - -// Possible values for DeploymentStatus -const ( - Empty DeploymentStatus = iota - Prepared - Running - Finished - TimedOut -) - -func (s DeploymentStatus) String() string { - switch s { - case Empty: - return "No dependencies loaded" - case Prepared: - return "Deployment not started" - case Running: - return "Deployment is running" - case Finished: - return "Deployment finished" - case TimedOut: - return "Deployment timed out" - } - panic("Unreachable") -} - -// ScheduledResourceStatus describes possible status of a single resource -type ScheduledResourceStatus int - -// Possible values for ScheduledResourceStatus -const ( - Init ScheduledResourceStatus = iota - Creating - Ready - Error -) diff --git a/pkg/interfaces/interfaces.go b/pkg/interfaces/interfaces.go index c470831..61ce251 100644 --- a/pkg/interfaces/interfaces.go +++ b/pkg/interfaces/interfaces.go @@ -18,42 +18,15 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" ) -// ResourceStatus is an enum of k8s resource statuses -type ResourceStatus string - -// Possible ResourceStatus values -const ( - ResourceReady ResourceStatus = "ready" - ResourceNotReady ResourceStatus = "not ready" - ResourceError ResourceStatus = "error" -) - // DefaultFlowName is the name of default flow (main dependency graph) const DefaultFlowName = "DEFAULT" -// BaseResource is an interface for AppController supported resources -type BaseResource interface { +// Resource is an interface for AppController supported resources +type Resource interface { Key() string - // Ensure that Status() supports nil as meta - Status(meta map[string]string) (ResourceStatus, error) + GetProgress() (float32, error) Create() error Delete() error - Meta(string) interface{} -} - -// DependencyReport is a report of a single dependency of a node in graph -type DependencyReport struct { - Dependency string - Blocks bool - Percentage int - Needed int - Message string -} - -// Resource is an interface for a base resource that implements getting dependency reports -type Resource interface { - BaseResource - GetDependencyReport(map[string]string) DependencyReport } // ResourceTemplate is an interface for AppController supported resource templates @@ -64,14 +37,28 @@ type ResourceTemplate interface { NewExisting(string, client.Interface, GraphContext) Resource } -// DeploymentReport is an interface to get string representation of current deployment progress -type DeploymentReport interface { - AsText(int) []string +// DeploymentStatus is the structure containing deployment status - different stats and progress info +type DeploymentStatus struct { + Total int + TotalGroups int + Failed int + Skipped int + Finished int + Replicas int + Progress float32 +} + +// NodeStatus represents status of each graph node +type NodeStatus struct { + Name string + Status string + Progress int } // DependencyGraph represents operations on dependency graph type DependencyGraph interface { - GetStatus() (DeploymentStatus, DeploymentReport) + GetDeploymentStatus() DeploymentStatus + GetNodeStatuses() []NodeStatus Deploy(<-chan struct{}) bool Options() DependencyGraphOptions } diff --git a/pkg/mocks/countingresource.go b/pkg/mocks/countingresource.go index 02175b1..be2b161 100644 --- a/pkg/mocks/countingresource.go +++ b/pkg/mocks/countingresource.go @@ -19,14 +19,13 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" ) // CountingResource is a fake resource that becomes ready after given timeout. // It also increases the counter when started and decreases it when becomes ready type CountingResource struct { key string - status interfaces.ResourceStatus + progress float32 counter *CounterWithMemo timeout time.Duration startTime time.Time @@ -37,15 +36,15 @@ func (c CountingResource) Key() string { return c.key } -// Status returns a status of the CountingResource. It also updates the status +// GetProgress returns a progress of the CountingResource. It also updates the progress // after provided timeout and decrements counter -func (c *CountingResource) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - if time.Since(c.startTime) >= c.timeout && c.status != interfaces.ResourceReady { +func (c *CountingResource) GetProgress() (float32, error) { + if time.Since(c.startTime) >= c.timeout && c.progress < 1 { c.counter.Dec() - c.status = interfaces.ResourceReady + c.progress = 1 } - return c.status, nil + return c.progress, nil } // Create increments counter and sets creation time @@ -60,32 +59,22 @@ func (c *CountingResource) Delete() error { return nil } -// Meta returns empty string -func (c *CountingResource) Meta(string) interface{} { - return nil -} - -// NameMatches returns true -func (c *CountingResource) NameMatches(_ client.ResourceDefinition, _ string) bool { - return true -} - // New returns new fake resource -func (c *CountingResource) New(_ client.ResourceDefinition, _ client.Interface) interfaces.BaseResource { - return report.SimpleReporter{BaseResource: NewResource("fake", interfaces.ResourceReady)} +func (c *CountingResource) New(_ client.ResourceDefinition, _ client.Interface) interfaces.Resource { + return NewResource("fake", 1) } // NewExisting returns new existing resource -func (c *CountingResource) NewExisting(name string, _ client.Interface) interfaces.BaseResource { - return report.SimpleReporter{BaseResource: NewResource(name, interfaces.ResourceReady)} +func (c *CountingResource) NewExisting(name string, _ client.Interface) interfaces.Resource { + return NewResource(name, 1) } // NewCountingResource creates new instance of CountingResource func NewCountingResource(key string, counter *CounterWithMemo, timeout time.Duration) *CountingResource { return &CountingResource{ - key: key, - status: interfaces.ResourceNotReady, - counter: counter, - timeout: timeout, + key: key, + progress: 0, + counter: counter, + timeout: timeout, } } diff --git a/pkg/mocks/resource.go b/pkg/mocks/resource.go index 52e774a..8f2ce1f 100644 --- a/pkg/mocks/resource.go +++ b/pkg/mocks/resource.go @@ -17,14 +17,12 @@ package mocks import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" ) // Resource is a fake resource type Resource struct { - key string - status interfaces.ResourceStatus - meta map[string]interface{} + key string + progress float32 } // Key returns a key of the Resource @@ -32,9 +30,9 @@ func (c Resource) Key() string { return c.key } -// Status returns a status of the Resource -func (c *Resource) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return c.status, nil +// GetProgress returns progress of the Resource +func (c *Resource) GetProgress() (float32, error) { + return c.progress, nil } // Create does nothing @@ -47,36 +45,17 @@ func (c *Resource) Delete() error { return nil } -// Meta returns empty string -func (c *Resource) Meta(key string) interface{} { - return c.meta[key] -} - -// NameMatches returns true -func (c *Resource) NameMatches(_ client.ResourceDefinition, _ string) bool { - return true -} - // New returns new fake resource func (c *Resource) New(_ client.ResourceDefinition, _ client.Interface) interfaces.Resource { - return report.SimpleReporter{BaseResource: NewResource("fake", "ready")} + return NewResource("fake", 1) } // NewExisting returns new existing resource -func (c *Resource) NewExisting(name string, _ client.Interface) interfaces.BaseResource { - return NewResource(name, "ready") +func (c *Resource) NewExisting(name string, _ client.Interface) interfaces.Resource { + return NewResource(name, 1) } // NewResource creates new instance of Resource -func NewResource(key string, status interfaces.ResourceStatus) *Resource { - return NewResourceWithMeta(key, status, nil) -} - -// NewResourceWithMeta creates new instance of Resource -func NewResourceWithMeta(key string, status interfaces.ResourceStatus, meta map[string]interface{}) *Resource { - return &Resource{ - key: key, - status: status, - meta: meta, - } +func NewResource(key string, progress float32) *Resource { + return &Resource{key: key, progress: progress} } diff --git a/pkg/report/report.go b/pkg/report/report.go deleted file mode 100644 index 796c3b0..0000000 --- a/pkg/report/report.go +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright 2017 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 report - -import ( - "fmt" - "strings" - - "github.com/Mirantis/k8s-AppController/pkg/interfaces" -) - -const reportIndentSize = 4 - -// NodeReport is a report of a node in graph -type NodeReport struct { - Dependent string - Blocked bool - Ready bool - Dependencies []interfaces.DependencyReport -} - -// AsText returns a human-readable representation of the report as a slice -func (n NodeReport) AsText(indent int) []string { - var blockedStr, readyStr string - if n.Blocked { - blockedStr = "BLOCKED" - } else { - blockedStr = "NOT BLOCKED" - } - - if n.Ready { - readyStr = "READY" - } else { - readyStr = "NOT READY" - } - - ret := []string{ - fmt.Sprintf("Resource: %s", n.Dependent), - blockedStr, - readyStr, - } - for _, dependency := range n.Dependencies { - ret = append(ret, dependencyReportAsText(dependency, reportIndentSize)...) - } - return Indent(indent, ret) -} - -// DeploymentReport is a full report of the status of deployment -type DeploymentReport []NodeReport - -// AsText returns a human-readable representation of the report as a slice -func (d DeploymentReport) AsText(indent int) []string { - ret := make([]string, 0, len(d)*4) - for _, n := range d { - ret = append(ret, n.AsText(reportIndentSize)...) - } - return Indent(indent, ret) -} - -// SimpleReporter creates report for simple binary cases -type SimpleReporter struct { - interfaces.BaseResource -} - -// GetDependencyReport returns a dependency report for this reporter -func (r SimpleReporter) GetDependencyReport(meta map[string]string) interfaces.DependencyReport { - status, err := r.Status(meta) - if err != nil { - return ErrorReport(r.Key(), err) - } - if status == "ready" { - return interfaces.DependencyReport{ - Dependency: r.Key(), - Blocks: false, - Percentage: 100, - Needed: 100, - Message: string(status), - } - } - return interfaces.DependencyReport{ - Dependency: r.Key(), - Blocks: true, - Percentage: 0, - Needed: 0, - Message: string(status), - } -} - -// GetResource returns the underlying resource -func (r SimpleReporter) GetResource() interfaces.BaseResource { - return r.BaseResource -} - -// ErrorReport creates a report for error cases -func ErrorReport(name string, err error) interfaces.DependencyReport { - return interfaces.DependencyReport{ - Dependency: name, - Blocks: true, - Percentage: 0, - Needed: 100, - Message: err.Error(), - } -} - -// Indent indents every line -func Indent(indent int, data []string) []string { - ret := make([]string, 0, cap(data)) - for _, line := range data { - ret = append(ret, strings.Repeat(" ", indent)+line) - } - return ret -} - -// dependencyReportAsText returns a human-readable representation of the report as a slice -func dependencyReportAsText(d interfaces.DependencyReport, indent int) []string { - var blocksStr, percStr string - if d.Blocks { - blocksStr = "BLOCKS" - } else { - blocksStr = "DOESN'T BLOCK" - } - if d.Percentage == 100 { - percStr = "" - } else { - percStr = fmt.Sprintf("%d%%/%d%%", d.Percentage, d.Needed) - } - ret := []string{ - fmt.Sprintf("Dependency: %s", d.Dependency), - blocksStr, - } - if percStr != "" { - ret = append(ret, percStr) - } - return Indent(indent, ret) -} diff --git a/pkg/resources/common.go b/pkg/resources/common.go index bcbae07..1243278 100644 --- a/pkg/resources/common.go +++ b/pkg/resources/common.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "log" - "strconv" "strings" "github.com/Mirantis/k8s-AppController/pkg/client" @@ -51,27 +50,6 @@ func init() { } } -// Base is a base struct that contains data common for all resources -type Base struct { - meta map[string]interface{} -} - -// Meta returns metadata parameter with given name, or empty string, -// if no metadata were provided or such parameter does not exist. -func (b Base) Meta(paramName string) interface{} { - if b.meta == nil { - return nil - } - - val, ok := b.meta[paramName] - - if !ok { - return nil - } - - return val -} - // KindToResourceTemplate is a map mapping kind strings to empty structs representing proper resources // structs implement interfaces.ResourceTemplate var KindToResourceTemplate = map[string]interfaces.ResourceTemplate{} @@ -87,56 +65,43 @@ func getKeys(m map[string]interfaces.ResourceTemplate) (keys []string) { return keys } -func resourceListStatus(resources []interfaces.BaseResource) (interfaces.ResourceStatus, error) { +func resourceListProgress(resources []interfaces.Resource) (float32, error) { + var total float32 + var lastErr error + if len(resources) == 0 { + return 1, nil + } for _, r := range resources { - status, err := r.Status(nil) - if err != nil { - return interfaces.ResourceError, err - } - if status != interfaces.ResourceReady { - return interfaces.ResourceNotReady, fmt.Errorf("resource %s is not ready", r.Key()) + progress, err := r.GetProgress() + if err == nil { + total += progress + } else { + lastErr = err } } - return interfaces.ResourceReady, nil -} - -func getPercentage(factorName string, meta map[string]string) (int32, error) { - var factor string - var ok bool - if meta == nil { - factor = "100" - } else if factor, ok = meta[factorName]; !ok { - factor = "100" - } - - f, err := strconv.ParseInt(factor, 10, 32) - if (f < 0 || f > 100) && err == nil { - err = fmt.Errorf("%s factor not between 0 and 100", factorName) + if total > 0 { + lastErr = nil } - return int32(f), err + total /= float32(len(resources)) + return total, lastErr } -func checkExistence(r interfaces.BaseResource) error { +func checkExistence(r interfaces.Resource) bool { log.Println("Looking for", r.Key()) - status, err := r.Status(nil) - - if err == nil { - log.Printf("Found %s, status: %s ", r.Key(), status) - return nil - } + _, err := r.GetProgress() + return err == nil - return err } -func createExistingResource(r interfaces.BaseResource) error { - if err := checkExistence(r); err != nil { +func createExistingResource(r interfaces.Resource) error { + if !checkExistence(r) { log.Printf("Expected resource %s to exist, not found", r.Key()) return errors.New("resource not found") } return nil } -func podsStateFromLabels(apiClient client.Interface, objLabels map[string]string) (interfaces.ResourceStatus, error) { +func podsStateFromLabels(apiClient client.Interface, objLabels map[string]string) (float32, error) { var labelSelectors []string for k, v := range objLabels { labelSelectors = append(labelSelectors, fmt.Sprintf("%s=%s", k, v)) @@ -144,66 +109,21 @@ func podsStateFromLabels(apiClient client.Interface, objLabels map[string]string stringSelector := strings.Join(labelSelectors, ",") selector, err := labels.Parse(stringSelector) if err != nil { - return interfaces.ResourceError, err + return 0, err } options := v1.ListOptions{LabelSelector: selector.String()} pods, err := apiClient.Pods().List(options) if err != nil { - return interfaces.ResourceError, err + return 0, err } - resources := make([]interfaces.BaseResource, 0, len(pods.Items)) + resources := make([]interfaces.Resource, 0, len(pods.Items)) for _, pod := range pods.Items { p := pod - resources = append(resources, createNewPod(&p, apiClient.Pods(), nil)) - } - - status, err := resourceListStatus(resources) - if status != interfaces.ResourceReady || err != nil { - return status, err - } - - return interfaces.ResourceReady, nil -} - -// GetIntMeta returns metadata value for parameter 'paramName', or 'defaultValue' -// if parameter is not set or is not an integer value -func GetIntMeta(r interfaces.BaseResource, paramName string, defaultValue int) int { - value := r.Meta(paramName) - if value == nil { - return defaultValue - } - - 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(floatVal) -} - -// GetStringMeta returns metadata value for parameter 'paramName', or 'defaultValue' -// if parameter is not set or is not a string value -func GetStringMeta(r interfaces.BaseResource, paramName string, defaultValue string) string { - value := r.Meta(paramName) - if value == nil { - return defaultValue - } - - strVal, ok := value.(string) - if !ok { - log.Printf("Metadata parameter '%s' for resource '%s' is set to '%v' but it does not seem to be a string, using default value %s", paramName, r.Key(), value, defaultValue) - return defaultValue + resources = append(resources, createNewPod(&p, apiClient.Pods())) } - return string(strVal) + return resourceListProgress(resources) } // Substitutes flow arguments into resource structure. Returns modified copy of the resource diff --git a/pkg/resources/common_test.go b/pkg/resources/common_test.go deleted file mode 100644 index 4efd049..0000000 --- a/pkg/resources/common_test.go +++ /dev/null @@ -1,81 +0,0 @@ -// 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/resources/configmap.go b/pkg/resources/configmap.go index da55c1e..0115fa1 100644 --- a/pkg/resources/configmap.go +++ b/pkg/resources/configmap.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" @@ -30,15 +29,13 @@ var configMapParamFields = []string{ } type newConfigMap struct { - Base - ConfigMap *v1.ConfigMap - Client corev1.ConfigMapInterface + configMap *v1.ConfigMap + client corev1.ConfigMapInterface } type existingConfigMap struct { - Base - Name string - Client corev1.ConfigMapInterface + name string + client corev1.ConfigMapInterface } type configMapTemplateFactory struct{} @@ -59,12 +56,12 @@ func (configMapTemplateFactory) Kind() string { // New returns configMap controller for new resource based on resource definition func (configMapTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { cm := parametrizeResource(def.ConfigMap, gc, configMapParamFields).(*v1.ConfigMap) - return report.SimpleReporter{BaseResource: newConfigMap{Base: Base{def.Meta}, ConfigMap: cm, Client: c.ConfigMaps()}} + return newConfigMap{configMap: cm, client: c.ConfigMaps()} } // NewExisting returns configMap controller for existing resource by its name func (configMapTemplateFactory) NewExisting(name string, ci client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingConfigMap{Name: name, Client: ci.ConfigMaps()}} + return existingConfigMap{name: name, client: ci.ConfigMaps()} } func configMapKey(name string) string { @@ -73,46 +70,47 @@ func configMapKey(name string) string { // Key returns the configMap object identifier func (c newConfigMap) Key() string { - return configMapKey(c.ConfigMap.Name) + return configMapKey(c.configMap.Name) } -func configMapStatus(c corev1.ConfigMapInterface, name string) (interfaces.ResourceStatus, error) { +func configMapProgress(c corev1.ConfigMapInterface, name string) (float32, error) { _, err := c.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - return interfaces.ResourceReady, nil + return 1, nil } -// Status returns configMap status. interfaces.ResourceReady means that its dependencies can be created -func (c newConfigMap) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return configMapStatus(c.Client, c.ConfigMap.Name) +// GetProgress returns configMap deployment progress +func (c newConfigMap) GetProgress() (float32, error) { + return configMapProgress(c.client, c.configMap.Name) } // Create looks for DaemonSet in k8s and creates it if not present func (c newConfigMap) Create() error { - if err := checkExistence(c); err != nil { - log.Println("Creating", c.Key()) - c.ConfigMap, err = c.Client.Create(c.ConfigMap) - return err + if checkExistence(c) { + return nil } - return nil + log.Println("Creating", c.Key()) + obj, err := c.client.Create(c.configMap) + c.configMap = obj + return err } // Delete deletes configMap from the cluster func (c newConfigMap) Delete() error { - return c.Client.Delete(c.ConfigMap.Name, &v1.DeleteOptions{}) + return c.client.Delete(c.configMap.Name, &v1.DeleteOptions{}) } // Key returns the configMap object identifier func (c existingConfigMap) Key() string { - return configMapKey(c.Name) + return configMapKey(c.name) } -// Status returns configMap status. interfaces.ResourceReady means that its dependencies can be created -func (c existingConfigMap) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return configMapStatus(c.Client, c.Name) +// GetProgress returns configMap deployment progress +func (c existingConfigMap) GetProgress() (float32, error) { + return configMapProgress(c.client, c.name) } // Create looks for existing configMap and returns an error if there is no such configMap in a cluster @@ -122,5 +120,5 @@ func (c existingConfigMap) Create() error { // Delete deletes configMap from the cluster func (c existingConfigMap) Delete() error { - return c.Client.Delete(c.Name, nil) + return c.client.Delete(c.name, nil) } diff --git a/pkg/resources/configmap_test.go b/pkg/resources/configmap_test.go index 8bc91d5..399dafc 100644 --- a/pkg/resources/configmap_test.go +++ b/pkg/resources/configmap_test.go @@ -17,34 +17,29 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestConfigMapSuccessCheck checks status of ready ConfigMap func TestConfigMapSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.ConfigMaps("notfail")) - status, err := configMapStatus(c.ConfigMaps(), "notfail") + progress, err := configMapProgress(c.ConfigMaps(), "notfail") if err != nil { t.Error(err) } - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) + if progress != 1 { + t.Errorf("progress must be 1 but got %v", progress) } } // TestConfigMapFailCheck checks status of not existing ConfigMap func TestConfigMapFailCheck(t *testing.T) { c := mocks.NewClient(mocks.ConfigMaps()) - status, err := configMapStatus(c.ConfigMaps(), "fail") + _, err := configMapProgress(c.ConfigMaps(), "fail") if err == nil { t.Error("error not found, expected error") } - - if status != interfaces.ResourceError { - t.Errorf("status should be `error`, is `%s` instead.", status) - } } diff --git a/pkg/resources/daemonset.go b/pkg/resources/daemonset.go index 8fadfac..b8457d3 100644 --- a/pkg/resources/daemonset.go +++ b/pkg/resources/daemonset.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" "k8s.io/client-go/kubernetes/typed/extensions/v1beta1" "k8s.io/client-go/pkg/api/v1" @@ -34,11 +33,16 @@ var daemonSetParamFields = []string{ "Spec.Template.ObjectMeta", } -// DaemonSet is wrapper for K8s DaemonSet object -type DaemonSet struct { - Base - DaemonSet *extbeta1.DaemonSet - Client v1beta1.DaemonSetInterface +// newDaemonSet is wrapper for K8s DaemonSet object +type newDaemonSet struct { + daemonSet *extbeta1.DaemonSet + client v1beta1.DaemonSetInterface +} + +// existingDaemonSet is a wrapper for K8s DaemonSet object which is deployed on a cluster before AppController +type existingDaemonSet struct { + name string + client v1beta1.DaemonSetInterface } type daemonSetTemplateFactory struct{} @@ -58,83 +62,77 @@ func (daemonSetTemplateFactory) Kind() string { // New returns DaemonSets controller for new resource based on resource definition func (d daemonSetTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - newDaemonSet := parametrizeResource(def.DaemonSet, gc, daemonSetParamFields).(*extbeta1.DaemonSet) - return report.SimpleReporter{BaseResource: DaemonSet{Base: Base{def.Meta}, DaemonSet: newDaemonSet, Client: c.DaemonSets()}} + daemonSet := parametrizeResource(def.DaemonSet, gc, daemonSetParamFields).(*extbeta1.DaemonSet) + return newDaemonSet{daemonSet: daemonSet, client: c.DaemonSets()} } // NewExisting returns DaemonSets controller for existing resource by its name func (d daemonSetTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return NewExistingDaemonSet(name, c.DaemonSets()) + return newExistingDaemonSet(name, c.DaemonSets()) } func daemonSetKey(name string) string { return "daemonset/" + name } -func daemonSetStatus(d v1beta1.DaemonSetInterface, name string) (interfaces.ResourceStatus, error) { +func daemonSetProgress(d v1beta1.DaemonSetInterface, name string) (float32, error) { daemonSet, err := d.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - if daemonSet.Status.CurrentNumberScheduled == daemonSet.Status.DesiredNumberScheduled { - return interfaces.ResourceReady, nil + if daemonSet.Status.DesiredNumberScheduled == 0 { + return 1, nil } - return interfaces.ResourceNotReady, nil + return float32(daemonSet.Status.CurrentNumberScheduled) / float32(daemonSet.Status.DesiredNumberScheduled), nil } // Key return DaemonSet name -func (d DaemonSet) Key() string { - return daemonSetKey(d.DaemonSet.Name) +func (d newDaemonSet) Key() string { + return daemonSetKey(d.daemonSet.Name) } -// Status returns DaemonSet status. interfaces.ResourceReady means that its dependencies can be created -func (d DaemonSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return daemonSetStatus(d.Client, d.DaemonSet.Name) +// GetProgress returns DaemonSet deployment progress +func (d newDaemonSet) GetProgress() (float32, error) { + return daemonSetProgress(d.client, d.daemonSet.Name) } // Create looks for DaemonSet in k8s and creates it if not present -func (d DaemonSet) Create() error { - if err := checkExistence(d); err != nil { - log.Println("Creating", d.Key()) - d.DaemonSet, err = d.Client.Create(d.DaemonSet) - return err +func (d newDaemonSet) Create() error { + if checkExistence(d) { + return nil } - return nil + log.Println("Creating", d.Key()) + obj, err := d.client.Create(d.daemonSet) + d.daemonSet = obj + return err } // Delete deletes DaemonSet from the cluster -func (d DaemonSet) Delete() error { - return d.Client.Delete(d.DaemonSet.Name, &v1.DeleteOptions{}) -} - -// ExistingDaemonSet is a wrapper for K8s DaemonSet object which is deployed on a cluster before AppController -type ExistingDaemonSet struct { - Base - Name string - Client v1beta1.DaemonSetInterface +func (d newDaemonSet) Delete() error { + return d.client.Delete(d.daemonSet.Name, &v1.DeleteOptions{}) } // Key returns DaemonSet name -func (d ExistingDaemonSet) Key() string { - return daemonSetKey(d.Name) +func (d existingDaemonSet) Key() string { + return daemonSetKey(d.name) } -// Status returns DaemonSet status. interfaces.ResourceReady means that its dependencies can be created -func (d ExistingDaemonSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return daemonSetStatus(d.Client, d.Name) +// GetProgress returns DaemonSet deployment progress +func (d existingDaemonSet) GetProgress() (float32, error) { + return daemonSetProgress(d.client, d.name) } // Create looks for existing DaemonSet and returns error if there is no such DaemonSet -func (d ExistingDaemonSet) Create() error { +func (d existingDaemonSet) Create() error { return createExistingResource(d) } // Delete deletes DaemonSet from the cluster -func (d ExistingDaemonSet) Delete() error { - return d.Client.Delete(d.Name, nil) +func (d existingDaemonSet) Delete() error { + return d.client.Delete(d.name, nil) } -// NewExistingDaemonSet is a constructor -func NewExistingDaemonSet(name string, client v1beta1.DaemonSetInterface) interfaces.Resource { - return report.SimpleReporter{BaseResource: ExistingDaemonSet{Name: name, Client: client}} +// newExistingDaemonSet is a constructor +func newExistingDaemonSet(name string, client v1beta1.DaemonSetInterface) interfaces.Resource { + return existingDaemonSet{name: name, client: client} } diff --git a/pkg/resources/daemonset_test.go b/pkg/resources/daemonset_test.go index 91319e4..453ec4a 100644 --- a/pkg/resources/daemonset_test.go +++ b/pkg/resources/daemonset_test.go @@ -17,31 +17,27 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestDaemonSetSuccessCheck check status for ready DaemonSet func TestDaemonSetSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeDaemonSet("not-fail")) - status, err := daemonSetStatus(c.DaemonSets(), "not-fail") + progress, err := daemonSetProgress(c.DaemonSets(), "not-fail") if err != nil { t.Error(err) } - if status != interfaces.ResourceReady { - t.Errorf("status should be ready , is %s instead", status) + if progress != 1 { + t.Errorf("progress must be 1 but got %v", progress) } } // TestDaemonSetFailCheck status of not ready daemonset func TestDaemonSetFailCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeDaemonSet("fail")) - status, err := daemonSetStatus(c.DaemonSets(), "fail") + _, err := daemonSetProgress(c.DaemonSets(), "fail") if err != nil { t.Error(err) } - if status != interfaces.ResourceNotReady { - t.Errorf("status should be not ready, is %s instead.", status) - } } diff --git a/pkg/resources/deployment.go b/pkg/resources/deployment.go index cd2cdea..1b3b0f7 100644 --- a/pkg/resources/deployment.go +++ b/pkg/resources/deployment.go @@ -15,12 +15,10 @@ package resources import ( - "errors" "log" "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" "k8s.io/client-go/kubernetes/typed/extensions/v1beta1" extbeta1 "k8s.io/client-go/pkg/apis/extensions/v1beta1" @@ -34,11 +32,16 @@ var deploymentParamFields = []string{ "Spec.Template.ObjectMeta", } -// Deployment is wrapper for K8s Deployment object -type Deployment struct { - Base - Deployment *extbeta1.Deployment - Client v1beta1.DeploymentInterface +// deployment is wrapper for K8s Deployment object +type newDeployment struct { + deployment *extbeta1.Deployment + client v1beta1.DeploymentInterface +} + +// existingDeployment is a wrapper for K8s Deployment object which is deployed on a cluster before AppController +type existingDeployment struct { + name string + client v1beta1.DeploymentInterface } type deploymentTemplateFactory struct{} @@ -58,97 +61,74 @@ func (deploymentTemplateFactory) Kind() string { // New returns Deployment controller for new resource based on resource definition func (deploymentTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - newDeployment := parametrizeResource(def.Deployment, gc, deploymentParamFields).(*extbeta1.Deployment) - return report.SimpleReporter{BaseResource: Deployment{Base: Base{def.Meta}, Deployment: newDeployment, Client: c.Deployments()}} + deployment := parametrizeResource(def.Deployment, gc, deploymentParamFields).(*extbeta1.Deployment) + return newDeployment{deployment: deployment, client: c.Deployments()} } // NewExisting returns Deployment controller for existing resource by its name func (deploymentTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: ExistingDeployment{Name: name, Client: c.Deployments()}} + return existingDeployment{name: name, client: c.Deployments()} } func deploymentKey(name string) string { return "deployment/" + name } -func deploymentStatus(d v1beta1.DeploymentInterface, name string) (interfaces.ResourceStatus, error) { +func deploymentProgress(d v1beta1.DeploymentInterface, name string) (float32, error) { deployment, err := d.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - if deployment.Status.UpdatedReplicas >= *deployment.Spec.Replicas && deployment.Status.AvailableReplicas >= *deployment.Spec.Replicas { - return interfaces.ResourceReady, nil + var totalReplicas float32 = 1 + if deployment.Spec.Replicas != nil { + totalReplicas = float32(*deployment.Spec.Replicas) } - return interfaces.ResourceNotReady, nil + return float32(deployment.Status.AvailableReplicas) / totalReplicas, nil } // Key return Deployment name -func (d Deployment) Key() string { - return deploymentKey(d.Deployment.Name) +func (d newDeployment) Key() string { + return deploymentKey(d.deployment.Name) } -// Status returns Deployment status. interfaces.ResourceReady means that its dependencies can be created -func (d Deployment) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return deploymentStatus(d.Client, d.Deployment.Name) +// GetProgress returns Deployment progress +func (d newDeployment) GetProgress() (float32, error) { + return deploymentProgress(d.client, d.deployment.Name) } // Create looks for the Deployment in K8s and creates it if not present -func (d Deployment) Create() error { - log.Println("Looking for deployment", d.Deployment.Name) - status, err := d.Status(nil) - - if err == nil { - log.Printf("Found deployment %s, status: %s", d.Deployment.Name, status) - log.Println("Skipping creation of deployment", d.Deployment.Name) +func (d newDeployment) Create() error { + if checkExistence(d) { + return nil } - log.Println("Creating deployment", d.Deployment.Name) - d.Deployment, err = d.Client.Create(d.Deployment) + log.Println("Creating", d.Key()) + obj, err := d.client.Create(d.deployment) + d.deployment = obj return err } // Delete deletes Deployment from the cluster -func (d Deployment) Delete() error { - return d.Client.Delete(d.Deployment.Name, nil) -} - -// ExistingDeployment is a wrapper for K8s Deployment object which is deployed on a cluster before AppController -type ExistingDeployment struct { - Base - Name string - Client v1beta1.DeploymentInterface -} - -// UpdateMeta does nothing at the moment -func (d ExistingDeployment) UpdateMeta(meta map[string]string) error { - return nil +func (d newDeployment) Delete() error { + return d.client.Delete(d.deployment.Name, nil) } // Key returns Deployment name -func (d ExistingDeployment) Key() string { - return deploymentKey(d.Name) +func (d existingDeployment) Key() string { + return deploymentKey(d.name) } -// Status returns Deployment status. interfaces.ResourceReady means that its dependencies can be created -func (d ExistingDeployment) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return deploymentStatus(d.Client, d.Name) +// GetProgress returns Deployment deployment progress +func (d existingDeployment) GetProgress() (float32, error) { + return deploymentProgress(d.client, d.name) } // Create looks for existing Deployment and returns error if there is no such Deployment -func (d ExistingDeployment) Create() error { - log.Println("Looking for deployment", d.Name) - status, err := d.Status(nil) - - if err == nil { - log.Printf("Found deployment %s, status: %s", d.Name, status) - return nil - } - - log.Fatalf("Deployment %s not found", d.Name) - return errors.New("Deployment not found") +func (d existingDeployment) Create() error { + return createExistingResource(d) } // Delete deletes Deployment from the cluster -func (d ExistingDeployment) Delete() error { - return d.Client.Delete(d.Name, nil) +func (d existingDeployment) Delete() error { + return d.client.Delete(d.name, nil) } diff --git a/pkg/resources/deployment_test.go b/pkg/resources/deployment_test.go index 66b8c67..bc25ba9 100644 --- a/pkg/resources/deployment_test.go +++ b/pkg/resources/deployment_test.go @@ -17,48 +17,35 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestDeploymentSuccessCheck checks status of ready Deployment func TestDeploymentSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeDeployment("notfail")) - status, err := deploymentStatus(c.Deployments(), "notfail") + _, err := deploymentProgress(c.Deployments(), "notfail") if err != nil { t.Error(err) } - - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) - } } // TestDeploymentFailUpdatedCheck checks status of not ready deployment func TestDeploymentFailUpdatedCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeDeployment("fail")) - status, err := deploymentStatus(c.Deployments(), "fail") + _, err := deploymentProgress(c.Deployments(), "fail") if err != nil { t.Error(err) } - - if status != interfaces.ResourceNotReady { - t.Errorf("status should be `not ready`, is `%s` instead.", status) - } } // TestDeploymentFailAvailableCheck checks status of not ready deployment func TestDeploymentFailAvailableCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeDeployment("failav")) - status, err := deploymentStatus(c.Deployments(), "failav") + _, err := deploymentProgress(c.Deployments(), "failav") if err != nil { t.Error(err) } - - if status != interfaces.ResourceNotReady { - t.Errorf("status should be `not ready`, is `%s` instead.", status) - } } diff --git a/pkg/resources/flow.go b/pkg/resources/flow.go index 9b11f84..800ffc7 100644 --- a/pkg/resources/flow.go +++ b/pkg/resources/flow.go @@ -19,11 +19,9 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" ) type flow struct { - Base flow *client.Flow context interfaces.GraphContext originalName string @@ -49,13 +47,11 @@ func (flowTemplateFactory) Kind() string { func (flowTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { newFlow := parametrizeResource(def.Flow, gc, []string{"*"}).(*client.Flow) - return report.SimpleReporter{ - BaseResource: &flow{ - Base: Base{def.Meta}, - flow: newFlow, - context: gc, - originalName: def.Flow.Name, - }} + return &flow{ + flow: newFlow, + context: gc, + originalName: def.Flow.Name, + } } // NewExisting returns Flow controller for existing resource by its name. Since flow is not a real k8s resource @@ -121,7 +117,7 @@ func (f *flow) Create() error { // Delete releases resources allocated to the last flow replica (i.e. decreases replica count by 1) // Note, that unlike Create() method Delete() is not idempotent. However, it doesn't create any issues since -// Delete is called during dlow destruction which can happen only once while Create ensures that at least one flow +// Delete is called during flow destruction which can happen only once while Create ensures that at least one flow // replica exists, and as such can be called any number of times func (f flow) Delete() error { graph, err := f.buildDependencyGraph(0, false) @@ -133,29 +129,17 @@ func (f flow) Delete() error { return nil } -// Status returns current status of the flow deployment -func (f flow) Status(meta map[string]string) (interfaces.ResourceStatus, error) { +// GetProgress returns current status of the flow deployment +func (f flow) GetProgress() (float32, error) { graph := f.currentGraph if graph == nil { var err error graph, err = f.buildDependencyGraph(-1, true) if err != nil { - return interfaces.ResourceError, err + return 0, err } } - status, _ := graph.GetStatus() - - switch status { - case interfaces.Empty: - fallthrough - case interfaces.Finished: - return interfaces.ResourceReady, nil - case interfaces.Prepared: - fallthrough - case interfaces.Running: - return interfaces.ResourceNotReady, nil - default: - return interfaces.ResourceError, nil - } + deploymentReport := graph.GetDeploymentStatus() + return deploymentReport.Progress, nil } diff --git a/pkg/resources/job.go b/pkg/resources/job.go index 3b1e331..fb7d899 100644 --- a/pkg/resources/job.go +++ b/pkg/resources/job.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" batchv1 "k8s.io/client-go/kubernetes/typed/batch/v1" "k8s.io/client-go/pkg/apis/batch/v1" @@ -34,9 +33,13 @@ var jobParamFields = []string{ } type newJob struct { - Base - Job *v1.Job - Client batchv1.JobInterface + job *v1.Job + client batchv1.JobInterface +} + +type existingJob struct { + name string + client batchv1.JobInterface } func jobKey(name string) string { @@ -61,72 +64,67 @@ func (jobTemplateFactory) Kind() string { // New returns Job controller for new resource based on resource definition func (jobTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { job := parametrizeResource(def.Job, gc, jobParamFields).(*v1.Job) - return createNewJob(job, c.Jobs(), def.Meta) + return createNewJob(job, c.Jobs()) } // NewExisting returns Job controller for existing resource by its name func (jobTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingJob{Name: name, Client: c.Jobs()}} + return existingJob{name: name, client: c.Jobs()} } -func jobStatus(j batchv1.JobInterface, name string) (interfaces.ResourceStatus, error) { +func jobProgress(j batchv1.JobInterface, name string) (float32, error) { job, err := j.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } for _, cond := range job.Status.Conditions { if cond.Type == "Complete" && cond.Status == "True" { - return interfaces.ResourceReady, nil + return 1, nil } } - return interfaces.ResourceNotReady, nil + return 0, nil } // Key returns job name func (j newJob) Key() string { - return jobKey(j.Job.Name) + return jobKey(j.job.Name) } -// Status returns job status -func (j newJob) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return jobStatus(j.Client, j.Job.Name) +// GetProgress returns job deployment progress +func (j newJob) GetProgress() (float32, error) { + return jobProgress(j.client, j.job.Name) } // Create looks for the Job in k8s and creates it if not present func (j newJob) Create() error { - if err := checkExistence(j); err != nil { - log.Println("Creating", j.Key()) - j.Job, err = j.Client.Create(j.Job) - return err + if checkExistence(j) { + return nil } - return nil + log.Println("Creating", j.Key()) + obj, err := j.client.Create(j.job) + j.job = obj + return err } // Delete deletes Job from the cluster func (j newJob) Delete() error { - return j.Client.Delete(j.Job.Name, nil) + return j.client.Delete(j.job.Name, nil) } -func createNewJob(job *v1.Job, client batchv1.JobInterface, meta map[string]interface{}) interfaces.Resource { - return report.SimpleReporter{BaseResource: newJob{Base: Base{meta}, Job: job, Client: client}} -} - -type existingJob struct { - Base - Name string - Client batchv1.JobInterface +func createNewJob(job *v1.Job, client batchv1.JobInterface) interfaces.Resource { + return newJob{job: job, client: client} } // Key return Job name func (j existingJob) Key() string { - return jobKey(j.Name) + return jobKey(j.name) } -// Status returns job status -func (j existingJob) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return jobStatus(j.Client, j.Name) +// GetProgress returns job deployment progress +func (j existingJob) GetProgress() (float32, error) { + return jobProgress(j.client, j.name) } // Create looks for existing Job and returns error if there is no such Job @@ -136,5 +134,5 @@ func (j existingJob) Create() error { // Delete deletes Job from the cluster func (j existingJob) Delete() error { - return j.Client.Delete(j.Name, nil) + return j.client.Delete(j.name, nil) } diff --git a/pkg/resources/persistentvolumeclaim.go b/pkg/resources/persistentvolumeclaim.go index af69a17..369277f 100644 --- a/pkg/resources/persistentvolumeclaim.go +++ b/pkg/resources/persistentvolumeclaim.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" @@ -30,9 +29,13 @@ var persistentVolumeClaimParamFields = []string{ } type newPersistentVolumeClaim struct { - Base - PersistentVolumeClaim *v1.PersistentVolumeClaim - Client corev1.PersistentVolumeClaimInterface + persistentVolumeClaim *v1.PersistentVolumeClaim + client corev1.PersistentVolumeClaimInterface +} + +type existingPersistentVolumeClaim struct { + name string + client corev1.PersistentVolumeClaimInterface } type persistentVolumeClaimTemplateFactory struct{} @@ -52,17 +55,17 @@ func (persistentVolumeClaimTemplateFactory) Kind() string { // New returns PVC controller for new resource based on resource definition func (persistentVolumeClaimTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{ - BaseResource: newPersistentVolumeClaim{ - Base: Base{def.Meta}, - PersistentVolumeClaim: parametrizeResource(def.PersistentVolumeClaim, gc, persistentVolumeClaimParamFields).(*v1.PersistentVolumeClaim), - Client: c.PersistentVolumeClaims(), - }} + pvc := parametrizeResource(def.PersistentVolumeClaim, gc, persistentVolumeClaimParamFields).(*v1.PersistentVolumeClaim) + return newPersistentVolumeClaim{ + persistentVolumeClaim: pvc, + client: c.PersistentVolumeClaims(), + } + } // NewExisting returns PVC controller for existing resource by its name func (persistentVolumeClaimTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingPersistentVolumeClaim{Name: name, Client: c.PersistentVolumeClaims()}} + return existingPersistentVolumeClaim{name: name, client: c.PersistentVolumeClaims()} } func persistentVolumeClaimKey(name string) string { @@ -71,51 +74,46 @@ func persistentVolumeClaimKey(name string) string { // Key returns the PersistentVolumeClaim object identifier func (p newPersistentVolumeClaim) Key() string { - return persistentVolumeClaimKey(p.PersistentVolumeClaim.Name) + return persistentVolumeClaimKey(p.persistentVolumeClaim.Name) } -func persistentVolumeClaimStatus(p corev1.PersistentVolumeClaimInterface, name string) (interfaces.ResourceStatus, error) { +func persistentVolumeClaimProgress(p corev1.PersistentVolumeClaimInterface, name string) (float32, error) { persistentVolumeClaim, err := p.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } if persistentVolumeClaim.Status.Phase == v1.ClaimBound { - return interfaces.ResourceReady, nil + return 1, nil } - return interfaces.ResourceNotReady, nil + return 0, nil } // Create looks for the PersistentVolumeClaim in k8s and creates it if not present func (p newPersistentVolumeClaim) Create() error { - if err := checkExistence(p); err != nil { - log.Println("Creating", p.Key()) - p.PersistentVolumeClaim, err = p.Client.Create(p.PersistentVolumeClaim) - return err + if checkExistence(p) { + return nil } - return nil + log.Println("Creating", p.Key()) + obj, err := p.client.Create(p.persistentVolumeClaim) + p.persistentVolumeClaim = obj + return err } // Delete deletes persistentVolumeClaim from the cluster func (p newPersistentVolumeClaim) Delete() error { - return p.Client.Delete(p.PersistentVolumeClaim.Name, &v1.DeleteOptions{}) -} - -// Status returns PVC status. -func (p newPersistentVolumeClaim) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return persistentVolumeClaimStatus(p.Client, p.PersistentVolumeClaim.Name) + return p.client.Delete(p.persistentVolumeClaim.Name, &v1.DeleteOptions{}) } -type existingPersistentVolumeClaim struct { - Base - Name string - Client corev1.PersistentVolumeClaimInterface +// GetProgress returns PVC deployment progress. +func (p newPersistentVolumeClaim) GetProgress() (float32, error) { + return persistentVolumeClaimProgress(p.client, p.persistentVolumeClaim.Name) } // Key returns the PersistentVolumeClaim object identifier func (p existingPersistentVolumeClaim) Key() string { - return persistentVolumeClaimKey(p.Name) + return persistentVolumeClaimKey(p.name) } // Create looks for existing PVC and returns error if there is no such PVC @@ -123,12 +121,12 @@ func (p existingPersistentVolumeClaim) Create() error { return createExistingResource(p) } -// Status returns PVC status. -func (p existingPersistentVolumeClaim) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return persistentVolumeClaimStatus(p.Client, p.Name) +// GetProgress returns PVC deployment progress +func (p existingPersistentVolumeClaim) GetProgress() (float32, error) { + return persistentVolumeClaimProgress(p.client, p.name) } // Delete deletes persistentVolumeClaim from the cluster func (p existingPersistentVolumeClaim) Delete() error { - return p.Client.Delete(p.Name, nil) + return p.client.Delete(p.name, nil) } diff --git a/pkg/resources/petset.go b/pkg/resources/petset.go index 9004a88..cd17fab 100644 --- a/pkg/resources/petset.go +++ b/pkg/resources/petset.go @@ -19,9 +19,7 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" appsalpha1 "github.com/Mirantis/k8s-AppController/pkg/client/petsets/apis/apps/v1alpha1" - "github.com/Mirantis/k8s-AppController/pkg/client/petsets/typed/apps/v1alpha1" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" ) var petSetParamFields = []string{ @@ -32,12 +30,16 @@ var petSetParamFields = []string{ "Spec.Template.ObjectMeta", } -// PetSet is a wrapper for K8s PetSet object -type PetSet struct { - Base - PetSet *appsalpha1.PetSet - Client v1alpha1.PetSetInterface - APIClient client.Interface +// newPetSet is a wrapper for K8s PetSet object +type newPetSet struct { + petSet *appsalpha1.PetSet + client client.Interface +} + +// existingPetSet is a wrapper for K8s PetSet object which is meant to already be in a cluster bofer AppController execution +type existingPetSet struct { + name string + client client.Interface } type petSetTemplateFactory struct{} @@ -58,21 +60,21 @@ func (petSetTemplateFactory) Kind() string { // New returns PetSet controller for new resource based on resource definition func (petSetTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { petSet := parametrizeResource(def.PetSet, gc, petSetParamFields).(*appsalpha1.PetSet) - return newPetSet(petSet, c.PetSets(), c, def.Meta) + return createNewPetSet(petSet, c) } // NewExisting returns PetSet controller for existing resource by its name func (petSetTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: ExistingPetSet{Name: name, Client: c.PetSets(), APIClient: c}} + return existingPetSet{name: name, client: c} } -func petsetStatus(p v1alpha1.PetSetInterface, name string, apiClient client.Interface) (interfaces.ResourceStatus, error) { +func petsetProgress(client client.Interface, name string) (float32, error) { // Use label from petset spec to get needed pods - ps, err := p.Get(name) + ps, err := client.PetSets().Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - return podsStateFromLabels(apiClient, ps.Spec.Template.ObjectMeta.Labels) + return podsStateFromLabels(client, ps.Spec.Template.ObjectMeta.Labels) } func petsetKey(name string) string { @@ -80,59 +82,52 @@ func petsetKey(name string) string { } // Key returns PetSet name -func (p PetSet) Key() string { - return petsetKey(p.PetSet.Name) +func (p newPetSet) Key() string { + return petsetKey(p.petSet.Name) } // Create looks for the PetSet in Kubernetes cluster and creates it if it's not there -func (p PetSet) Create() error { - if err := checkExistence(p); err != nil { - log.Println("Creating", p.Key()) - _, err = p.Client.Create(p.PetSet) - return err +func (p newPetSet) Create() error { + if checkExistence(p) { + return nil + } - return nil + log.Println("Creating", p.Key()) + obj, err := p.client.PetSets().Create(p.petSet) + p.petSet = obj + return err } // Delete deletes PetSet from the cluster -func (p PetSet) Delete() error { - return p.Client.Delete(p.PetSet.Name, nil) -} - -// Status returns PetSet status. interfaces.ResourceReady is regarded as sufficient for it's dependencies to be created. -func (p PetSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return petsetStatus(p.Client, p.PetSet.Name, p.APIClient) +func (p newPetSet) Delete() error { + return p.client.PetSets().Delete(p.petSet.Name, nil) } -// newPetSet is a constructor -func newPetSet(petset *appsalpha1.PetSet, client v1alpha1.PetSetInterface, apiClient client.Interface, meta map[string]interface{}) interfaces.Resource { - return report.SimpleReporter{BaseResource: PetSet{Base: Base{meta}, PetSet: petset, Client: client, APIClient: apiClient}} +// GetProgress returns PetSet deployment progress +func (p newPetSet) GetProgress() (float32, error) { + return petsetProgress(p.client, p.petSet.Name) } -// ExistingPetSet is a wrapper for K8s PetSet object which is meant to already be in a cluster bofer AppController execution -type ExistingPetSet struct { - Base - Name string - Client v1alpha1.PetSetInterface - APIClient client.Interface +func createNewPetSet(petset *appsalpha1.PetSet, client client.Interface) interfaces.Resource { + return newPetSet{petSet: petset, client: client} } // Key returns PetSet name -func (p ExistingPetSet) Key() string { - return petsetKey(p.Name) +func (p existingPetSet) Key() string { + return petsetKey(p.name) } // Create looks for existing PetSet and returns error if there is no such PetSet -func (p ExistingPetSet) Create() error { +func (p existingPetSet) Create() error { return createExistingResource(p) } -// Status returns PetSet status. interfaces.ResourceReady is regarded as sufficient for it's dependencies to be created. -func (p ExistingPetSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return petsetStatus(p.Client, p.Name, p.APIClient) +// GetProgress returns PetSet deployment progress +func (p existingPetSet) GetProgress() (float32, error) { + return petsetProgress(p.client, p.name) } // Delete deletes PetSet from the cluster -func (p ExistingPetSet) Delete() error { - return p.Client.Delete(p.Name, nil) +func (p existingPetSet) Delete() error { + return p.client.PetSets().Delete(p.name, nil) } diff --git a/pkg/resources/pod.go b/pkg/resources/pod.go index 915eb82..c53d8fc 100644 --- a/pkg/resources/pod.go +++ b/pkg/resources/pod.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" @@ -33,9 +32,13 @@ var podParamFields = []string{ } type newPod struct { - Base - Pod *v1.Pod - Client corev1.PodInterface + pod *v1.Pod + client corev1.PodInterface +} + +type existingPod struct { + name string + client corev1.PodInterface } type podTemplateFactory struct{} @@ -56,12 +59,12 @@ func (podTemplateFactory) Kind() string { // New returns Pod controller for new resource based on resource definition func (podTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { pod := parametrizeResource(def.Pod, gc, podParamFields).(*v1.Pod) - return createNewPod(pod, c.Pods(), def.Meta) + return createNewPod(pod, c.Pods()) } // NewExisting returns Pod controller for existing resource by its name func (podTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingPod{Name: name, Client: c.Pods()}} + return existingPod{name: name, client: c.Pods()} } func podKey(name string) string { @@ -70,24 +73,20 @@ func podKey(name string) string { // Key returns Pod name func (p newPod) Key() string { - return podKey(p.Pod.Name) + return podKey(p.pod.Name) } -func podStatus(p corev1.PodInterface, name string) (interfaces.ResourceStatus, error) { +func podProgress(p corev1.PodInterface, name string) (float32, error) { pod, err := p.Get(name) if err != nil { - return interfaces.ResourceError, err - } - - if pod.Status.Phase == "Succeeded" { - return interfaces.ResourceReady, nil + return 0, err } - if pod.Status.Phase == "Running" && isReady(pod) { - return interfaces.ResourceReady, nil + if pod.Status.Phase == "Succeeded" || pod.Status.Phase == "Running" && isReady(pod) { + return 1, nil } - return interfaces.ResourceNotReady, nil + return 0, nil } func isReady(pod *v1.Pod) bool { @@ -102,37 +101,32 @@ func isReady(pod *v1.Pod) bool { // Create looks for the Pod in k8s and creates it if not present func (p newPod) Create() error { - if err := checkExistence(p); err != nil { - log.Println("Creating", p.Key()) - p.Pod, err = p.Client.Create(p.Pod) - return err + if checkExistence(p) { + return nil } - return nil + log.Println("Creating", p.Key()) + obj, err := p.client.Create(p.pod) + p.pod = obj + return err } // Delete deletes pod from the cluster func (p newPod) Delete() error { - return p.Client.Delete(p.Pod.Name, nil) -} - -// Status returns pod status. It returns interfaces.ResourceReady if the pod is succeeded or running with succeeding readiness probe. -func (p newPod) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return podStatus(p.Client, p.Pod.Name) + return p.client.Delete(p.pod.Name, nil) } -func createNewPod(pod *v1.Pod, client corev1.PodInterface, meta map[string]interface{}) interfaces.Resource { - return report.SimpleReporter{BaseResource: newPod{Base: Base{meta}, Pod: pod, Client: client}} +// GetProgress returns Pod deployment progress +func (p newPod) GetProgress() (float32, error) { + return podProgress(p.client, p.pod.Name) } -type existingPod struct { - Base - Name string - Client corev1.PodInterface +func createNewPod(pod *v1.Pod, client corev1.PodInterface) interfaces.Resource { + return newPod{pod: pod, client: client} } // Key returns Pod name func (p existingPod) Key() string { - return podKey(p.Name) + return podKey(p.name) } // Create looks for existing Pod and returns error if there is no such Pod @@ -140,12 +134,12 @@ func (p existingPod) Create() error { return createExistingResource(p) } -// Status returns pod status. It returns interfaces.ResourceReady if the pod is succeeded or running with succeeding readiness probe. -func (p existingPod) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return podStatus(p.Client, p.Name) +// GetProgress returns Pod deployment progress +func (p existingPod) GetProgress() (float32, error) { + return podProgress(p.client, p.name) } // Delete deletes pod from the cluster func (p existingPod) Delete() error { - return p.Client.Delete(p.Name, nil) + return p.client.Delete(p.name, nil) } diff --git a/pkg/resources/replicaset.go b/pkg/resources/replicaset.go index 86643ee..45335c2 100644 --- a/pkg/resources/replicaset.go +++ b/pkg/resources/replicaset.go @@ -15,12 +15,10 @@ package resources import ( - "fmt" "log" "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" "k8s.io/client-go/kubernetes/typed/extensions/v1beta1" extbeta1 "k8s.io/client-go/pkg/apis/extensions/v1beta1" @@ -34,12 +32,14 @@ var replicaSetParamFields = []string{ "Spec.Template.ObjectMeta", } -const successFactorKey = "success_factor" - type newReplicaSet struct { - Base - ReplicaSet *extbeta1.ReplicaSet - Client v1beta1.ReplicaSetInterface + replicaSet *extbeta1.ReplicaSet + client v1beta1.ReplicaSetInterface +} + +type existingReplicaSet struct { + name string + client v1beta1.ReplicaSetInterface } type replicaSetTemplateFactory struct{} @@ -60,65 +60,25 @@ func (replicaSetTemplateFactory) Kind() string { // New returns ReplicaSet controller for new resource based on resource definition func (replicaSetTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { replicaSet := parametrizeResource(def.ReplicaSet, gc, replicaSetParamFields).(*extbeta1.ReplicaSet) - return createNewReplicaSet(replicaSet, c.ReplicaSets(), def.Meta) + return createNewReplicaSet(replicaSet, c.ReplicaSets()) } // NewExisting returns ReplicaSet controller for existing resource by its name func (replicaSetTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return existingReplicaSet{Name: name, Client: c.ReplicaSets()} + return existingReplicaSet{name: name, client: c.ReplicaSets()} } -func replicaSetStatus(r v1beta1.ReplicaSetInterface, name string, meta map[string]string) (interfaces.ResourceStatus, error) { +func replicaSetProgress(r v1beta1.ReplicaSetInterface, name string) (float32, error) { rs, err := r.Get(name) if err != nil { - return interfaces.ResourceError, err - } - - successFactor, err := getPercentage(successFactorKey, meta) - if err != nil { - return interfaces.ResourceError, err - } - - if rs.Status.Replicas*100 < *rs.Spec.Replicas*successFactor { - return interfaces.ResourceNotReady, nil + return 0, err } - return interfaces.ResourceReady, nil -} - -func replicaSetReport(r v1beta1.ReplicaSetInterface, name string, meta map[string]string) interfaces.DependencyReport { - rs, err := r.Get(name) - if err != nil { - return report.ErrorReport(name, err) - } - successFactor, err := getPercentage(successFactorKey, meta) - if err != nil { - return report.ErrorReport(name, err) - } - percentage := (*rs.Spec.Replicas * 100 / rs.Status.Replicas) - message := fmt.Sprintf( - "%d of %d replicas up (%d %%, needed %d%%)", - rs.Status.Replicas, - rs.Spec.Replicas, - percentage, - successFactor, - ) - if percentage >= successFactor { - return interfaces.DependencyReport{ - Dependency: name, - Blocks: false, - Percentage: int(percentage), - Needed: int(successFactor), - Message: message, - } - } - return interfaces.DependencyReport{ - Dependency: name, - Blocks: false, - Percentage: int(percentage), - Needed: int(successFactor), - Message: message, + total := float32(1) + if rs.Spec.Replicas != nil && *rs.Spec.Replicas != 0 { + total = float32(*rs.Spec.Replicas) } + return float32(rs.Status.Replicas) / total, nil } func replicaSetKey(name string) string { @@ -127,47 +87,37 @@ func replicaSetKey(name string) string { // Key returns ReplicaSet name func (r newReplicaSet) Key() string { - return replicaSetKey(r.ReplicaSet.Name) + return replicaSetKey(r.replicaSet.Name) } // Create looks for the ReplicaSet in k8s and creates it if not present func (r newReplicaSet) Create() error { - if err := checkExistence(r); err != nil { - log.Println("Creating", r.Key()) - r.ReplicaSet, err = r.Client.Create(r.ReplicaSet) - return err + if checkExistence(r) { + return nil } - return nil + log.Println("Creating", r.Key()) + obj, err := r.client.Create(r.replicaSet) + r.replicaSet = obj + return err } // Delete deletes ReplicaSet from the cluster func (r newReplicaSet) Delete() error { - return r.Client.Delete(r.ReplicaSet.Name, nil) -} - -// Status returns ReplicaSet status based on provided meta. -func (r newReplicaSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return replicaSetStatus(r.Client, r.ReplicaSet.Name, meta) -} - -// GetDependencyReport returns a DependencyReport for this ReplicaSet -func (r newReplicaSet) GetDependencyReport(meta map[string]string) interfaces.DependencyReport { - return replicaSetReport(r.Client, r.ReplicaSet.Name, meta) + return r.client.Delete(r.replicaSet.Name, nil) } -func createNewReplicaSet(replicaSet *extbeta1.ReplicaSet, client v1beta1.ReplicaSetInterface, meta map[string]interface{}) newReplicaSet { - return newReplicaSet{Base: Base{meta}, ReplicaSet: replicaSet, Client: client} +// GetProgress returns ReplicaSet deployment progress +func (r newReplicaSet) GetProgress() (float32, error) { + return replicaSetProgress(r.client, r.replicaSet.Name) } -type existingReplicaSet struct { - Base - Name string - Client v1beta1.ReplicaSetInterface +func createNewReplicaSet(replicaSet *extbeta1.ReplicaSet, client v1beta1.ReplicaSetInterface) newReplicaSet { + return newReplicaSet{replicaSet: replicaSet, client: client} } // Key returns ReplicaSet name func (r existingReplicaSet) Key() string { - return replicaSetKey(r.Name) + return replicaSetKey(r.name) } // Create looks for existing ReplicaSet and returns error if there is no such ReplicaSet @@ -175,17 +125,12 @@ func (r existingReplicaSet) Create() error { return createExistingResource(r) } -// Status returns ReplicaSet status based on provided meta. -func (r existingReplicaSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return replicaSetStatus(r.Client, r.Name, meta) +// GetProgress returns ReplicaSet deployment progress +func (r existingReplicaSet) GetProgress() (float32, error) { + return replicaSetProgress(r.client, r.name) } // Delete deletes ReplicaSet from the cluster func (r existingReplicaSet) Delete() error { - return r.Client.Delete(r.Name, nil) -} - -// GetDependencyReport returns a DependencyReport for this ReplicaSet -func (r existingReplicaSet) GetDependencyReport(meta map[string]string) interfaces.DependencyReport { - return replicaSetReport(r.Client, r.Name, meta) + return r.client.Delete(r.name, nil) } diff --git a/pkg/resources/replicaset_test.go b/pkg/resources/replicaset_test.go index 8b17345..51e35a7 100644 --- a/pkg/resources/replicaset_test.go +++ b/pkg/resources/replicaset_test.go @@ -17,32 +17,23 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) func TestSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeReplicaSet("notfail")) - status, err := replicaSetStatus(c.ReplicaSets(), "notfail", nil) + _, err := replicaSetProgress(c.ReplicaSets(), "notfail") if err != nil { t.Error(err) } - - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) - } } func TestFailCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeReplicaSet("fail")) - status, err := replicaSetStatus(c.ReplicaSets(), "fail", map[string]string{successFactorKey: "80"}) + _, err := replicaSetProgress(c.ReplicaSets(), "fail") if err != nil { t.Error(err) } - - if status != interfaces.ResourceNotReady { - t.Errorf("status should be `not ready`, is `%s` instead.", status) - } } diff --git a/pkg/resources/secrets.go b/pkg/resources/secrets.go index f408c89..7be18bf 100644 --- a/pkg/resources/secrets.go +++ b/pkg/resources/secrets.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" @@ -31,15 +30,13 @@ var secretParamFields = []string{ } type newSecret struct { - Base - Secret *v1.Secret - Client corev1.SecretInterface + secret *v1.Secret + client corev1.SecretInterface } type existingSecret struct { - Base - Name string - Client corev1.SecretInterface + name string + client corev1.SecretInterface } type secretTemplateFactory struct{} @@ -60,12 +57,12 @@ func (secretTemplateFactory) Kind() string { // New returns Secret controller for new resource based on resource definition func (secretTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { secret := parametrizeResource(def.Secret, gc, secretParamFields).(*v1.Secret) - return report.SimpleReporter{BaseResource: newSecret{Base: Base{def.Meta}, Secret: secret, Client: c.Secrets()}} + return newSecret{secret: secret, client: c.Secrets()} } // NewExisting returns Secret controller for existing resource by its name func (secretTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingSecret{Name: name, Client: c.Secrets()}} + return existingSecret{name: name, client: c.Secrets()} } func secretKey(name string) string { @@ -74,46 +71,46 @@ func secretKey(name string) string { // Key returns the Secret object identifier func (s newSecret) Key() string { - return secretKey(s.Secret.Name) + return secretKey(s.secret.Name) } // Key returns the Secret object identifier func (s existingSecret) Key() string { - return secretKey(s.Name) + return secretKey(s.name) } -func secretStatus(s corev1.SecretInterface, name string) (interfaces.ResourceStatus, error) { +func secretProgress(s corev1.SecretInterface, name string) (float32, error) { _, err := s.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - - return interfaces.ResourceReady, nil + return 1, nil } -// Status returns interfaces.ResourceReady if the secret is available in cluster -func (s newSecret) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return secretStatus(s.Client, s.Secret.Name) +// GetProgress returns Secret deployment progress +func (s newSecret) GetProgress() (float32, error) { + return secretProgress(s.client, s.secret.Name) } // Create looks for the Secret in k8s and creates it if not present func (s newSecret) Create() error { - if err := checkExistence(s); err != nil { - log.Println("Creating", s.Key()) - s.Secret, err = s.Client.Create(s.Secret) - return err + if checkExistence(s) { + return nil } - return nil + log.Println("Creating", s.Key()) + obj, err := s.client.Create(s.secret) + s.secret = obj + return err } // Delete deletes Secret from the cluster func (s newSecret) Delete() error { - return s.Client.Delete(s.Secret.Name, nil) + return s.client.Delete(s.secret.Name, nil) } -// Status returns interfaces.ResourceReady if the secret is available in cluster -func (s existingSecret) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return secretStatus(s.Client, s.Name) +// GetProgress returns Secret deployment progress +func (s existingSecret) GetProgress() (float32, error) { + return secretProgress(s.client, s.name) } // Create looks for existing Secret and returns error if there is no such Secret @@ -123,5 +120,5 @@ func (s existingSecret) Create() error { // Delete deletes Secret from the cluster func (s existingSecret) Delete() error { - return s.Client.Delete(s.Name, nil) + return s.client.Delete(s.name, nil) } diff --git a/pkg/resources/secrets_test.go b/pkg/resources/secrets_test.go index 82e29a0..040bb5f 100644 --- a/pkg/resources/secrets_test.go +++ b/pkg/resources/secrets_test.go @@ -17,34 +17,25 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestSecretSuccessCheck checks status of ready Secret func TestSecretSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeSecret("notfail")) - status, err := secretStatus(c.Secrets(), "notfail") + _, err := secretProgress(c.Secrets(), "notfail") if err != nil { t.Error(err) } - - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) - } } // TestSecretFailCheck checks status of not existing Secret func TestSecretFailCheck(t *testing.T) { c := mocks.NewClient() - status, err := secretStatus(c.Secrets(), "fail") + _, err := secretProgress(c.Secrets(), "fail") if err == nil { t.Error("error not found, expected error") } - - if status != interfaces.ResourceError { - t.Errorf("status should be `error`, is `%s` instead.", status) - } } diff --git a/pkg/resources/service.go b/pkg/resources/service.go index 5c3fcb2..1b02144 100644 --- a/pkg/resources/service.go +++ b/pkg/resources/service.go @@ -19,9 +19,7 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" - corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api" "k8s.io/client-go/pkg/api/v1" "k8s.io/client-go/pkg/apis/apps/v1beta1" @@ -33,10 +31,13 @@ var serviceParamFields = []string{ } type newService struct { - Base - Service *v1.Service - Client corev1.ServiceInterface - APIClient client.Interface + service *v1.Service + client client.Interface +} + +type existingService struct { + name string + client client.Interface } type serviceTemplateFactory struct{} @@ -57,71 +58,65 @@ func (serviceTemplateFactory) Kind() string { // New returns Service controller for new resource based on resource definition func (serviceTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { service := parametrizeResource(def.Service, gc, serviceParamFields).(*v1.Service) - return report.SimpleReporter{BaseResource: newService{Base: Base{def.Meta}, Service: service, Client: c.Services(), APIClient: c}} + return newService{service: service, client: c} } // NewExisting returns Service controller for existing resource by its name func (serviceTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingService{Name: name, Client: c.Services()}} + return existingService{name: name, client: c} } -func serviceStatus(s corev1.ServiceInterface, name string, apiClient client.Interface) (interfaces.ResourceStatus, error) { - service, err := s.Get(name) +func serviceProgress(client client.Interface, name string) (float32, error) { + service, err := client.Services().Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - log.Printf("Checking service status for selector %v", service.Spec.Selector) selector := labels.SelectorFromSet(service.Spec.Selector) options := v1.ListOptions{LabelSelector: selector.String()} - pods, err := apiClient.Pods().List(options) + pods, err := client.Pods().List(options) if err != nil { - return interfaces.ResourceError, err + return 0, err } - jobs, err := apiClient.Jobs().List(options) + jobs, err := client.Jobs().List(options) if err != nil { - return interfaces.ResourceError, err + return 0, err } - replicasets, err := apiClient.ReplicaSets().List(options) + replicasets, err := client.ReplicaSets().List(options) if err != nil { - return interfaces.ResourceError, err + return 0, err } - resources := make([]interfaces.BaseResource, 0, len(pods.Items)+len(jobs.Items)+len(replicasets.Items)) + resources := make([]interfaces.Resource, 0, len(pods.Items)+len(jobs.Items)+len(replicasets.Items)) for _, pod := range pods.Items { - resources = append(resources, createNewPod(&pod, apiClient.Pods(), nil)) + resources = append(resources, createNewPod(&pod, client.Pods())) } for _, j := range jobs.Items { - resources = append(resources, createNewJob(&j, apiClient.Jobs(), nil)) + resources = append(resources, createNewJob(&j, client.Jobs())) } for _, r := range replicasets.Items { - resources = append(resources, createNewReplicaSet(&r, apiClient.ReplicaSets(), nil)) + resources = append(resources, createNewReplicaSet(&r, client.ReplicaSets())) } - if apiClient.IsEnabled(v1beta1.SchemeGroupVersion) { - statefulsets, err := apiClient.StatefulSets().List(options) + if client.IsEnabled(v1beta1.SchemeGroupVersion) { + statefulsets, err := client.StatefulSets().List(options) if err != nil { - return interfaces.ResourceError, err + return 0, err } for _, ps := range statefulsets.Items { - resources = append(resources, newStatefulSet(&ps, apiClient.StatefulSets(), apiClient, nil)) + resources = append(resources, createNewStatefulSet(&ps, client)) } } else { - petsets, err := apiClient.PetSets().List(api.ListOptions{LabelSelector: selector}) + petsets, err := client.PetSets().List(api.ListOptions{LabelSelector: selector}) if err != nil { - return interfaces.ResourceError, err + return 0, err } for _, ps := range petsets.Items { - resources = append(resources, newPetSet(&ps, apiClient.PetSets(), apiClient, nil)) + resources = append(resources, createNewPetSet(&ps, client)) } } - status, err := resourceListStatus(resources) - if status != interfaces.ResourceReady || err != nil { - return status, err - } - - return interfaces.ResourceReady, nil + return resourceListProgress(resources) } func serviceKey(name string) string { @@ -130,39 +125,33 @@ func serviceKey(name string) string { // Key returns service name func (s newService) Key() string { - return serviceKey(s.Service.Name) + return serviceKey(s.service.Name) } // Create looks for the Service in k8s and creates it if not present func (s newService) Create() error { - if err := checkExistence(s); err != nil { - log.Println("Creating", s.Key()) - s.Service, err = s.Client.Create(s.Service) - return err + if checkExistence(s) { + return nil } - return nil + log.Println("Creating", s.Key()) + obj, err := s.client.Services().Create(s.service) + s.service = obj + return err } // Delete deletes Service from the cluster func (s newService) Delete() error { - return s.Client.Delete(s.Service.Name, nil) -} - -// Status returns Service Status. It is based on the status of all objects which match the service selector. If all of them are ready, the Service is considered ready. -func (s newService) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return serviceStatus(s.Client, s.Service.Name, s.APIClient) + return s.client.Services().Delete(s.service.Name, nil) } -type existingService struct { - Base - Name string - Client corev1.ServiceInterface - APIClient client.Interface +// GetProgress returns Service deployment progress +func (s newService) GetProgress() (float32, error) { + return serviceProgress(s.client, s.service.Name) } // Key returns service name func (s existingService) Key() string { - return serviceKey(s.Name) + return serviceKey(s.name) } // Create looks for existing Service and returns error if there is no such Service @@ -170,12 +159,12 @@ func (s existingService) Create() error { return createExistingResource(s) } -// Status returns Service Status. It is based on the status of all objects which match the service selector. If all of them are ready, the Service is considered ready. -func (s existingService) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return serviceStatus(s.Client, s.Name, s.APIClient) +// GetProgress returns Service deployment progress +func (s existingService) GetProgress() (float32, error) { + return serviceProgress(s.client, s.name) } // Delete deletes Service from the cluster func (s existingService) Delete() error { - return s.Client.Delete(s.Name, nil) + return s.client.Services().Delete(s.name, nil) } diff --git a/pkg/resources/service_test.go b/pkg/resources/service_test.go index fabd900..3019568 100644 --- a/pkg/resources/service_test.go +++ b/pkg/resources/service_test.go @@ -15,24 +15,22 @@ package resources import ( - "fmt" "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestCheckServiceStatusReady checks if the service status check is fine for healthy service func TestCheckServiceStatusReady(t *testing.T) { c := mocks.NewClient(mocks.MakeService("success")) - status, err := serviceStatus(c.Services(), "success", c) + progress, err := serviceProgress(c, "success") if err != nil { t.Errorf("%s", err) } - if status != interfaces.ResourceReady { - t.Errorf("service should be `ready`, is `%s` instead", status) + if progress != 1 { + t.Errorf("progress must be 1 but got %v", progress) } } @@ -42,18 +40,13 @@ func TestCheckServiceStatusPodNotReady(t *testing.T) { pod := mocks.MakePod("error") pod.Labels = svc.Spec.Selector c := mocks.NewClient(svc, pod) - status, err := serviceStatus(c.Services(), "failedpod", c) + progress, err := serviceProgress(c, "failedpod") - if err == nil { - t.Fatal("error should be returned, got nil") - } - expectedError := fmt.Sprintf("resource pod/%v is not ready", pod.Name) - if err.Error() != expectedError { - t.Errorf("expected `%s` as error, got `%s`", expectedError, err.Error()) + if err != nil { + t.Fatal(err) } - - if status != interfaces.ResourceNotReady { - t.Errorf("service should be `not ready`, is `%s` instead", status) + if progress != 0 { + t.Errorf("progress must be 0 but got %v", progress) } } @@ -63,19 +56,14 @@ func TestCheckServiceStatusJobNotReady(t *testing.T) { job := mocks.MakeJob("error") job.Labels = svc.Spec.Selector c := mocks.NewClient(svc, job) - status, err := serviceStatus(c.Services(), "failedjob", c) + progress, err := serviceProgress(c, "failedjob") - if err == nil { - t.Error("error should be returned, got nil") - } - - expectedError := fmt.Sprintf("resource job/%v is not ready", job.Name) - if err.Error() != expectedError { - t.Errorf("expected `%s` as error, got `%s`", expectedError, err.Error()) + if err != nil { + t.Fatal(err) } - if status != interfaces.ResourceNotReady { - t.Errorf("service should be `not ready`, is `%s` instead", status) + if progress != 0 { + t.Errorf("progress must be 0 but got %v", progress) } } @@ -85,19 +73,14 @@ func TestCheckServiceStatusReplicaSetNotReady(t *testing.T) { rc := mocks.MakeReplicaSet("fail") rc.Labels = svc.Spec.Selector c := mocks.NewClient(svc, rc) - status, err := serviceStatus(c.Services(), "failedrc", c) + progress, err := serviceProgress(c, "failedrc") - if err == nil { - t.Error("error should be returned, got nil") - } - - expectedError := fmt.Sprintf("resource replicaset/%v is not ready", rc.Name) - if err.Error() != expectedError { - t.Errorf("expected `%s` as error, got `%s`", expectedError, err.Error()) + if err != nil { + t.Fatal(err) } - if status != interfaces.ResourceNotReady { - t.Errorf("service should be `not ready`, is `%s` instead", status) + if progress != 0 { + t.Errorf("progress must be 0 but got %v", progress) } } @@ -124,8 +107,11 @@ func TestCheckServiceStatusOnPartialSelector(t *testing.T) { } c := mocks.NewClient(svc, rs, pod, job) - status, err := serviceStatus(c.Services(), "svc", c) + progress, err := serviceProgress(c, "svc") + if progress != 1 { + t.Errorf("progress must be 0.5 but got %v", progress) + } if err != nil { t.Fatalf("error should be nil, got %v", err) } @@ -138,17 +124,13 @@ func TestCheckServiceStatusOnPartialSelector(t *testing.T) { } c.ReplicaSets().Create(rs2) - status, err = serviceStatus(c.Services(), "svc", c) + progress, err = serviceProgress(c, "svc") - if err == nil { - t.Fatal("error should be returned, got nil") - } - expectedError := fmt.Sprintf("resource replicaset/%s is not ready", rs2.Name) - if err.Error() != expectedError { - t.Errorf("expected `%s` as error, got `%s`", expectedError, err.Error()) + if err != nil { + t.Error(err) } - if status != interfaces.ResourceNotReady { - t.Errorf("service should be `not ready`, is `%s` instead", status) + if progress != 0.5 { + t.Errorf("progress must be 0.5 but got %v", progress) } } diff --git a/pkg/resources/serviceaccount.go b/pkg/resources/serviceaccount.go index 68a9196..29344e1 100644 --- a/pkg/resources/serviceaccount.go +++ b/pkg/resources/serviceaccount.go @@ -19,7 +19,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" @@ -31,15 +30,13 @@ var serviceAccountParamFields = []string{ } type newServiceAccount struct { - Base - ServiceAccount *v1.ServiceAccount - Client corev1.ServiceAccountInterface + serviceAccount *v1.ServiceAccount + client corev1.ServiceAccountInterface } type existingServiceAccount struct { - Base - Name string - Client corev1.ServiceAccountInterface + name string + client corev1.ServiceAccountInterface } type serviceAccountTemplateFactory struct{} @@ -57,73 +54,72 @@ func (serviceAccountTemplateFactory) Kind() string { return "serviceaccount" } -// New returns ServiceAccount controller for new resource based on resource definition +// New returns serviceAccount controller for new resource based on resource definition func (serviceAccountTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { serviceAccount := parametrizeResource(def.ServiceAccount, gc, serviceAccountParamFields).(*v1.ServiceAccount) - return report.SimpleReporter{ - BaseResource: newServiceAccount{Base: Base{def.Meta}, ServiceAccount: serviceAccount, Client: c.ServiceAccounts()}, - } + return newServiceAccount{serviceAccount: serviceAccount, client: c.ServiceAccounts()} } -// NewExisting returns ServiceAccount controller for existing resource by its name +// NewExisting returns serviceAccount controller for existing resource by its name func (serviceAccountTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: existingServiceAccount{Name: name, Client: c.ServiceAccounts()}} + return existingServiceAccount{name: name, client: c.ServiceAccounts()} } func serviceAccountKey(name string) string { return "serviceaccount/" + name } -// Key returns ServiceAccount name +// Key returns serviceAccount name func (c newServiceAccount) Key() string { - return serviceAccountKey(c.ServiceAccount.Name) + return serviceAccountKey(c.serviceAccount.Name) } -func serviceAccountStatus(c corev1.ServiceAccountInterface, name string) (interfaces.ResourceStatus, error) { +func serviceAccountProgress(c corev1.ServiceAccountInterface, name string) (float32, error) { _, err := c.Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - return interfaces.ResourceReady, nil + return 1, nil } -// Status returns ServiceAccount status -func (c newServiceAccount) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return serviceAccountStatus(c.Client, c.ServiceAccount.Name) +// GetProgress returns ServiceAccount deployment progress +func (c newServiceAccount) GetProgress() (float32, error) { + return serviceAccountProgress(c.client, c.serviceAccount.Name) } -// Create looks for the ServiceAccount in k8s and creates it if not present +// Create looks for the serviceAccount in k8s and creates it if not present func (c newServiceAccount) Create() error { - if err := checkExistence(c); err != nil { - log.Println("Creating", c.Key()) - c.ServiceAccount, err = c.Client.Create(c.ServiceAccount) - return err + if checkExistence(c) { + return nil } - return nil + log.Println("Creating", c.Key()) + obj, err := c.client.Create(c.serviceAccount) + c.serviceAccount = obj + return err } -// Delete deletes ServiceAccount from the cluster +// Delete deletes serviceAccount from the cluster func (c newServiceAccount) Delete() error { - return c.Client.Delete(c.ServiceAccount.Name, &v1.DeleteOptions{}) + return c.client.Delete(c.serviceAccount.Name, &v1.DeleteOptions{}) } -// Key returns ServiceAccount name +// Key returns serviceAccount name func (c existingServiceAccount) Key() string { - return serviceAccountKey(c.Name) + return serviceAccountKey(c.name) } -// Status returns ServiceAccount status -func (c existingServiceAccount) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return serviceAccountStatus(c.Client, c.Name) +// GetProgress returns serviceAccount deployment progress +func (c existingServiceAccount) GetProgress() (float32, error) { + return serviceAccountProgress(c.client, c.name) } -// Create looks for existing ServiceAccount and returns error if there is no such ServiceAccount +// Create looks for existing serviceAccount and returns error if there is no such serviceAccount func (c existingServiceAccount) Create() error { return createExistingResource(c) } -// Delete deletes ServiceAccount from the cluster +// Delete deletes serviceAccount from the cluster func (c existingServiceAccount) Delete() error { - return c.Client.Delete(c.Name, nil) + return c.client.Delete(c.name, nil) } diff --git a/pkg/resources/serviceaccount_test.go b/pkg/resources/serviceaccount_test.go index 5bbc081..5bbb6cf 100644 --- a/pkg/resources/serviceaccount_test.go +++ b/pkg/resources/serviceaccount_test.go @@ -17,34 +17,29 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" ) // TestServiceAccountSuccessCheck checks status of ready ServiceAccount func TestServiceAccountSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.ServiceAccounts("notfail")) - status, err := serviceAccountStatus(c.ServiceAccounts(), "notfail") + progress, err := serviceAccountProgress(c.ServiceAccounts(), "notfail") if err != nil { t.Error(err) } - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) + if progress != 1 { + t.Errorf("progress must be 0 but got %v", progress) } } // TestServiceAccountFailCheck checks status of not existing ServiceAccount func TestServiceAccountFailCheck(t *testing.T) { c := mocks.NewClient(mocks.ServiceAccounts()) - status, err := serviceAccountStatus(c.ServiceAccounts(), "fail") + _, err := serviceAccountProgress(c.ServiceAccounts(), "fail") if err == nil { t.Error("error not found, expected error") } - - if status != interfaces.ResourceError { - t.Errorf("status should be `error`, is `%s` instead.", status) - } } diff --git a/pkg/resources/statefulset.go b/pkg/resources/statefulset.go index e6f4c74..5c43425 100644 --- a/pkg/resources/statefulset.go +++ b/pkg/resources/statefulset.go @@ -19,9 +19,7 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" - "k8s.io/client-go/kubernetes/typed/apps/v1beta1" appsbeta1 "k8s.io/client-go/pkg/apis/apps/v1beta1" ) @@ -33,12 +31,16 @@ var statefulSetParamFields = []string{ "Spec.Template.ObjectMeta", } -// StatefulSet is a wrapper for K8s StatefulSet object -type StatefulSet struct { - Base - StatefulSet *appsbeta1.StatefulSet - Client v1beta1.StatefulSetInterface - APIClient client.Interface +// newStatefulSet is a wrapper for K8s StatefulSet object +type newStatefulSet struct { + statefulSet *appsbeta1.StatefulSet + client client.Interface +} + +// existingStatefulSet is a wrapper for K8s StatefulSet object which is meant to already be in a cluster before AppController execution +type existingStatefulSet struct { + name string + client client.Interface } type statefulSetTemplateFactory struct{} @@ -59,21 +61,21 @@ func (statefulSetTemplateFactory) Kind() string { // New returns StatefulSet controller for new resource based on resource definition func (statefulSetTemplateFactory) New(def client.ResourceDefinition, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { statefulSet := parametrizeResource(def.StatefulSet, gc, statefulSetParamFields).(*appsbeta1.StatefulSet) - return newStatefulSet(statefulSet, c.StatefulSets(), c, def.Meta) + return createNewStatefulSet(statefulSet, c) } // NewExisting returns StatefulSet controller for existing resource by its name func (statefulSetTemplateFactory) NewExisting(name string, c client.Interface, gc interfaces.GraphContext) interfaces.Resource { - return report.SimpleReporter{BaseResource: ExistingStatefulSet{Name: name, Client: c.StatefulSets(), APIClient: c}} + return existingStatefulSet{name: name, client: c} } -func statefulsetStatus(p v1beta1.StatefulSetInterface, name string, apiClient client.Interface) (interfaces.ResourceStatus, error) { +func statefulsetProgress(client client.Interface, name string) (float32, error) { // Use label from StatefulSet spec to get needed pods - ps, err := p.Get(name) + ps, err := client.StatefulSets().Get(name) if err != nil { - return interfaces.ResourceError, err + return 0, err } - return podsStateFromLabels(apiClient, ps.Spec.Template.ObjectMeta.Labels) + return podsStateFromLabels(client, ps.Spec.Template.ObjectMeta.Labels) } func statefulsetKey(name string) string { @@ -81,58 +83,51 @@ func statefulsetKey(name string) string { } // Key returns StatefulSet name -func (p StatefulSet) Key() string { - return statefulsetKey(p.StatefulSet.Name) +func (p newStatefulSet) Key() string { + return statefulsetKey(p.statefulSet.Name) } // Create looks for the StatefulSet in Kubernetes cluster and creates it if it's not there -func (p StatefulSet) Create() error { - if err := checkExistence(p); err != nil { - log.Println("Creating", p.Key()) - _, err = p.Client.Create(p.StatefulSet) - return err +func (p newStatefulSet) Create() error { + if checkExistence(p) { + return nil } - return nil + log.Println("Creating", p.Key()) + obj, err := p.client.StatefulSets().Create(p.statefulSet) + p.statefulSet = obj + return err } // Delete deletes StatefulSet from the cluster -func (p StatefulSet) Delete() error { - return p.Client.Delete(p.StatefulSet.Name, nil) -} - -// Status returns StatefulSet status. interfaces.ResourceReady is regarded as sufficient for it's dependencies to be created. -func (p StatefulSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return statefulsetStatus(p.Client, p.StatefulSet.Name, p.APIClient) +func (p newStatefulSet) Delete() error { + return p.client.StatefulSets().Delete(p.statefulSet.Name, nil) } -func newStatefulSet(statefulset *appsbeta1.StatefulSet, client v1beta1.StatefulSetInterface, apiClient client.Interface, meta map[string]interface{}) interfaces.Resource { - return report.SimpleReporter{BaseResource: StatefulSet{Base: Base{meta}, StatefulSet: statefulset, Client: client, APIClient: apiClient}} +// GetProgress returns StatefulSet deployment progress +func (p newStatefulSet) GetProgress() (float32, error) { + return statefulsetProgress(p.client, p.statefulSet.Name) } -// ExistingStatefulSet is a wrapper for K8s StatefulSet object which is meant to already be in a cluster before AppController execution -type ExistingStatefulSet struct { - Base - Name string - Client v1beta1.StatefulSetInterface - APIClient client.Interface +func createNewStatefulSet(statefulset *appsbeta1.StatefulSet, client client.Interface) interfaces.Resource { + return newStatefulSet{statefulSet: statefulset, client: client} } // Key returns StatefulSet name -func (p ExistingStatefulSet) Key() string { - return statefulsetKey(p.Name) +func (p existingStatefulSet) Key() string { + return statefulsetKey(p.name) } // Create looks for existing StatefulSet and returns error if there is no such StatefulSet -func (p ExistingStatefulSet) Create() error { +func (p existingStatefulSet) Create() error { return createExistingResource(p) } -// Status returns StatefulSet status. interfaces.ResourceReady is regarded as sufficient for it's dependencies to be created. -func (p ExistingStatefulSet) Status(meta map[string]string) (interfaces.ResourceStatus, error) { - return statefulsetStatus(p.Client, p.Name, p.APIClient) +// GetProgress returns StatefulSet deployment progress +func (p existingStatefulSet) GetProgress() (float32, error) { + return statefulsetProgress(p.client, p.name) } // Delete deletes StatefulSet from the cluster -func (p ExistingStatefulSet) Delete() error { - return p.Client.Delete(p.Name, nil) +func (p existingStatefulSet) Delete() error { + return p.client.StatefulSets().Delete(p.name, nil) } diff --git a/pkg/resources/statefulset_test.go b/pkg/resources/statefulset_test.go index 9474fad..74e690f 100644 --- a/pkg/resources/statefulset_test.go +++ b/pkg/resources/statefulset_test.go @@ -17,7 +17,6 @@ package resources import ( "testing" - "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" "k8s.io/client-go/pkg/apis/apps/v1beta1" @@ -26,14 +25,14 @@ import ( // TestStatefulSetSuccessCheck checks status of ready StatefulSet func TestStatefulSetSuccessCheck(t *testing.T) { c := mocks.NewClient(mocks.MakeStatefulSet("notfail")) - status, err := statefulsetStatus(c.StatefulSets(), "notfail", c) + progress, err := statefulsetProgress(c, "notfail") if err != nil { t.Error(err) } - if status != interfaces.ResourceReady { - t.Errorf("status should be `ready`, is `%s` instead.", status) + if progress != 1 { + t.Errorf("progress must be 1 but got %v", progress) } } @@ -43,16 +42,16 @@ func TestStatefulSetFailCheck(t *testing.T) { pod := mocks.MakePod("fail") pod.Labels = ss.Spec.Template.ObjectMeta.Labels c := mocks.NewClient(ss, pod) - status, err := statefulsetStatus(c.StatefulSets(), "fail", c) + progress, err := statefulsetProgress(c, "fail") - expectedError := "resource pod/fail is not ready" - if err.Error() != expectedError { - t.Errorf("expected `%s` as error, got `%s`", expectedError, err.Error()) + if err != nil { + t.Error(err) } - if status != interfaces.ResourceNotReady { - t.Errorf("status should be `not ready`, is `%s` instead.", status) + if progress != 0 { + t.Errorf("progress must be 0 but got %v", progress) } + } func TestStatefulSetIsEnabled(t *testing.T) { diff --git a/pkg/resources/void.go b/pkg/resources/void.go index 3459fe4..09f7051 100644 --- a/pkg/resources/void.go +++ b/pkg/resources/void.go @@ -17,13 +17,9 @@ package resources import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" ) -type void struct { - Base - name string -} +type void string type voidTemplateFactory struct{} @@ -45,17 +41,17 @@ func (voidTemplateFactory) New(_ client.ResourceDefinition, _ client.Interface, // NewExisting returns new void with specified name func (voidTemplateFactory) NewExisting(name string, _ client.Interface, gc interfaces.GraphContext) interfaces.Resource { name = parametrizeResource(name, gc, []string{"*"}).(string) - return report.SimpleReporter{BaseResource: void{name: name}} + return void(name) } // Key returns void name func (v void) Key() string { - return "void/" + v.name + return "void/" + string(v) } // Status always returns "ready" -func (void) Status(_ map[string]string) (interfaces.ResourceStatus, error) { - return interfaces.ResourceReady, nil +func (void) GetProgress() (float32, error) { + return 1, nil } // Delete is a no-op method to create resource diff --git a/pkg/scheduler/dependency_graph.go b/pkg/scheduler/dependency_graph.go index c093c0e..9fcf8b9 100644 --- a/pkg/scheduler/dependency_graph.go +++ b/pkg/scheduler/dependency_graph.go @@ -88,16 +88,17 @@ func (gc graphContext) Graph() interfaces.DependencyGraph { } // newScheduledResourceFor returns new scheduled resource for given resource in init state -func newScheduledResourceFor(r interfaces.Resource, suffix string, context *graphContext, existing bool) *scheduledResource { +func newScheduledResourceFor(r interfaces.Resource, suffix string, context *graphContext, existing bool, meta map[string]interface{}) *scheduledResource { return &scheduledResource{ - started: false, - ignored: false, - error: nil, - Resource: r, - meta: map[string]map[string]string{}, - context: context, - existing: existing, - suffix: copier.EvaluateString(suffix, getArgFunc(context)), + started: false, + ignored: false, + error: nil, + Resource: r, + dependenciesMeta: map[string]map[string]string{}, + resourceMeta: meta, + context: context, + existing: existing, + suffix: copier.EvaluateString(suffix, getArgFunc(context)), } } @@ -276,26 +277,28 @@ func (sched scheduler) newScheduledResource(kind, name, suffix string, resDefs m return nil, fmt.Errorf("not a proper resource kind: %s. Expected '%s'", kind, strings.Join(resources.Kinds, "', '")) } - r, existing, err := sched.newResource(kind, name, resDefs, gc, resourceTemplate, silent) + r, existing, meta, err := sched.newResource(kind, name, resDefs, gc, resourceTemplate, silent) if err != nil { return nil, err } - return newScheduledResourceFor(r, suffix, gc, existing), nil + return newScheduledResourceFor(r, suffix, gc, existing, meta), nil } // newResource returns creates a resource controller for a given resources name and factory. // It returns the created controller (implementation of interfaces.Resource), flag saying if the created controller -// will create new resource or check the status of existing resource outside (i.e. that doesn't have resource defintition) +// will create new resource or check the status of existing resource outside (i.e. that doesn't have resource definition) // and error (or nil, if no error happened) func (sched scheduler) newResource(kind, name string, resDefs map[string]client.ResourceDefinition, - gc interfaces.GraphContext, resourceTemplate interfaces.ResourceTemplate, silent bool) (interfaces.Resource, bool, error) { + gc interfaces.GraphContext, resourceTemplate interfaces.ResourceTemplate, + silent bool) (interfaces.Resource, bool, map[string]interface{}, error) { + rd, ok := resDefs[kind+"/"+name] if ok { if !silent { log.Printf("Found resource definition for %s/%s", kind, name) } - return resourceTemplate.New(rd, sched.client, gc), false, nil + return resourceTemplate.New(rd, sched.client, gc), false, rd.Meta, nil } if !silent { @@ -304,9 +307,9 @@ func (sched scheduler) newResource(kind, name string, resDefs map[string]client. name = copier.EvaluateString(name, getArgFunc(gc)) r := resourceTemplate.NewExisting(name, sched.client, gc) if r == nil { - return nil, true, fmt.Errorf("existing resource %s/%s cannot be reffered", kind, name) + return nil, true, nil, fmt.Errorf("existing resource %s/%s cannot be reffered", kind, name) } - return r, true, nil + return r, true, nil, nil } func keyParts(key string) (kind, name, suffix string, err error) { @@ -718,68 +721,6 @@ func listDependencies(dependencies map[string][]client.Dependency, parent string return result } -func permute(variants [][]string) [][]string { - switch len(variants) { - case 0: - return variants - case 1: - var result [][]string - for _, v := range variants[0] { - result = append(result, []string{v}) - } - return result - default: - var result [][]string - for _, tail := range variants[len(variants)-1] { - for _, p := range permute(variants[:len(variants)-1]) { - result = append(result, append(p, tail)) - } - } - return result - } -} - -func expandListExpression(expr string) []string { - var result []string - for _, part := range strings.Split(expr, ",") { - part = strings.TrimSpace(part) - if part == "" { - continue - } - - isRange := true - var from, to int - - rangeParts := strings.SplitN(part, "..", 2) - if len(rangeParts) != 2 { - isRange = false - } - - var err error - if isRange { - from, err = strconv.Atoi(rangeParts[0]) - if err != nil { - isRange = false - } - } - if isRange { - to, err = strconv.Atoi(rangeParts[1]) - if err != nil { - isRange = false - } - } - - if isRange { - for i := from; i <= to; i++ { - result = append(result, strconv.Itoa(i)) - } - } else { - result = append(result, part) - } - } - return result -} - type interimGraphVertex struct { dependency client.Dependency scheduledResource *scheduledResource @@ -815,8 +756,10 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, } } parentContext := replicaContext + distance := 1 if parent.scheduledResource != nil { parentContext = parent.scheduledResource.context + distance = parent.scheduledResource.distance + 1 } kind, name, suffix, err := keyParts(dep.Child) @@ -827,6 +770,7 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, return err } sr.usedInReplicas = []string{replicaName} + sr.distance = distance vertex := interimGraphVertex{ scheduledResource: sr, @@ -838,7 +782,7 @@ func (sched *scheduler) fillDependencyGraph(rootContext *graphContext, if parent.scheduledResource != nil { sr.requires = append(sr.requires, parent.scheduledResource.Key()) parent.scheduledResource.requiredBy = append(parent.scheduledResource.requiredBy, sr.Key()) - sr.meta[parent.dependency.Child] = dep.Meta + sr.dependenciesMeta[parent.dependency.Child] = dep.Meta } queue.PushBack(vertex) } @@ -866,12 +810,13 @@ func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc options interfaces.DependencyGraphOptions) { graph := gc.graph.graph var previousReplicaGraph map[string]*scheduledResource + maxDistance := 0 for i, replicaVertices := range vertices { replicaGraph := map[string]*scheduledResource{} - sched.mergeInterimGraphVertices(replicaVertices, replicaGraph, options) + replicaDistance := sched.mergeInterimGraphVertices(replicaVertices, replicaGraph, options) if i > 0 { - correctDuplicateResources(graph, replicaGraph, i) + adjustConcatenatedReplicaResources(graph, replicaGraph, i, maxDistance) for _, leafName := range getLeafs(previousReplicaGraph) { for _, rootName := range getRoots(replicaGraph) { @@ -886,15 +831,17 @@ func (sched *scheduler) concatenateReplicas(vertices [][]interimGraphVertex, gc for key, value := range replicaGraph { graph[key] = value } + maxDistance += replicaDistance } } -func correctDuplicateResources(existingGraph, newGraph map[string]*scheduledResource, index int) { +func adjustConcatenatedReplicaResources(existingGraph, newGraph map[string]*scheduledResource, index, startDistance int) { toReplace := map[string]*scheduledResource{} for key, sr := range newGraph { if existingGraph[key] != nil { toReplace[key] = sr } + sr.distance += startDistance } for key, sr := range toReplace { sr.context.id = existingGraph[key].context.id @@ -951,8 +898,9 @@ func getLeafs(graph map[string]*scheduledResource) []string { } func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, graph map[string]*scheduledResource, - options interfaces.DependencyGraphOptions) { + options interfaces.DependencyGraphOptions) int { + maxDistance := 0 for _, entry := range vertices { key := entry.scheduledResource.Key() existingSr := graph[key] @@ -960,17 +908,22 @@ func (sched *scheduler) mergeInterimGraphVertices(vertices []interimGraphVertex, if !options.Silent { log.Printf("Adding resource %s to the dependency graph flow %s", key, options.FlowName) } - graph[key] = entry.scheduledResource + existingSr = entry.scheduledResource + graph[key] = existingSr } else { sched.updateContext(existingSr.context, entry.parentContext, entry.dependency) existingSr.requires = append(existingSr.requires, entry.scheduledResource.requires...) existingSr.requiredBy = append(existingSr.requiredBy, entry.scheduledResource.requiredBy...) existingSr.usedInReplicas = append(existingSr.usedInReplicas, entry.scheduledResource.usedInReplicas...) - for metaKey, metaValue := range entry.scheduledResource.meta { - existingSr.meta[metaKey] = metaValue + for metaKey, metaValue := range entry.scheduledResource.dependenciesMeta { + existingSr.dependenciesMeta[metaKey] = metaValue } } + if existingSr.distance > maxDistance { + maxDistance = existingSr.distance + } } + return maxDistance } // getResourceDestructors builds a list of functions, each of them delete one of replica resources diff --git a/pkg/scheduler/dependency_graph_test.go b/pkg/scheduler/dependency_graph_test.go index 07ae319..9cc7068 100644 --- a/pkg/scheduler/dependency_graph_test.go +++ b/pkg/scheduler/dependency_graph_test.go @@ -15,7 +15,6 @@ package scheduler import ( - "strings" "testing" "github.com/Mirantis/k8s-AppController/pkg/client" @@ -320,76 +319,3 @@ func TestDependencyToFlowMatching(t *testing.T) { } } } - -// TestPermute tests permute function -func TestPermute(t *testing.T) { - alphabets := [][]string{ - {"1", "2", "3"}, - {"+", "-"}, - {"a", "b"}, - {"="}, - } - - expected := map[string]bool{ - "1+a=": true, - "1+b=": true, - "1-a=": true, - "1-b=": true, - "2+a=": true, - "2+b=": true, - "2-a=": true, - "2-b=": true, - "3+a=": true, - "3+b=": true, - "3-a=": true, - "3-b=": true, - } - permutations := permute(alphabets) - for _, combination := range permutations { - combinationStr := strings.Join(combination, "") - if !expected[combinationStr] { - t.Errorf("unexpected combination %s", combinationStr) - } else { - delete(expected, combinationStr) - } - } - if len(expected) != 0 { - t.Error("not all combinations were generated") - } - - alphabets = append(alphabets, make([]string, 0)) - if len(permute(alphabets)) != 0 { - t.Error("empty alphabet didin't result in empty permutation list") - } -} - -// TestExpendListExpression tests list expression translation to list of strings -func TestExpendListExpression(t *testing.T) { - table := map[string][]string{ - "1": {"1"}, - "1..5": {"1", "2", "3", "4", "5"}, - "2..-1": {}, - "a, b": {"a", "b"}, - "a, b, 2..4": {"a", "b", "2", "3", "4"}, - "-1..1, 2..4, x": {"-1", "0", "1", "2", "3", "4", "x"}, - "a..b": {"a..b"}, - "..": {".."}, - "1...3": {"1...3"}, - "1..b": {"1..b"}, - "a..b, 1..3": {"a..b", "1", "2", "3"}, - "a..b, c..d": {"a..b", "c..d"}, - "": {}, - } - for expr, expected := range table { - result := expandListExpression(expr) - if len(result) != len(expected) { - t.Errorf("unexpected result length for expression %s: %d != %d", expr, len(result), len(expected)) - } else { - for i := range expected { - if expected[i] != result[i] { - t.Errorf("invalid entry %d for expression %s: %s != %s", i, expr, expected[i], result[i]) - } - } - } - } -} diff --git a/pkg/scheduler/frontend.go b/pkg/scheduler/frontend.go index 45174a4..7810fb2 100644 --- a/pkg/scheduler/frontend.go +++ b/pkg/scheduler/frontend.go @@ -182,11 +182,10 @@ func Deploy(sched interfaces.Scheduler, options interfaces.DependencyGraphOption return task, nil } -// GetStatus returns deployment status -func GetStatus(client client.Interface, selector labels.Selector, - options interfaces.DependencyGraphOptions) (interfaces.DeploymentStatus, interfaces.DeploymentReport, error) { +// GetStatusGraph returns deployment graph suitable for status queries +func GetStatusGraph(client client.Interface, selector labels.Selector, + options interfaces.DependencyGraphOptions) (interfaces.DependencyGraph, error) { - silent := options.Silent if options.FlowName == "" { options.FlowName = interfaces.DefaultFlowName } @@ -194,14 +193,6 @@ func GetStatus(client client.Interface, selector labels.Selector, options.FixedNumberOfReplicas = true options.Silent = true - if !silent { - log.Println("Getting status of flow", options.FlowName) - } sched := New(client, selector, 0) - graph, err := sched.BuildDependencyGraph(options) - if err != nil { - return interfaces.Empty, nil, err - } - status, report := graph.GetStatus() - return status, report, nil + return sched.BuildDependencyGraph(options) } diff --git a/pkg/scheduler/helpers.go b/pkg/scheduler/helpers.go new file mode 100644 index 0000000..604745a --- /dev/null +++ b/pkg/scheduler/helpers.go @@ -0,0 +1,138 @@ +// Copyright 2017 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 scheduler + +import ( + "log" + "strconv" + "strings" +) + +func permute(variants [][]string) [][]string { + switch len(variants) { + case 0: + return variants + case 1: + var result [][]string + for _, v := range variants[0] { + result = append(result, []string{v}) + } + return result + default: + var result [][]string + for _, tail := range variants[len(variants)-1] { + for _, p := range permute(variants[:len(variants)-1]) { + result = append(result, append(p, tail)) + } + } + return result + } +} + +func expandListExpression(expr string) []string { + var result []string + for _, part := range strings.Split(expr, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + isRange := true + var from, to int + + rangeParts := strings.SplitN(part, "..", 2) + if len(rangeParts) != 2 { + isRange = false + } + + var err error + if isRange { + from, err = strconv.Atoi(rangeParts[0]) + if err != nil { + isRange = false + } + } + if isRange { + to, err = strconv.Atoi(rangeParts[1]) + if err != nil { + isRange = false + } + } + + if isRange { + for i := from; i <= to; i++ { + result = append(result, strconv.Itoa(i)) + } + } else { + result = append(result, part) + } + } + return result +} + +func getSuccessFactor(meta map[string]string) float32 { + var factor string + var ok bool + if factor, ok = meta["successFactor"]; !ok { + if factor, ok = meta["success_factor"]; !ok { + factor = "100" + } + } + + f, err := strconv.ParseFloat(factor, 32) + if err != nil || f < 0 || f > 100 { + return 1 + } + return float32(f / 100) +} + +func getIntMeta(sr *scheduledResource, paramName string, defaultValue int) int { + value := sr.resourceMeta[paramName] + if value == nil { + return defaultValue + } + + intVal, ok := value.(int) + if ok { + return intVal + } + + floatVal, ok := value.(float64) + if ok { + return int(floatVal) + } + + 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, sr.Key(), value, defaultValue) + return defaultValue + +} + +func getStringMeta(sr *scheduledResource, paramName string, defaultValue string) string { + value := sr.resourceMeta[paramName] + if value == nil { + return defaultValue + } + + strVal, ok := value.(string) + if ok { + return strVal + } + log.Printf( + "Metadata parameter '%s' for resource '%s' is set to '%v' but it does not seem to be a string, using default value %s", + paramName, sr.Key(), value, defaultValue) + return defaultValue +} diff --git a/pkg/scheduler/helpers_test.go b/pkg/scheduler/helpers_test.go new file mode 100644 index 0000000..c87af1c --- /dev/null +++ b/pkg/scheduler/helpers_test.go @@ -0,0 +1,169 @@ +// Copyright 2017 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 scheduler + +import ( + "strings" + "testing" + + "github.com/Mirantis/k8s-AppController/pkg/mocks" +) + +// TestPermute tests permute function +func TestPermute(t *testing.T) { + alphabets := [][]string{ + {"1", "2", "3"}, + {"+", "-"}, + {"a", "b"}, + {"="}, + } + + expected := map[string]bool{ + "1+a=": true, + "1+b=": true, + "1-a=": true, + "1-b=": true, + "2+a=": true, + "2+b=": true, + "2-a=": true, + "2-b=": true, + "3+a=": true, + "3+b=": true, + "3-a=": true, + "3-b=": true, + } + permutations := permute(alphabets) + for _, combination := range permutations { + combinationStr := strings.Join(combination, "") + if !expected[combinationStr] { + t.Errorf("unexpected combination %s", combinationStr) + } else { + delete(expected, combinationStr) + } + } + if len(expected) != 0 { + t.Error("not all combinations were generated") + } + + alphabets = append(alphabets, make([]string, 0)) + if len(permute(alphabets)) != 0 { + t.Error("empty alphabet didin't result in empty permutation list") + } +} + +// TestExpendListExpression tests list expression translation to list of strings +func TestExpendListExpression(t *testing.T) { + table := map[string][]string{ + "1": {"1"}, + "1..5": {"1", "2", "3", "4", "5"}, + "2..-1": {}, + "a, b": {"a", "b"}, + "a, b, 2..4": {"a", "b", "2", "3", "4"}, + "-1..1, 2..4, x": {"-1", "0", "1", "2", "3", "4", "x"}, + "a..b": {"a..b"}, + "..": {".."}, + "1...3": {"1...3"}, + "1..b": {"1..b"}, + "a..b, 1..3": {"a..b", "1", "2", "3"}, + "a..b, c..d": {"a..b", "c..d"}, + "": {}, + } + for expr, expected := range table { + result := expandListExpression(expr) + if len(result) != len(expected) { + t.Errorf("unexpected result length for expression %s: %d != %d", expr, len(result), len(expected)) + } else { + for i := range expected { + if expected[i] != result[i] { + t.Errorf("invalid entry %d for expression %s: %s != %s", i, expr, expected[i], result[i]) + } + } + } + } +} + +// TestGetStringMeta checks metadata retrieval from a resource +func TestGetStringMeta(t *testing.T) { + r := &scheduledResource{Resource: mocks.NewResource("fake", 0)} + + if getStringMeta(r, "non-existing key", "default") != "default" { + t.Error("GetStringMeta for non-existing key returned not a default value") + } + + r = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: 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 = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: map[string]interface{}{"key": 1}, + } + + if getStringMeta(r, "key", "default") != "default" { + t.Error("GetStringMeta for non-string value returned not a default value") + } + + r = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: 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 := &scheduledResource{Resource: mocks.NewResource("fake", 0)} + + if getIntMeta(r, "non-existing key", -1) != -1 { + t.Error("GetIntMeta for non-existing key returned not a default value") + } + + r = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: 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") + } + + if getIntMeta(r, "key", -1) != -1 { + t.Error("GetIntMeta for non-int value returned not a default value") + } + + r = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: map[string]interface{}{"key": 42}, + } + if getIntMeta(r, "key", -1) != 42 { + t.Error("GetIntMeta returned not an actual value") + } + + r = &scheduledResource{ + Resource: mocks.NewResource("fake", 0), + resourceMeta: 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 73ad75f..6d62ff4 100644 --- a/pkg/scheduler/scheduler.go +++ b/pkg/scheduler/scheduler.go @@ -22,10 +22,8 @@ import ( "time" "github.com/Mirantis/k8s-AppController/pkg/interfaces" - "github.com/Mirantis/k8s-AppController/pkg/report" - "github.com/Mirantis/k8s-AppController/pkg/resources" - "k8s.io/client-go/pkg/api/errors" + api_errors "k8s.io/client-go/pkg/api/errors" "k8s.io/client-go/pkg/api/unversioned" ) @@ -37,20 +35,22 @@ const ( // scheduledResource is a wrapper for Resource with attached relationship data type scheduledResource struct { - requires []string - requiredBy []string - started bool - ignored bool - skipped bool - error error - existing bool - context *graphContext - usedInReplicas []string - status interfaces.ResourceStatus - suffix string + requires []string + requiredBy []string + started bool + ignored bool + skipped bool + error error + existing bool + context *graphContext + usedInReplicas []string + finished bool + distance int + suffix string + resourceMeta map[string]interface{} + dependenciesMeta map[string]map[string]string interfaces.Resource // parentKey -> dependencyMetadata - meta map[string]map[string]string sync.RWMutex } @@ -63,7 +63,7 @@ func (sr *scheduledResource) Key() string { return baseKey + "/" + sr.suffix } -// requestCreation does not create a scheduled resource immediately, but updates status +// requestCreation does not create a scheduled resource immediately, but updates state // and puts the scheduled resource to corresponding channel. Returns true if // scheduled resource creation was actually requested, false otherwise. func (sr *scheduledResource) requestCreation(toCreate chan<- *scheduledResource) bool { @@ -88,17 +88,14 @@ func (sr *scheduledResource) requestCreation(toCreate chan<- *scheduledResource) } func isResourceFinished(sr *scheduledResource) (bool, error) { - status, err := sr.Status(nil) - if err != nil { - return status != interfaces.ResourceNotReady, err - } - if status == interfaces.ResourceReady { - return true, nil + progress, err := sr.Resource.GetProgress() + if err == nil { + return progress == 1, nil } return false, nil } -// wait periodically checks resource status and returns if the resource processing is finished, +// wait periodically checks resource deployment progress and returns if the resource processing is finished, // regardless successful or not. The actual result of processing could be obtained from returned error. func (sr *scheduledResource) wait(checkInterval time.Duration, timeout time.Duration, stopChan <-chan struct{}) (bool, error) { log.Printf("%s flow: waiting for %v to be created", sr.context.graph.graphOptions.FlowName, sr.Key()) @@ -126,33 +123,38 @@ func (sr *scheduledResource) wait(checkInterval time.Duration, timeout time.Dura } } -// Status either returns cached copy of resource's status or retrieves it via Resource.Status. -// Only ResourceReady is cached to avoid inconsistency for resources that may go to failure state over time +// GetProgress either returns cached copy of resource's progress or retrieves it via Resource.GetProgress. +// Only 100% completion is cached to avoid inconsistency for resources that may go to failure state over time // so that if resource becomes ready it stays in this status for the whole deployment duration. // Errors returned by the resource are never cached, however if AC sees permanent problem with resource it may set the // error field -func (sr *scheduledResource) Status(meta map[string]string) (interfaces.ResourceStatus, error) { +func (sr *scheduledResource) GetProgress() (float32, error) { sr.Lock() defer sr.Unlock() - if sr.status != "" || sr.error != nil { - return sr.status, sr.error + if sr.error != nil { + return 0, sr.error + } + if sr.finished { + return 1, nil } - status, err := sr.Resource.Status(meta) - if err == nil && status == interfaces.ResourceReady { - sr.status = status + progress, err := sr.Resource.GetProgress() + if err == nil && progress == 1 { + sr.finished = true } - return status, err + return progress, err } -// isBlocked checks whether a scheduled resource can be created. It checks status of resources -// it depends on, via API. func (sr *scheduledResource) isBlocked() bool { - isBlocked := false + return len(sr.getBlockedBy()) > 0 +} + +func (sr *scheduledResource) getBlockedBy() []string { + blockedBy := make([]string, 0, len(sr.requires)) var permanentlyBlockedKeys []string skipped := true for _, reqKey := range sr.requires { - meta := sr.meta[reqKey] + meta := sr.dependenciesMeta[reqKey] onErrorValue, onErrorSet := meta["on-error"] onErrorSet = onErrorSet && onErrorValue != "false" req := sr.context.graph.graph[reqKey] @@ -165,18 +167,19 @@ func (sr *scheduledResource) isBlocked() bool { if ignored { continue } - status, err := req.Status(meta) + progress, err := req.GetProgress() if onErrorSet { if permanentError == nil { - isBlocked = true - if err == nil && status == interfaces.ResourceReady { + blockedBy = append(blockedBy, reqKey) + if err == nil && progress == 1 { permanentlyBlockedKeys = append(permanentlyBlockedKeys, reqKey) } } } else { skipped = false - if permanentError != nil || err != nil || status != interfaces.ResourceReady { - isBlocked = true + threshold := getSuccessFactor(meta) + if permanentError != nil || err != nil || progress < threshold { + blockedBy = append(blockedBy, reqKey) if permanentError != nil { permanentlyBlockedKeys = append(permanentlyBlockedKeys, reqKey) } @@ -190,7 +193,7 @@ func (sr *scheduledResource) isBlocked() bool { sr.Unlock() } - return isBlocked + return blockedBy } func createResources(toCreate chan *scheduledResource, finished chan<- *scheduledResource, ccLimiter chan struct{}, stopChan <-chan struct{}) { @@ -208,9 +211,9 @@ func createResources(toCreate chan *scheduledResource, finished chan<- *schedule <-ccLimiter }() - attempts := resources.GetIntMeta(r.Resource, "retry", 1) - timeoutInSeconds := resources.GetIntMeta(r.Resource, "timeout", -1) - onError := resources.GetStringMeta(r.Resource, "on-error", "") + attempts := getIntMeta(r, "retry", 1) + timeoutInSeconds := getIntMeta(r, "timeout", -1) + onError := getStringMeta(r, "on-error", "") waitTimeout := WaitTimeout if timeoutInSeconds >= 0 { @@ -372,65 +375,36 @@ func (depGraph dependencyGraph) Deploy(stopChan <-chan struct{}) bool { return result } -// getNodeReport acts as a more verbose version of isBlocked. It performs the -// same check as isBlocked, but returns the DeploymentReport -func (sr *scheduledResource) getNodeReport(name string) report.NodeReport { - var ready bool - isBlocked := false - dependencies := make([]interfaces.DependencyReport, 0, len(sr.requires)) - status, err := sr.Status(nil) - if err != nil { - ready = false - } else { - ready = status == "ready" - } - for _, rKey := range sr.requires { - r := sr.context.graph.graph[rKey] - r.RLock() - meta := r.meta[sr.Key()] - depReport := r.GetDependencyReport(meta) - r.RUnlock() - if depReport.Blocks { - isBlocked = true - } - dependencies = append(dependencies, depReport) - } - return report.NodeReport{ - Dependent: name, - Dependencies: dependencies, - Blocked: isBlocked, - Ready: ready, - } -} - -// GetStatus generates data for getting the status of deployment. Returns -// a DeploymentStatus and a human readable report string -// TODO: Allow for other formats of report (e.g. json for visualisations) -func (depGraph dependencyGraph) GetStatus() (interfaces.DeploymentStatus, interfaces.DeploymentReport) { - var readyExist, nonReadyExist bool - var status interfaces.DeploymentStatus - deploymentReport := make(report.DeploymentReport, 0, len(depGraph.graph)) - for key, resource := range depGraph.graph { - depReport := resource.getNodeReport(key) - deploymentReport = append(deploymentReport, depReport) - if depReport.Ready { - readyExist = true - } else { - nonReadyExist = true - } - } - switch { - case readyExist && nonReadyExist: - status = interfaces.Running - case readyExist: - status = interfaces.Finished - case nonReadyExist: - status = interfaces.Prepared - default: - status = interfaces.Empty - } - return status, deploymentReport -} +//// getNodeReport acts as a more verbose version of isBlocked. It performs the +//// same check as isBlocked, but returns the DeploymentReport +//func (sr *scheduledResource) getNodeReport(name string) report.NodeReport { +// var ready bool +// isBlocked := false +// dependencies := make([]interfaces.DependencyReport, 0, len(sr.requires)) +// status, err := sr.GetProgress(nil) +// if err != nil { +// ready = false +// } else { +// ready = status == "ready" +// } +// for _, rKey := range sr.requires { +// r := sr.context.graph.graph[rKey] +// r.RLock() +// meta := r.meta[sr.Key()] +// depReport := r.GetDependencyReport(meta) +// r.RUnlock() +// if depReport.Blocks { +// isBlocked = true +// } +// dependencies = append(dependencies, depReport) +// } +// return report.NodeReport{ +// Dependent: name, +// Dependencies: dependencies, +// Blocked: isBlocked, +// Ready: ready, +// } +//} func runConcurrently(funcs []func(<-chan struct{}) bool, concurrency int, stopChan <-chan struct{}) bool { if concurrency < 1 { @@ -469,7 +443,7 @@ func deleteResource(resource *scheduledResource) error { log.Printf("%s flow: Deleting resource %s", resource.context.flow.Name, resource.Key()) err := resource.Delete() if err != nil { - statusError, ok := err.(*errors.StatusError) + statusError, ok := err.(*api_errors.StatusError) if ok && statusError.Status().Reason == unversioned.StatusReasonNotFound { return nil } diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 484b56e..f6c5c57 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -25,7 +25,6 @@ import ( "github.com/Mirantis/k8s-AppController/pkg/client" "github.com/Mirantis/k8s-AppController/pkg/interfaces" "github.com/Mirantis/k8s-AppController/pkg/mocks" - "github.com/Mirantis/k8s-AppController/pkg/report" "github.com/Mirantis/k8s-AppController/pkg/resources" ) @@ -113,9 +112,9 @@ func TestIsBlocked(t *testing.T) { context := &graphContext{graph: depGraph} one := &scheduledResource{ - Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, - meta: map[string]map[string]string{}, - context: context, + Resource: mocks.NewResource("fake1", 0), + dependenciesMeta: map[string]map[string]string{}, + context: context, } depGraph.graph["fake1"] = one @@ -125,9 +124,9 @@ func TestIsBlocked(t *testing.T) { } two := &scheduledResource{ - Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake2", "ready")}, - meta: map[string]map[string]string{}, - context: context, + Resource: mocks.NewResource("fake2", 1), + dependenciesMeta: map[string]map[string]string{}, + context: context, } depGraph.graph["fake2"] = two @@ -144,8 +143,7 @@ func TestIsBlocked(t *testing.T) { } depGraph.graph["fake3"] = &scheduledResource{ - Resource: report.SimpleReporter{mocks.NewResource("fake3", "not ready")}, - meta: map[string]map[string]string{}, + Resource: mocks.NewResource("fake3", 0), context: context, } @@ -170,9 +168,9 @@ func TestIsBlockedWithOnErrorDependency(t *testing.T) { context := &graphContext{graph: depGraph} one := &scheduledResource{ - Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, - meta: map[string]map[string]string{}, - context: context, + Resource: mocks.NewResource("fake1", 0), + dependenciesMeta: map[string]map[string]string{"fake2": {"on-error": "true"}}, + context: context, } depGraph.graph["fake1"] = one @@ -181,14 +179,12 @@ func TestIsBlockedWithOnErrorDependency(t *testing.T) { } two := &scheduledResource{ - Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake2", "not ready")}, - meta: map[string]map[string]string{}, + Resource: mocks.NewResource("fake2", 0), context: context, } depGraph.graph["fake2"] = two one.requires = []string{"fake2"} - one.meta["fake2"] = map[string]string{"on-error": "true"} if !one.isBlocked() { t.Error("scheduled resource is not blocked but it must be") @@ -329,9 +325,9 @@ func TestLimitConcurrency(t *testing.T) { for i := 0; i < 15; i++ { key := fmt.Sprintf("resource%d", i) - r := report.SimpleReporter{BaseResource: mocks.NewCountingResource(key, counter, time.Second/4)} + r := mocks.NewCountingResource(key, counter, time.Second/4) context := &graphContext{graph: depGraph} - sr := newScheduledResourceFor(r, "", context, false) + sr := newScheduledResourceFor(r, "", context, false, nil) depGraph.graph[sr.Key()] = sr } stopChan := make(chan struct{}) @@ -342,7 +338,7 @@ func TestLimitConcurrency(t *testing.T) { concurrency = len(depGraph.graph) } if counter.Max() != concurrency { - t.Errorf("expected max concurrency counter %d, but got %d", concurrency, counter.Max()) + t.Errorf("expected max concurrency counter %d but got %d", concurrency, counter.Max()) } } } @@ -350,15 +346,15 @@ func TestLimitConcurrency(t *testing.T) { func TestStopBeforeDeploymentStarted(t *testing.T) { depGraph := newDependencyGraph(&scheduler{}, interfaces.DependencyGraphOptions{}) sr := &scheduledResource{ - Resource: report.SimpleReporter{BaseResource: mocks.NewResource("fake1", "not ready")}, + Resource: mocks.NewResource("fake1", 0), } depGraph.graph[sr.Key()] = sr stopChan := make(chan struct{}) close(stopChan) depGraph.Deploy(stopChan) - status, _ := sr.Resource.Status(nil) - if status == interfaces.ResourceReady { - t.Errorf("expected that resource %v wont be in ready status, but got %v", sr.Key(), status) + progress, _ := sr.Resource.GetProgress() + if progress != 0 { + t.Errorf("expected that resource %v wont be in ready progress but got %v", sr.Key(), progress) } } @@ -425,9 +421,9 @@ func TestEmptyStatus(t *testing.T) { if err != nil { t.Fatal(err) } - status, _ := depGraph.GetStatus() - if status != interfaces.Empty { - t.Errorf("expected status to be Empty, but got %s", status) + status := depGraph.GetDeploymentStatus() + if status.Total != 0 { + t.Errorf("expected total number to be zero but got %d", status.Total) } } @@ -446,9 +442,11 @@ func TestPreparedStatus(t *testing.T) { if err != nil { t.Fatal(err) } - status, _ := depGraph.GetStatus() - if status != interfaces.Prepared { - t.Errorf("expected status to be Prepared, but got %s", status) + status := depGraph.GetDeploymentStatus() + if status.Total != 2 || status.Failed > 0 || status.Skipped > 0 || status.Progress > 0 || status.Replicas != 1 || + status.Finished > 0 || status.TotalGroups != 2 { + + t.Errorf("got unexpected status %v", status) } } @@ -467,9 +465,11 @@ func TestRunningStatus(t *testing.T) { if err != nil { t.Fatal(err) } - status, _ := depGraph.GetStatus() - if status != interfaces.Running { - t.Errorf("expected status to be Running, but got %s", status) + status := depGraph.GetDeploymentStatus() + if status.Total != 2 || status.Failed > 0 || status.Skipped > 0 || status.Progress != 0.5 || status.Replicas != 1 || + status.Finished != 1 || status.TotalGroups != 2 { + + t.Errorf("got unexpected status %v", status) } } @@ -488,14 +488,16 @@ func TestFinishedStatus(t *testing.T) { if err != nil { t.Fatal(err) } - status, _ := depGraph.GetStatus() - if status != interfaces.Finished { - t.Errorf("expected status to be Finished, but got %s", status) + status := depGraph.GetDeploymentStatus() + if status.Total != 2 || status.Failed > 0 || status.Skipped > 0 || status.Progress != 1 || status.Replicas != 1 || + status.Finished != 2 || status.TotalGroups != 2 { + + t.Errorf("got unexpected status %v", status) } } -// TestGraph tests a simple DependencyGraph report -func TestGraph(t *testing.T) { +// TestGraphNodeStatuses tests a node status report +func TestGraphNodeStatuses(t *testing.T) { c := mocks.NewClient( mocks.MakeJob("1"), mocks.MakeJob("ready-2"), @@ -513,35 +515,19 @@ func TestGraph(t *testing.T) { if err != nil { t.Fatal(err) } - status, rep := depGraph.GetStatus() - if status != interfaces.Running { - t.Errorf("expected status to be Running, but got %s", status) - } - - deploymentReport := rep.(report.DeploymentReport) - - if len(deploymentReport) != 3 { - t.Errorf("wrong length of a graph 3 != %d", len(deploymentReport)) - } - for _, nodeReport := range deploymentReport { - if nodeReport.Dependent == "job/1" { - if len(nodeReport.Dependencies) != 2 { - t.Errorf("wrong length of dependencies 2 != %d", len(nodeReport.Dependencies)) - for _, dependency := range nodeReport.Dependencies { - if dependency.Dependency == "job/ready-2" { - if dependency.Blocks { - t.Error("job 2 should not block") - } - } else if dependency.Dependency == "job/3" { - if !dependency.Blocks { - t.Error("job 3 should block") - } - } else { - t.Errorf("unexpected dependency %s", dependency.Dependency) - } - } - - } + nodeStatuses := depGraph.GetNodeStatuses() + + if len(nodeStatuses) != 3 { + t.Fatalf("wrong length of a graph 3 != %d", len(nodeStatuses)) + } + expected := []interfaces.NodeStatus{ + {Name: "job/3", Status: "Starting / in progress", Progress: 0}, + {Name: "job/ready-2", Status: "Finished", Progress: 100}, + {Name: "job/1", Status: "Waiting for job/3", Progress: 0}, + } + for i, e := range expected { + if e != nodeStatuses[i] { + t.Errorf("unexpected report entry %d", i) } } } @@ -599,7 +585,7 @@ func TestWaitWithZeroTimeout(t *testing.T) { graph := &dependencyGraph{graphOptions: options} gc := &graphContext{graph: graph} resource := resources.KindToResourceTemplate["pod"].New(resdef, mocks.NewClient(pod), gc) - sr := newScheduledResourceFor(resource, "", gc, false) + sr := newScheduledResourceFor(resource, "", gc, false, nil) stopChan := make(chan struct{}) defer close(stopChan) diff --git a/pkg/scheduler/status.go b/pkg/scheduler/status.go new file mode 100644 index 0000000..fbe2214 --- /dev/null +++ b/pkg/scheduler/status.go @@ -0,0 +1,141 @@ +// Copyright 2017 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 scheduler + +import ( + "fmt" + "sort" + "strings" + + "github.com/Mirantis/k8s-AppController/pkg/interfaces" +) + +// GetDeploymentStatus returns stat info of the dependency graph which allows to track its progress +func (depGraph dependencyGraph) GetDeploymentStatus() interfaces.DeploymentStatus { + result := interfaces.DeploymentStatus{Total: len(depGraph.graph)} + replicas := map[string]bool{} + resourceGroups := map[string]bool{} + + for _, resource := range depGraph.graph { + resourceGroups[resource.Resource.Key()] = true + progress, err := resource.GetProgress() + if err != nil { + continue + } + resource.RLock() + if resource.skipped { + result.Skipped++ + result.Progress++ + } else if resource.error != nil { + result.Failed++ + result.Progress++ + } else if resource.ignored { + result.Progress++ + } else if resource.finished { + result.Finished++ + result.Progress++ + } else { + result.Progress += progress + } + for _, r := range resource.usedInReplicas { + replicas[r] = true + } + resource.RUnlock() + } + if result.Total > 0 { + result.Progress /= float32(result.Total) + } + result.TotalGroups = len(resourceGroups) + result.Replicas = len(replicas) + return result +} + +type sortableNodeList []*scheduledResource + +// Len is the number of elements in the node list. +func (lst sortableNodeList) Len() int { + return len(lst) +} + +// Less compares two items in node list +func (lst sortableNodeList) Less(i, j int) bool { + if lst[i].distance == lst[j].distance { + return lst[i].Key() < lst[j].Key() + } + return lst[i].distance < lst[j].distance +} + +// Swap swaps elements in node list +func (lst sortableNodeList) Swap(i, j int) { + lst[i], lst[j] = lst[j], lst[i] +} + +var _ sort.Interface = sortableNodeList{} + +func (depGraph dependencyGraph) GetNodeStatuses() []interfaces.NodeStatus { + nodes := make(sortableNodeList, 0, len(depGraph.graph)) + for _, node := range depGraph.graph { + nodes = append(nodes, node) + } + sort.Sort(nodes) + + result := make([]interfaces.NodeStatus, 0, len(nodes)) + + for _, node := range nodes { + nodeStatus := interfaces.NodeStatus{Name: node.Key()} + progress, err := node.GetProgress() + node.RLock() + + switch { + case node.error != nil: + nodeStatus.Progress = 100 + if node.skipped { + nodeStatus.Status = "Skipped" + } else { + nodeStatus.Status = fmt.Sprintf("Failed: %v", node.error) + } + case node.ignored: + nodeStatus.Status = "Ignored" + nodeStatus.Progress = 100 + case node.finished: + nodeStatus.Status = "Finished" + nodeStatus.Progress = 100 + case node.started: + if err != nil { + nodeStatus.Status = fmt.Sprintf("Error: %v", err) + } else { + nodeStatus.Status = "In progress" + nodeStatus.Progress = int(progress * 100) + } + default: + blockedBy := node.getBlockedBy() + if len(blockedBy) == 0 { + if err != nil && progress > 0 { + nodeStatus.Status = "In progress" + nodeStatus.Progress = int(progress * 100) + } else { + nodeStatus.Status = "Starting / in progress" + } + } else if len(blockedBy) == len(node.requires) { + nodeStatus.Status = "Pending" + } else { + nodeStatus.Status = fmt.Sprintf("Waiting for %s", strings.Join(blockedBy, ", ")) + } + } + node.RUnlock() + result = append(result, nodeStatus) + } + return result +}