From fffd5af1199f7bc13540c6b3226b24b2dd01802a Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Fri, 6 May 2022 11:59:54 +0200 Subject: [PATCH 1/3] Create local variables for an action --- CHANGELOG.md | 1 + Makefile | 4 ++-- pkg/apis/deployment/v1/plan.go | 29 +++++++++++++++++++++-- pkg/deployment/reconcile/action.go | 6 ++++- pkg/deployment/reconcile/action_helper.go | 8 ++++++- pkg/deployment/reconcile/plan_executor.go | 7 +++++- 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ee674277..937806ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - (Refactor) Anonymous inspector functions - (Feature) Recursive OwnerReference discovery - (Maintenance) Add check make targets +- (Feature) Create support for local variables in actions. ## [1.2.11](https://github.com/arangodb/kube-arangodb/tree/1.2.11) (2022-04-30) - (Bugfix) Orphan PVC are not removed diff --git a/Makefile b/Makefile index 5544a664d..ab329d414 100644 --- a/Makefile +++ b/Makefile @@ -82,7 +82,7 @@ ifeq ($(DEBUG),true) DEBUG := true DOCKERFILE := Dockerfile.debug # required by DLV https://github.com/go-delve/delve/blob/master/Documentation/usage/dlv_exec.md - COMPILE_DEBUG_FLAGS := -gcflags="all=-N -l" + COMPILE_DEBUG_FLAGS := -gcflags="all=-N -l" -ldflags "-extldflags '-static'" else DEBUG := false DOCKERFILE := Dockerfile @@ -522,4 +522,4 @@ check-community: @$(MAKE) _check RELEASE_MODE=community _check: - @$(MAKE) fmt license-verify linter run-unit-tests bin \ No newline at end of file + @$(MAKE) fmt license-verify linter run-unit-tests bin diff --git a/pkg/apis/deployment/v1/plan.go b/pkg/apis/deployment/v1/plan.go index 581eb29cf..9f7872245 100644 --- a/pkg/apis/deployment/v1/plan.go +++ b/pkg/apis/deployment/v1/plan.go @@ -21,12 +21,13 @@ package v1 import ( - "github.com/arangodb/kube-arangodb/pkg/util" "github.com/dchest/uniuri" "k8s.io/apimachinery/pkg/api/equality" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" + + "github.com/arangodb/kube-arangodb/pkg/util" ) // ActionPriority define action priority @@ -225,6 +226,8 @@ type Action struct { Image string `json:"image,omitempty"` // Params additional parameters used for action Params map[string]string `json:"params,omitempty"` + // Locals additional storage for local variables which are produced during the action. + Locals map[string]string `json:"locals,omitempty"` } // Equal compares two Actions @@ -237,7 +240,29 @@ func (a Action) Equal(other Action) bool { util.TimeCompareEqualPointer(a.StartTime, other.StartTime) && a.Reason == other.Reason && a.Image == other.Image && - equality.Semantic.DeepEqual(a.Params, other.Params) + equality.Semantic.DeepEqual(a.Params, other.Params) && + equality.Semantic.DeepEqual(a.Locals, other.Locals) +} + +// AddLocal returns a copy of an action with set local variable. +// If a local variable already exits then it is overwritten. +func (a Action) AddLocal(key, value string) Action { + if a.Locals == nil { + a.Locals = map[string]string{} + } + + a.Locals[key] = value + + return a +} + +// GetLocal returns an action's local variable. +// If a variable does not exist then false is returned. +func (a Action) GetLocal(key string) (string, bool) { + // The Locals variable can be nil. + i, ok := a.Locals[key] + + return i, ok } // AddParam returns copy of action with set parameter diff --git a/pkg/deployment/reconcile/action.go b/pkg/deployment/reconcile/action.go index cf1a22f53..beadf0406 100644 --- a/pkg/deployment/reconcile/action.go +++ b/pkg/deployment/reconcile/action.go @@ -26,9 +26,10 @@ import ( "sync" "time" + "github.com/rs/zerolog" + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/arangodb/kube-arangodb/pkg/util/k8sutil/inspector/throttle" - "github.com/rs/zerolog" ) func GetAllActions() []api.ActionType { @@ -58,6 +59,9 @@ type Action interface { // MemberID Return the MemberID used / created in this action MemberID() string + + // GetLocals returns locals variable which can be added during the action. + GetLocals() map[string]string } // ActionPost keep interface which is executed after action is completed. diff --git a/pkg/deployment/reconcile/action_helper.go b/pkg/deployment/reconcile/action_helper.go index 9eb817906..4db24b4a9 100644 --- a/pkg/deployment/reconcile/action_helper.go +++ b/pkg/deployment/reconcile/action_helper.go @@ -23,8 +23,9 @@ package reconcile import ( "context" - api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" "github.com/rs/zerolog" + + api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1" ) type actionEmpty struct { @@ -90,3 +91,8 @@ type actionImpl struct { func (a actionImpl) MemberID() string { return *a.memberIDRef } + +// GetLocals returns locals variable which can be added during the action. +func (a actionImpl) GetLocals() map[string]string { + return a.action.Locals +} diff --git a/pkg/deployment/reconcile/plan_executor.go b/pkg/deployment/reconcile/plan_executor.go index eb32d507a..1afe0c8aa 100644 --- a/pkg/deployment/reconcile/plan_executor.go +++ b/pkg/deployment/reconcile/plan_executor.go @@ -168,7 +168,11 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte } for k, v := range planAction.Params { - logContext = logContext.Str(k, v) + logContext = logContext.Str("param."+k, v) + } + + for k, v := range planAction.Locals { + logContext = logContext.Str("local."+k, v) } log := logContext.Logger() @@ -245,6 +249,7 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte now := metav1.Now() plan[0].StartTime = &now + plan[0].Locals = action.GetLocals() } return plan, recall, nil From a0ecd00fd7a8c898bdb3ef9428206609742413a8 Mon Sep 17 00:00:00 2001 From: Tomasz Mielech Date: Fri, 6 May 2022 13:11:00 +0200 Subject: [PATCH 2/3] check variables after CheckProgress --- pkg/deployment/reconcile/plan_executor.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/deployment/reconcile/plan_executor.go b/pkg/deployment/reconcile/plan_executor.go index 1afe0c8aa..39b738025 100644 --- a/pkg/deployment/reconcile/plan_executor.go +++ b/pkg/deployment/reconcile/plan_executor.go @@ -249,9 +249,11 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte now := metav1.Now() plan[0].StartTime = &now - plan[0].Locals = action.GetLocals() } + // action Start or action CheckProgress could add some new variables. + plan[0].Locals = action.GetLocals() + return plan, recall, nil } } From db465d0120e71a82775b408d01ab7f6fa1073843 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Sun, 8 May 2022 13:42:55 +0000 Subject: [PATCH 3/3] Standardize API & add DeepCopy funcs --- pkg/apis/deployment/v1/plan.go | 25 +-- pkg/apis/deployment/v1/plan_locals.go | 114 ++++++++++++++ pkg/apis/deployment/v1/plan_locals_test.go | 148 ++++++++++++++++++ .../deployment/v1/zz_generated.deepcopy.go | 29 ++++ pkg/apis/deployment/v2alpha1/plan.go | 8 +- pkg/apis/deployment/v2alpha1/plan_locals.go | 114 ++++++++++++++ .../deployment/v2alpha1/plan_locals_test.go | 148 ++++++++++++++++++ .../v2alpha1/zz_generated.deepcopy.go | 29 ++++ pkg/deployment/reconcile/action.go | 3 - pkg/deployment/reconcile/action_context.go | 22 +++ pkg/deployment/reconcile/action_helper.go | 5 - pkg/deployment/reconcile/plan_executor.go | 11 +- 12 files changed, 617 insertions(+), 39 deletions(-) create mode 100644 pkg/apis/deployment/v1/plan_locals.go create mode 100644 pkg/apis/deployment/v1/plan_locals_test.go create mode 100644 pkg/apis/deployment/v2alpha1/plan_locals.go create mode 100644 pkg/apis/deployment/v2alpha1/plan_locals_test.go diff --git a/pkg/apis/deployment/v1/plan.go b/pkg/apis/deployment/v1/plan.go index 9f7872245..76e4c8dff 100644 --- a/pkg/apis/deployment/v1/plan.go +++ b/pkg/apis/deployment/v1/plan.go @@ -227,7 +227,7 @@ type Action struct { // Params additional parameters used for action Params map[string]string `json:"params,omitempty"` // Locals additional storage for local variables which are produced during the action. - Locals map[string]string `json:"locals,omitempty"` + Locals PlanLocals `json:"locals,omitempty"` } // Equal compares two Actions @@ -241,28 +241,7 @@ func (a Action) Equal(other Action) bool { a.Reason == other.Reason && a.Image == other.Image && equality.Semantic.DeepEqual(a.Params, other.Params) && - equality.Semantic.DeepEqual(a.Locals, other.Locals) -} - -// AddLocal returns a copy of an action with set local variable. -// If a local variable already exits then it is overwritten. -func (a Action) AddLocal(key, value string) Action { - if a.Locals == nil { - a.Locals = map[string]string{} - } - - a.Locals[key] = value - - return a -} - -// GetLocal returns an action's local variable. -// If a variable does not exist then false is returned. -func (a Action) GetLocal(key string) (string, bool) { - // The Locals variable can be nil. - i, ok := a.Locals[key] - - return i, ok + a.Locals.Equal(other.Locals) } // AddParam returns copy of action with set parameter diff --git a/pkg/apis/deployment/v1/plan_locals.go b/pkg/apis/deployment/v1/plan_locals.go new file mode 100644 index 000000000..f913b225f --- /dev/null +++ b/pkg/apis/deployment/v1/plan_locals.go @@ -0,0 +1,114 @@ +// +// DISCLAIMER +// +// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + +package v1 + +type PlanLocalKey string + +func (p PlanLocalKey) String() string { + return string(p) +} + +type PlanLocals map[PlanLocalKey]string + +func (p *PlanLocals) Remove(key PlanLocalKey) bool { + if *p == nil { + return false + } + + z := *p + + if _, ok := z[key]; ok { + delete(z, key) + *p = z + return true + } + + return false +} + +func (p PlanLocals) Get(key PlanLocalKey) (string, bool) { + v, ok := p[key] + return v, ok +} + +func (p PlanLocals) GetWithParent(parent PlanLocals, key PlanLocalKey) (string, bool) { + v, ok := p[key] + if ok { + return v, true + } + return parent.Get(key) +} + +func (p *PlanLocals) Merge(merger PlanLocals) (changed bool) { + for k, v := range merger { + if p.Add(k, v, true) { + changed = true + } + } + + return +} + +func (p *PlanLocals) Add(key PlanLocalKey, value string, override bool) bool { + if value == "" { + return p.Remove(key) + } + + if *p == nil { + *p = PlanLocals{ + key: value, + } + + return true + } + + z := *p + + if v, ok := z[key]; ok { + if v == value { + return true + } + + if !override { + return false + } + } + + z[key] = value + + *p = z + + return true +} + +func (p PlanLocals) Equal(other PlanLocals) bool { + if len(p) != len(other) { + return false + } + + for k, v := range p { + if v2, ok := other[k]; !ok || v != v2 { + return false + } + } + + return true +} diff --git a/pkg/apis/deployment/v1/plan_locals_test.go b/pkg/apis/deployment/v1/plan_locals_test.go new file mode 100644 index 000000000..e815f21dc --- /dev/null +++ b/pkg/apis/deployment/v1/plan_locals_test.go @@ -0,0 +1,148 @@ +// +// DISCLAIMER +// +// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + +package v1 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_PlanLocals(t *testing.T) { + var l PlanLocals + + var key PlanLocalKey = "test" + v1, v2 := "v1", "v2" + + t.Run("Get on nil", func(t *testing.T) { + v, ok := l.Get(key) + + require.Equal(t, "", v) + require.False(t, ok) + }) + + t.Run("Remove on nil", func(t *testing.T) { + ok := l.Remove(key) + + require.False(t, ok) + }) + + t.Run("Add", func(t *testing.T) { + ok := l.Add(key, v1, false) + + require.True(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v1, v) + }) + + t.Run("Update", func(t *testing.T) { + ok := l.Add(key, v2, false) + + require.False(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v1, v) + }) + + t.Run("Update - override", func(t *testing.T) { + ok := l.Add(key, v2, true) + + require.True(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v2, v) + }) + + t.Run("Remove", func(t *testing.T) { + ok := l.Remove(key) + + require.True(t, ok) + }) + + t.Run("Remove missing", func(t *testing.T) { + ok := l.Remove(key) + + require.False(t, ok) + }) +} + +func Test_PlanLocals_Equal(t *testing.T) { + cmp := func(name string, a, b PlanLocals, expected bool) { + t.Run(name, func(t *testing.T) { + require.True(t, a.Equal(a)) + require.True(t, b.Equal(b)) + if expected { + require.True(t, a.Equal(b)) + require.True(t, b.Equal(a)) + } else { + require.False(t, a.Equal(b)) + require.False(t, b.Equal(a)) + } + }) + } + + cmp("Nil", nil, nil, true) + + cmp("Nil & empty", nil, PlanLocals{}, true) + + cmp("Empty", PlanLocals{}, PlanLocals{}, true) + + cmp("Same keys & values", PlanLocals{ + "key1": "v1", + }, PlanLocals{ + "key1": "v1", + }, true) + + cmp("Diff keys", PlanLocals{ + "key2": "v1", + }, PlanLocals{ + "key1": "v1", + }, false) + + cmp("Same keys & diff values", PlanLocals{ + "key1": "v1", + }, PlanLocals{ + "key1": "v2", + }, false) + + cmp("Same multi keys & values", PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, true) + + cmp("Same multi keys & values - reorder", PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, PlanLocals{ + "ket2": "v2", + "key1": "v1", + }, true) +} diff --git a/pkg/apis/deployment/v1/zz_generated.deepcopy.go b/pkg/apis/deployment/v1/zz_generated.deepcopy.go index 1f4ca8ef2..c20fce9dd 100644 --- a/pkg/apis/deployment/v1/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1/zz_generated.deepcopy.go @@ -49,6 +49,13 @@ func (in *Action) DeepCopyInto(out *Action) { (*out)[key] = val } } + if in.Locals != nil { + in, out := &in.Locals, &out.Locals + *out = make(PlanLocals, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -1852,6 +1859,28 @@ func (in Plan) DeepCopy() Plan { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in PlanLocals) DeepCopyInto(out *PlanLocals) { + { + in := &in + *out = make(PlanLocals, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlanLocals. +func (in PlanLocals) DeepCopy() PlanLocals { + if in == nil { + return nil + } + out := new(PlanLocals) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RocksDBEncryptionSpec) DeepCopyInto(out *RocksDBEncryptionSpec) { *out = *in diff --git a/pkg/apis/deployment/v2alpha1/plan.go b/pkg/apis/deployment/v2alpha1/plan.go index f6961eaf8..215513dd4 100644 --- a/pkg/apis/deployment/v2alpha1/plan.go +++ b/pkg/apis/deployment/v2alpha1/plan.go @@ -21,12 +21,13 @@ package v2alpha1 import ( - "github.com/arangodb/kube-arangodb/pkg/util" "github.com/dchest/uniuri" "k8s.io/apimachinery/pkg/api/equality" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" + + "github.com/arangodb/kube-arangodb/pkg/util" ) // ActionPriority define action priority @@ -225,6 +226,8 @@ type Action struct { Image string `json:"image,omitempty"` // Params additional parameters used for action Params map[string]string `json:"params,omitempty"` + // Locals additional storage for local variables which are produced during the action. + Locals PlanLocals `json:"locals,omitempty"` } // Equal compares two Actions @@ -237,7 +240,8 @@ func (a Action) Equal(other Action) bool { util.TimeCompareEqualPointer(a.StartTime, other.StartTime) && a.Reason == other.Reason && a.Image == other.Image && - equality.Semantic.DeepEqual(a.Params, other.Params) + equality.Semantic.DeepEqual(a.Params, other.Params) && + a.Locals.Equal(other.Locals) } // AddParam returns copy of action with set parameter diff --git a/pkg/apis/deployment/v2alpha1/plan_locals.go b/pkg/apis/deployment/v2alpha1/plan_locals.go new file mode 100644 index 000000000..18fb4c239 --- /dev/null +++ b/pkg/apis/deployment/v2alpha1/plan_locals.go @@ -0,0 +1,114 @@ +// +// DISCLAIMER +// +// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + +package v2alpha1 + +type PlanLocalKey string + +func (p PlanLocalKey) String() string { + return string(p) +} + +type PlanLocals map[PlanLocalKey]string + +func (p *PlanLocals) Remove(key PlanLocalKey) bool { + if *p == nil { + return false + } + + z := *p + + if _, ok := z[key]; ok { + delete(z, key) + *p = z + return true + } + + return false +} + +func (p PlanLocals) Get(key PlanLocalKey) (string, bool) { + v, ok := p[key] + return v, ok +} + +func (p PlanLocals) GetWithParent(parent PlanLocals, key PlanLocalKey) (string, bool) { + v, ok := p[key] + if ok { + return v, true + } + return parent.Get(key) +} + +func (p *PlanLocals) Merge(merger PlanLocals) (changed bool) { + for k, v := range merger { + if p.Add(k, v, true) { + changed = true + } + } + + return +} + +func (p *PlanLocals) Add(key PlanLocalKey, value string, override bool) bool { + if value == "" { + return p.Remove(key) + } + + if *p == nil { + *p = PlanLocals{ + key: value, + } + + return true + } + + z := *p + + if v, ok := z[key]; ok { + if v == value { + return true + } + + if !override { + return false + } + } + + z[key] = value + + *p = z + + return true +} + +func (p PlanLocals) Equal(other PlanLocals) bool { + if len(p) != len(other) { + return false + } + + for k, v := range p { + if v2, ok := other[k]; !ok || v != v2 { + return false + } + } + + return true +} diff --git a/pkg/apis/deployment/v2alpha1/plan_locals_test.go b/pkg/apis/deployment/v2alpha1/plan_locals_test.go new file mode 100644 index 000000000..ce0ef7452 --- /dev/null +++ b/pkg/apis/deployment/v2alpha1/plan_locals_test.go @@ -0,0 +1,148 @@ +// +// DISCLAIMER +// +// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany +// +// 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. +// +// Copyright holder is ArangoDB GmbH, Cologne, Germany +// + +package v2alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_PlanLocals(t *testing.T) { + var l PlanLocals + + var key PlanLocalKey = "test" + v1, v2 := "v1", "v2" + + t.Run("Get on nil", func(t *testing.T) { + v, ok := l.Get(key) + + require.Equal(t, "", v) + require.False(t, ok) + }) + + t.Run("Remove on nil", func(t *testing.T) { + ok := l.Remove(key) + + require.False(t, ok) + }) + + t.Run("Add", func(t *testing.T) { + ok := l.Add(key, v1, false) + + require.True(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v1, v) + }) + + t.Run("Update", func(t *testing.T) { + ok := l.Add(key, v2, false) + + require.False(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v1, v) + }) + + t.Run("Update - override", func(t *testing.T) { + ok := l.Add(key, v2, true) + + require.True(t, ok) + + v, ok := l.Get(key) + + require.True(t, ok) + require.Equal(t, v2, v) + }) + + t.Run("Remove", func(t *testing.T) { + ok := l.Remove(key) + + require.True(t, ok) + }) + + t.Run("Remove missing", func(t *testing.T) { + ok := l.Remove(key) + + require.False(t, ok) + }) +} + +func Test_PlanLocals_Equal(t *testing.T) { + cmp := func(name string, a, b PlanLocals, expected bool) { + t.Run(name, func(t *testing.T) { + require.True(t, a.Equal(a)) + require.True(t, b.Equal(b)) + if expected { + require.True(t, a.Equal(b)) + require.True(t, b.Equal(a)) + } else { + require.False(t, a.Equal(b)) + require.False(t, b.Equal(a)) + } + }) + } + + cmp("Nil", nil, nil, true) + + cmp("Nil & empty", nil, PlanLocals{}, true) + + cmp("Empty", PlanLocals{}, PlanLocals{}, true) + + cmp("Same keys & values", PlanLocals{ + "key1": "v1", + }, PlanLocals{ + "key1": "v1", + }, true) + + cmp("Diff keys", PlanLocals{ + "key2": "v1", + }, PlanLocals{ + "key1": "v1", + }, false) + + cmp("Same keys & diff values", PlanLocals{ + "key1": "v1", + }, PlanLocals{ + "key1": "v2", + }, false) + + cmp("Same multi keys & values", PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, true) + + cmp("Same multi keys & values - reorder", PlanLocals{ + "key1": "v1", + "ket2": "v2", + }, PlanLocals{ + "ket2": "v2", + "key1": "v1", + }, true) +} diff --git a/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go b/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go index 6e6469478..a0de48f38 100644 --- a/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v2alpha1/zz_generated.deepcopy.go @@ -49,6 +49,13 @@ func (in *Action) DeepCopyInto(out *Action) { (*out)[key] = val } } + if in.Locals != nil { + in, out := &in.Locals, &out.Locals + *out = make(PlanLocals, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -1852,6 +1859,28 @@ func (in Plan) DeepCopy() Plan { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in PlanLocals) DeepCopyInto(out *PlanLocals) { + { + in := &in + *out = make(PlanLocals, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlanLocals. +func (in PlanLocals) DeepCopy() PlanLocals { + if in == nil { + return nil + } + out := new(PlanLocals) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RocksDBEncryptionSpec) DeepCopyInto(out *RocksDBEncryptionSpec) { *out = *in diff --git a/pkg/deployment/reconcile/action.go b/pkg/deployment/reconcile/action.go index beadf0406..4eabfde42 100644 --- a/pkg/deployment/reconcile/action.go +++ b/pkg/deployment/reconcile/action.go @@ -59,9 +59,6 @@ type Action interface { // MemberID Return the MemberID used / created in this action MemberID() string - - // GetLocals returns locals variable which can be added during the action. - GetLocals() map[string]string } // ActionPost keep interface which is executed after action is completed. diff --git a/pkg/deployment/reconcile/action_context.go b/pkg/deployment/reconcile/action_context.go index fa5763c08..d1993738d 100644 --- a/pkg/deployment/reconcile/action_context.go +++ b/pkg/deployment/reconcile/action_context.go @@ -67,6 +67,8 @@ type ActionContext interface { member.StateInspectorGetter + ActionLocalsContext + // GetMemberStatusByID returns the current member status // for the member with given id. // Returns member status, true when found, or false @@ -129,6 +131,13 @@ type ActionContext interface { SelectImage(spec api.DeploymentSpec, status api.DeploymentStatus) (api.ImageInfo, bool) } +type ActionLocalsContext interface { + CurrentLocals() api.PlanLocals + + Get(action api.Action, key api.PlanLocalKey) (string, bool) + Add(key api.PlanLocalKey, value string, override bool) bool +} + // newActionContext creates a new ActionContext implementation. func newActionContext(log zerolog.Logger, context Context, cachedStatus inspectorInterface.Inspector) ActionContext { return &actionContext{ @@ -143,6 +152,19 @@ type actionContext struct { context Context log zerolog.Logger cachedStatus inspectorInterface.Inspector + locals api.PlanLocals +} + +func (ac *actionContext) CurrentLocals() api.PlanLocals { + return ac.locals +} + +func (ac *actionContext) Get(action api.Action, key api.PlanLocalKey) (string, bool) { + return ac.locals.GetWithParent(action.Locals, key) +} + +func (ac *actionContext) Add(key api.PlanLocalKey, value string, override bool) bool { + return ac.locals.Add(key, value, override) } func (ac *actionContext) WithArangoMember(cache inspectorInterface.Inspector, timeout time.Duration, name string) reconciler.ArangoMemberModContext { diff --git a/pkg/deployment/reconcile/action_helper.go b/pkg/deployment/reconcile/action_helper.go index 4db24b4a9..3b96fe135 100644 --- a/pkg/deployment/reconcile/action_helper.go +++ b/pkg/deployment/reconcile/action_helper.go @@ -91,8 +91,3 @@ type actionImpl struct { func (a actionImpl) MemberID() string { return *a.memberIDRef } - -// GetLocals returns locals variable which can be added during the action. -func (a actionImpl) GetLocals() map[string]string { - return a.action.Locals -} diff --git a/pkg/deployment/reconcile/plan_executor.go b/pkg/deployment/reconcile/plan_executor.go index 39b738025..4a62b7479 100644 --- a/pkg/deployment/reconcile/plan_executor.go +++ b/pkg/deployment/reconcile/plan_executor.go @@ -172,12 +172,12 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte } for k, v := range planAction.Locals { - logContext = logContext.Str("local."+k, v) + logContext = logContext.Str("local."+k.String(), v) } log := logContext.Logger() - action := d.createAction(log, planAction, cachedStatus) + action, actionContext := d.createAction(log, planAction, cachedStatus) done, abort, recall, retry, err := d.executeAction(ctx, log, planAction, action) if err != nil { @@ -251,8 +251,7 @@ func (d *Reconciler) executePlan(ctx context.Context, cachedStatus inspectorInte plan[0].StartTime = &now } - // action Start or action CheckProgress could add some new variables. - plan[0].Locals = action.GetLocals() + plan[0].Locals.Merge(actionContext.CurrentLocals()) return plan, recall, nil } @@ -312,7 +311,7 @@ func (d *Reconciler) executeAction(ctx context.Context, log zerolog.Logger, plan } // createAction create action object based on action type -func (d *Reconciler) createAction(log zerolog.Logger, action api.Action, cachedStatus inspectorInterface.Inspector) Action { +func (d *Reconciler) createAction(log zerolog.Logger, action api.Action, cachedStatus inspectorInterface.Inspector) (Action, ActionContext) { actionCtx := newActionContext(log.With().Str("id", action.ID).Str("type", action.Type.String()).Logger(), d.context, cachedStatus) f, ok := getActionFactory(action.Type) @@ -320,5 +319,5 @@ func (d *Reconciler) createAction(log zerolog.Logger, action api.Action, cachedS panic(fmt.Sprintf("Unknown action type '%s'", action.Type)) } - return f(log, action, actionCtx) + return f(log, action, actionCtx), actionCtx }