From 2b8ea3eb46f4c003849ef42e7a19101e008ff555 Mon Sep 17 00:00:00 2001 From: liujian Date: Wed, 23 Jul 2025 11:25:08 +0800 Subject: [PATCH] feat: test variable store in memory (#2666) Signed-off-by: joyceliu --- pkg/const/test.go | 65 ++++++++++++++++++++++++ pkg/executor/executor_test.go | 45 +++++------------ pkg/executor/task_executor_test.go | 4 +- pkg/modules/add_hostvars_test.go | 80 ++++++++++++++++++++++++------ pkg/modules/assert_test.go | 50 ++++++++----------- pkg/modules/command_test.go | 12 +---- pkg/modules/copy_test.go | 10 ++-- pkg/modules/debug_test.go | 40 +++++++-------- pkg/modules/fetch_test.go | 4 +- pkg/modules/gen_cert_test.go | 10 ++-- pkg/modules/image_test.go | 6 +-- pkg/modules/module_test.go | 45 +++++++++-------- pkg/modules/prometheus_test.go | 6 +-- pkg/modules/result_test.go | 10 ++-- pkg/modules/set_fact_test.go | 10 ++-- pkg/modules/setup_test.go | 4 +- pkg/modules/template_test.go | 14 +++--- pkg/variable/variable_merge.go | 27 +++++----- 18 files changed, 258 insertions(+), 184 deletions(-) create mode 100644 pkg/const/test.go diff --git a/pkg/const/test.go b/pkg/const/test.go new file mode 100644 index 00000000..bccdd4ab --- /dev/null +++ b/pkg/const/test.go @@ -0,0 +1,65 @@ +package _const + +import ( + "context" + + kkcorev1 "github.com/kubesphere/kubekey/api/core/v1" + kkcorev1alpha1 "github.com/kubesphere/kubekey/api/core/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// NewTestPlaybook creates a fake controller-runtime client, an Inventory resource with the given hosts, +// and returns the client and a Playbook resource referencing the created Inventory. +// This is intended for use in unit tests. +func NewTestPlaybook(hosts []string) (ctrlclient.Client, *kkcorev1.Playbook, error) { + // Create a fake client with the required scheme and status subresources. + client := fake.NewClientBuilder(). + WithScheme(Scheme). + WithStatusSubresource(&kkcorev1.Playbook{}, &kkcorev1alpha1.Task{}). + Build() + + // Convert the slice of hostnames to an InventoryHost map. + inventoryHost := make(kkcorev1.InventoryHost) + for _, h := range hosts { + inventoryHost[h] = runtime.RawExtension{} + } + + // Create an Inventory resource with the generated hosts. + inventory := &kkcorev1.Inventory{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: corev1.NamespaceDefault, + }, + Spec: kkcorev1.InventorySpec{ + Hosts: inventoryHost, + }, + } + + // Persist the Inventory resource using the fake client. + if err := client.Create(context.TODO(), inventory); err != nil { + return nil, nil, err + } + + // Create a Playbook resource that references the created Inventory. + playbook := &kkcorev1.Playbook{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-", + Namespace: corev1.NamespaceDefault, + }, + Spec: kkcorev1.PlaybookSpec{ + InventoryRef: &corev1.ObjectReference{ + Name: inventory.Name, + Namespace: inventory.Namespace, + }, + }, + Status: kkcorev1.PlaybookStatus{}, + } + + return client, playbook, nil +} diff --git a/pkg/executor/executor_test.go b/pkg/executor/executor_test.go index 865f0f5e..238c3be7 100644 --- a/pkg/executor/executor_test.go +++ b/pkg/executor/executor_test.go @@ -5,58 +5,39 @@ import ( "os" kkcorev1 "github.com/kubesphere/kubekey/api/core/v1" - kkcorev1alpha1 "github.com/kubesphere/kubekey/api/core/v1alpha1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" _const "github.com/kubesphere/kubekey/v4/pkg/const" "github.com/kubesphere/kubekey/v4/pkg/variable" "github.com/kubesphere/kubekey/v4/pkg/variable/source" ) +// newTestOption creates a new test option struct for the given hosts. +// It sets up a fake client and playbook using the provided hosts, and initializes the variable subsystem +// with a memory-backed source. This is intended for use in unit tests. func newTestOption(hosts []string) (*option, error) { var err error - // convert host to InventoryHost + + // Convert the slice of hostnames to an InventoryHost map (not strictly needed here, but kept for clarity). inventoryHost := make(kkcorev1.InventoryHost) for _, h := range hosts { inventoryHost[h] = runtime.RawExtension{} } - client := fake.NewClientBuilder().WithScheme(_const.Scheme).WithStatusSubresource(&kkcorev1.Playbook{}, &kkcorev1alpha1.Task{}).Build() - inventory := &kkcorev1.Inventory{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-", - Namespace: corev1.NamespaceDefault, - }, - Spec: kkcorev1.InventorySpec{ - Hosts: inventoryHost, - }, - } - if err := client.Create(context.TODO(), inventory); err != nil { + + // Create a fake client and playbook for testing. + client, playbook, err := _const.NewTestPlaybook(hosts) + if err != nil { return nil, err } + // Initialize the option struct with the fake client, playbook, and standard output for logging. o := &option{ - client: client, - playbook: &kkcorev1.Playbook{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-", - Namespace: corev1.NamespaceDefault, - }, - Spec: kkcorev1.PlaybookSpec{ - InventoryRef: &corev1.ObjectReference{ - Name: inventory.Name, - Namespace: inventory.Namespace, - }, - }, - Status: kkcorev1.PlaybookStatus{}, - }, + client: client, + playbook: playbook, logOutput: os.Stdout, } + // Initialize the variable subsystem using the memory-backed source. o.variable, err = variable.New(context.TODO(), o.client, *o.playbook, source.MemorySource) if err != nil { return nil, err diff --git a/pkg/executor/task_executor_test.go b/pkg/executor/task_executor_test.go index c70e2645..b9f72a00 100644 --- a/pkg/executor/task_executor_test.go +++ b/pkg/executor/task_executor_test.go @@ -79,12 +79,12 @@ func TestTaskExecutor(t *testing.T) { } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() o, err := newTestOption(tc.hosts) if err != nil { t.Fatal(err) } - ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) - defer cancel() if err := (&taskExecutor{ option: o, diff --git a/pkg/modules/add_hostvars_test.go b/pkg/modules/add_hostvars_test.go index 441e3da0..27c98ffd 100644 --- a/pkg/modules/add_hostvars_test.go +++ b/pkg/modules/add_hostvars_test.go @@ -5,8 +5,6 @@ import ( "testing" "time" - kkcorev1 "github.com/kubesphere/kubekey/api/core/v1" - kkcorev1alpha1 "github.com/kubesphere/kubekey/api/core/v1alpha1" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime" ) @@ -14,53 +12,107 @@ import ( func TestModuleAddHostvars(t *testing.T) { type testcase struct { name string - args []byte + opt ExecOptions expectStdout string expectStderr string } cases := []testcase{ { name: "missing hosts", - args: []byte(` + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), + Args: runtime.RawExtension{ + Raw: []byte(` vars: foo: bar `), + }, + }, expectStdout: "", expectStderr: "\"hosts\" should be string or string array", }, { name: "missing vars", - args: []byte(` + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), + Args: runtime.RawExtension{ + Raw: []byte(` hosts: node1 `), + }, + }, expectStdout: "", expectStderr: "\"vars\" should not be empty", }, { name: "invalid hosts type", - args: []byte(` + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), + Args: runtime.RawExtension{ + Raw: []byte(` hosts: foo: bar vars: a: b `), + }, + }, expectStdout: "", expectStderr: "\"hosts\" should be string or string array", }, + { + name: "string value", + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), + Args: runtime.RawExtension{ + Raw: []byte(` +hosts: node1 +vars: + a: b +`), + }, + }, + }, + { + name: "string var value", + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{"a": "b"}), + Args: runtime.RawExtension{ + Raw: []byte(` +hosts: node1 +vars: + a: "{{ .a }}" +`), + }, + }, + }, + { + name: "map value", + opt: ExecOptions{ + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), + Args: runtime.RawExtension{ + Raw: []byte(` +hosts: node1 +vars: + a: + b: c +`), + }, + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - opt := ExecOptions{ - Args: runtime.RawExtension{Raw: tc.args}, - Host: "", - Variable: &testVariable{}, - Task: kkcorev1alpha1.Task{}, - Playbook: kkcorev1.Playbook{}, - } - stdout, stderr := ModuleAddHostvars(ctx, opt) + stdout, stderr := ModuleAddHostvars(ctx, tc.opt) require.Equal(t, tc.expectStdout, stdout, "stdout mismatch") require.Equal(t, tc.expectStderr, stderr, "stderr mismatch") }) diff --git a/pkg/modules/assert_test.go b/pkg/modules/assert_test.go index 8dbe79cd..79296107 100644 --- a/pkg/modules/assert_test.go +++ b/pkg/modules/assert_test.go @@ -35,8 +35,8 @@ func TestAssert(t *testing.T) { { name: "non-that", opt: ExecOptions{ - Host: "local", - Variable: &testVariable{}, + Host: "node1", + Variable: newTestVariable(nil, nil), Args: runtime.RawExtension{}, }, exceptStderr: "\"that\" should be []string or string", @@ -44,63 +44,55 @@ func TestAssert(t *testing.T) { { name: "success with non-msg", opt: ExecOptions{ - Host: "local", + Host: "node1", Args: runtime.RawExtension{ Raw: []byte(`{"that": ["true", "{{ eq .testvalue \"a\" }}"]}`), }, - Variable: &testVariable{ - value: map[string]any{ - "testvalue": "a", - }, - }, + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "testvalue": "a", + }), }, exceptStdout: StdoutTrue, }, { name: "success with success_msg", opt: ExecOptions{ - Host: "local", + Host: "node1", Args: runtime.RawExtension{ Raw: []byte(`{"that": ["true", "{{ eq .k1 \"v1\" }}"], "success_msg": "success {{ .k2 }}"}`), }, - Variable: &testVariable{ - value: map[string]any{ - "k1": "v1", - "k2": "v2", - }, - }, + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k1": "v1", + "k2": "v2", + }), }, exceptStdout: "success v2", }, { name: "failed with non-msg", opt: ExecOptions{ - Host: "local", + Host: "node1", Args: runtime.RawExtension{ Raw: []byte(`{"that": ["true", "{{ eq .k1 \"v2\" }}"]}`), }, - Variable: &testVariable{ - value: map[string]any{ - "k1": "v1", - "k2": "v2", - }, - }, + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k1": "v1", + "k2": "v2", + }), }, exceptStderr: "False", }, { name: "failed with failed_msg", opt: ExecOptions{ - Host: "local", + Host: "node1", Args: runtime.RawExtension{ Raw: []byte(`{"that": ["true", "{{ eq .k1 \"v2\" }}"], "fail_msg": "failed {{ .k2 }}"}`), }, - Variable: &testVariable{ - value: map[string]any{ - "k1": "v1", - "k2": "v2", - }, - }, + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k1": "v1", + "k2": "v2", + }), }, exceptStderr: "failed v2", }, diff --git a/pkg/modules/command_test.go b/pkg/modules/command_test.go index 65280bf3..a9ab99a9 100644 --- a/pkg/modules/command_test.go +++ b/pkg/modules/command_test.go @@ -33,14 +33,6 @@ func TestCommand(t *testing.T) { exceptStdout string exceptStderr string }{ - { - name: "non-host variable", - opt: ExecOptions{ - Variable: &testVariable{}, - }, - ctxFunc: context.Background, - exceptStderr: "failed to connector for \"\" error: workdir in variable should be string", - }, { name: "exec command success", ctxFunc: func() context.Context { @@ -49,7 +41,7 @@ func TestCommand(t *testing.T) { opt: ExecOptions{ Host: "test", Args: runtime.RawExtension{Raw: []byte("echo success")}, - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, exceptStdout: "success", }, @@ -59,7 +51,7 @@ func TestCommand(t *testing.T) { opt: ExecOptions{ Host: "test", Args: runtime.RawExtension{Raw: []byte("echo success")}, - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, exceptStderr: "failed", }, diff --git a/pkg/modules/copy_test.go b/pkg/modules/copy_test.go index 3cde7e82..9e48c85b 100644 --- a/pkg/modules/copy_test.go +++ b/pkg/modules/copy_test.go @@ -40,7 +40,7 @@ func TestCopy(t *testing.T) { Raw: []byte(`{"dest": "hello world"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -54,7 +54,7 @@ func TestCopy(t *testing.T) { Raw: []byte(`{"content": "hello world"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -68,7 +68,7 @@ func TestCopy(t *testing.T) { Raw: []byte(`{"content": "hello world", "dest": "/etc/"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -82,7 +82,7 @@ func TestCopy(t *testing.T) { Raw: []byte(`{"content": "hello world", "dest": "/etc/test.txt"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -96,7 +96,7 @@ func TestCopy(t *testing.T) { Raw: []byte(`{"content": "hello world", "dest": "/etc/test.txt"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, failedConnector) diff --git a/pkg/modules/debug_test.go b/pkg/modules/debug_test.go index 2cae3996..63e418f5 100644 --- a/pkg/modules/debug_test.go +++ b/pkg/modules/debug_test.go @@ -36,8 +36,8 @@ func TestDebug(t *testing.T) { name: "non-var and non-msg", opt: ExecOptions{ Args: runtime.RawExtension{}, - Host: "local", - Variable: &testVariable{}, + Host: "node1", + Variable: newTestVariable(nil, nil), }, exceptStderr: "\"msg\" is not found", }, @@ -47,12 +47,10 @@ func TestDebug(t *testing.T) { Args: runtime.RawExtension{ Raw: []byte(`{"msg": "{{ .k }}"}`), }, - Host: "local", - Variable: &testVariable{ - value: map[string]any{ - "k": "v", - }, - }, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k": "v", + }), }, exceptStdout: "\"v\"", }, @@ -62,12 +60,10 @@ func TestDebug(t *testing.T) { Args: runtime.RawExtension{ Raw: []byte(`{"msg": "{{ .k }}"}`), }, - Host: "local", - Variable: &testVariable{ - value: map[string]any{ - "k": 1, - }, - }, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k": 1, + }), }, exceptStdout: "1", }, @@ -77,16 +73,14 @@ func TestDebug(t *testing.T) { Args: runtime.RawExtension{ Raw: []byte(`{"msg": "{{ .k }}"}`), }, - Host: "local", - Variable: &testVariable{ - value: map[string]any{ - "k": map[string]any{ - "a1": 1, - "a2": 1.1, - "a3": "b3", - }, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "k": map[string]any{ + "a1": 1, + "a2": 1.1, + "a3": "b3", }, - }, + }), }, exceptStdout: "{\n \"a1\": 1,\n \"a2\": 1.1,\n \"a3\": \"b3\"\n}", }, diff --git a/pkg/modules/fetch_test.go b/pkg/modules/fetch_test.go index 7bd56f0a..09bedde4 100644 --- a/pkg/modules/fetch_test.go +++ b/pkg/modules/fetch_test.go @@ -38,7 +38,7 @@ func TestFetch(t *testing.T) { opt: ExecOptions{ Args: runtime.RawExtension{}, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -52,7 +52,7 @@ func TestFetch(t *testing.T) { Raw: []byte(`{"src": "/etc/test.txt"}`), }, Host: "local", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) diff --git a/pkg/modules/gen_cert_test.go b/pkg/modules/gen_cert_test.go index eb27859d..4322169c 100644 --- a/pkg/modules/gen_cert_test.go +++ b/pkg/modules/gen_cert_test.go @@ -44,12 +44,10 @@ func TestModuleGenCert(t *testing.T) { "out_cert": "./test_gen_cert/test-crt.pem" }`), }, - Host: "local", - Variable: &testVariable{ - value: map[string]any{ - "policy": "IfNotPresent", - }, - }, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "policy": "IfNotPresent", + }), }, exceptStdout: "success", }, diff --git a/pkg/modules/image_test.go b/pkg/modules/image_test.go index 62f4d0b9..4886b510 100644 --- a/pkg/modules/image_test.go +++ b/pkg/modules/image_test.go @@ -23,7 +23,7 @@ func TestModuleImage(t *testing.T) { "pull": "" }`), }, - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, exceptStderr: "\"pull\" should be map", }, @@ -35,7 +35,7 @@ func TestModuleImage(t *testing.T) { "pull": {} }`), }, - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, exceptStderr: "\"pull.manifests\" is required", }, @@ -47,7 +47,7 @@ func TestModuleImage(t *testing.T) { "push": "" }`), }, - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, exceptStderr: "\"push\" should be map", }, diff --git a/pkg/modules/module_test.go b/pkg/modules/module_test.go index 95936153..04dd7ab0 100644 --- a/pkg/modules/module_test.go +++ b/pkg/modules/module_test.go @@ -22,32 +22,14 @@ import ( "io/fs" "github.com/cockroachdb/errors" + "k8s.io/klog/v2" "github.com/kubesphere/kubekey/v4/pkg/connector" + _const "github.com/kubesphere/kubekey/v4/pkg/const" "github.com/kubesphere/kubekey/v4/pkg/variable" + "github.com/kubesphere/kubekey/v4/pkg/variable/source" ) -type testVariable struct { - value map[string]any - err error -} - -func (v testVariable) Key() string { - return "testModule" -} - -func (v testVariable) Get(variable.GetFunc) (any, error) { - return v.value, v.err -} - -func (v *testVariable) Merge(variable.MergeFunc) error { - v.value = map[string]any{ - "k": "v", - } - - return nil -} - var successConnector = &testConnector{output: []byte("success")} var failedConnector = &testConnector{ copyErr: errors.New("failed"), @@ -90,3 +72,24 @@ func (t testConnector) FetchFile(context.Context, string, io.Writer) error { func (t testConnector) ExecuteCommand(context.Context, string) ([]byte, error) { return t.output, t.commandErr } + +// newTestVariable creates a new variable.Variable for testing purposes. +// It initializes a test playbook and client, creates a new in-memory variable source, +// and merges the provided vars as remote variables for the specified hosts. +func newTestVariable(hosts []string, vars map[string]any) variable.Variable { + client, playbook, err := _const.NewTestPlaybook(hosts) + if err != nil { + klog.Error(err) + } + // Create a new variable in memory using the test client and playbook + v, err := variable.New(context.TODO(), client, *playbook, source.MemorySource) + if err != nil { + klog.Error(err) + } + // Set default values by merging the provided vars as remote variables for the hosts + if err := v.Merge(variable.MergeRemoteVariable(vars, hosts...)); err != nil { + klog.Error(err) + } + + return v +} diff --git a/pkg/modules/prometheus_test.go b/pkg/modules/prometheus_test.go index b17471c1..22c0a717 100644 --- a/pkg/modules/prometheus_test.go +++ b/pkg/modules/prometheus_test.go @@ -51,7 +51,7 @@ func TestPrometheus(t *testing.T) { "query": "up", }), Host: "", // Empty host - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: context.Background, // Add context background exceptStderr: "host name is empty, please specify a prometheus host", @@ -63,7 +63,7 @@ func TestPrometheus(t *testing.T) { "query": "up", // Add required query parameter }), Host: "test", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -77,7 +77,7 @@ func TestPrometheus(t *testing.T) { "query": "up", // Add required query parameter }), Host: "test", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, failedConnector) diff --git a/pkg/modules/result_test.go b/pkg/modules/result_test.go index cd70bd51..26d7236f 100644 --- a/pkg/modules/result_test.go +++ b/pkg/modules/result_test.go @@ -42,7 +42,7 @@ func TestResult(t *testing.T) { Raw: []byte(`{"k": "v"}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -55,7 +55,7 @@ func TestResult(t *testing.T) { Raw: []byte(`{"k": 1}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -68,7 +68,7 @@ func TestResult(t *testing.T) { Raw: []byte(`{"k": 1.1}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -81,7 +81,7 @@ func TestResult(t *testing.T) { Raw: []byte(`{"k": {"k1": "v1", "k2": "v2"}}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -94,7 +94,7 @@ func TestResult(t *testing.T) { Raw: []byte(`{"k": ["v1", "v2"]}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, diff --git a/pkg/modules/set_fact_test.go b/pkg/modules/set_fact_test.go index ff691742..462be82c 100644 --- a/pkg/modules/set_fact_test.go +++ b/pkg/modules/set_fact_test.go @@ -42,7 +42,7 @@ func TestSetFact(t *testing.T) { Raw: []byte(`{"k": "v"}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -55,7 +55,7 @@ func TestSetFact(t *testing.T) { Raw: []byte(`{"k": 1}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -68,7 +68,7 @@ func TestSetFact(t *testing.T) { Raw: []byte(`{"k": 1.1}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -81,7 +81,7 @@ func TestSetFact(t *testing.T) { Raw: []byte(`{"k": {"k1": "v1", "k2": "v2"}}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, @@ -94,7 +94,7 @@ func TestSetFact(t *testing.T) { Raw: []byte(`{"k": ["v1", "v2"]}`), }, Host: "", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), Task: kkcorev1alpha1.Task{}, Playbook: kkcorev1.Playbook{}, }, diff --git a/pkg/modules/setup_test.go b/pkg/modules/setup_test.go index 24cd0ff9..eb9459db 100644 --- a/pkg/modules/setup_test.go +++ b/pkg/modules/setup_test.go @@ -20,7 +20,7 @@ func TestModuleSetup(t *testing.T) { name: "successful setup with fact gathering", opt: ExecOptions{ Host: "test-host", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: func() context.Context { return context.WithValue(context.Background(), ConnKey, successConnector) @@ -32,7 +32,7 @@ func TestModuleSetup(t *testing.T) { name: "failed connector setup", opt: ExecOptions{ Host: "invalid-host", - Variable: &testVariable{}, + Variable: newTestVariable(nil, nil), }, ctxFunc: context.Background, exceptStdout: "", diff --git a/pkg/modules/template_test.go b/pkg/modules/template_test.go index 95f5c727..d1ed960f 100644 --- a/pkg/modules/template_test.go +++ b/pkg/modules/template_test.go @@ -47,8 +47,8 @@ func TestTemplate(t *testing.T) { name: "src is empty", opt: ExecOptions{ Args: runtime.RawExtension{}, - Host: "local", - Variable: &testVariable{}, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, nil), }, ctxFunc: context.Background, exceptStderr: "\"src\" should be string", @@ -61,12 +61,10 @@ func TestTemplate(t *testing.T) { "src": "{{ .absPath }}" }`), }, - Host: "local", - Variable: &testVariable{ - value: map[string]any{ - "absPath": absPath, - }, - }, + Host: "node1", + Variable: newTestVariable([]string{"node1"}, map[string]any{ + "absPath": absPath, + }), }, ctxFunc: context.Background, exceptStderr: "\"dest\" should be string", diff --git a/pkg/variable/variable_merge.go b/pkg/variable/variable_merge.go index 85c486d0..85aa0f62 100644 --- a/pkg/variable/variable_merge.go +++ b/pkg/variable/variable_merge.go @@ -7,27 +7,26 @@ import ( // ***************************** MergeFunc ***************************** // -// MergeRemoteVariable merges remote variables to a specific host. -// It takes a map of data and a hostname, and merges the data into the host's RemoteVars -// if the RemoteVars are currently empty. This prevents overwriting existing remote variables. -var MergeRemoteVariable = func(data map[string]any, hostname string) MergeFunc { +// MergeRemoteVariable merges the provided data as remote variables into the specified hosts. +// For each hostname, if the host exists and its RemoteVars is empty, it sets RemoteVars to the provided data. +// If the host does not exist, it returns an error. +var MergeRemoteVariable = func(data map[string]any, hostnames ...string) MergeFunc { return func(v Variable) error { vv, ok := v.(*variable) if !ok { return errors.New("variable type error") } - if hostname == "" { - return errors.New("when merge source is remote. HostName cannot be empty") - } - if _, ok := vv.value.Hosts[hostname]; !ok { - return errors.Errorf("when merge source is remote. HostName %s not exist", hostname) - } + for _, hostname := range hostnames { + if _, ok := vv.value.Hosts[hostname]; !ok { + return errors.Errorf("when merge source is remote. HostName %s not exist", hostname) + } - // it should not be changed - if hv := vv.value.Hosts[hostname]; len(hv.RemoteVars) == 0 { - hv.RemoteVars = data - vv.value.Hosts[hostname] = hv + // Only set RemoteVars if it is currently empty to avoid overwriting existing remote variables. + if hv := vv.value.Hosts[hostname]; len(hv.RemoteVars) == 0 { + hv.RemoteVars = data + vv.value.Hosts[hostname] = hv + } } return nil