From e0a1a4cc01f6b8fc3bf6f4e50a09699095415e40 Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 29 Jan 2024 14:51:22 +0800 Subject: [PATCH 1/4] feat: add 'WithLogWriter' for api 'RunWithOpts' Signed-off-by: zongz --- pkg/api/kpm_run.go | 1 + pkg/api/kpm_run_test.go | 29 ++++++++++++ .../test_run_with_no_log/kcl.mod | 7 +++ .../test_run_with_no_log/main.k | 45 +++++++++++++++++++ pkg/opt/opt.go | 14 ++++-- 5 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/kcl.mod create mode 100644 pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/main.k diff --git a/pkg/api/kpm_run.go b/pkg/api/kpm_run.go index 8e954462..c2f63a95 100644 --- a/pkg/api/kpm_run.go +++ b/pkg/api/kpm_run.go @@ -86,6 +86,7 @@ func RunWithOpt(opts *opt.CompileOptions) (*kcl.KCLResultList, error) { } // RunWithOpts will compile the kcl package with the compile options. +// Note: When combining multiple options, the 'WithLogWriter' should be set at the end. func RunWithOpts(opts ...opt.CompileOptions) (*kcl.KCLResultList, error) { mergedOpts := opt.MergeOptions(opts...) return runPkgWithOpt(&mergedOpts) diff --git a/pkg/api/kpm_run_test.go b/pkg/api/kpm_run_test.go index abf95598..58f130b4 100644 --- a/pkg/api/kpm_run_test.go +++ b/pkg/api/kpm_run_test.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "fmt" "os" "path/filepath" @@ -214,3 +215,31 @@ func TestRunWithOptsAndNoSumCheck(t *testing.T) { assert.Equal(t, err, nil) } } + +func TestRunWithOptsWithNoLog(t *testing.T) { + pkgPath := filepath.Join(getTestDir("test_run_pkg_in_path"), "test_run_with_no_log") + + defer func() { + _ = os.Remove(filepath.Join(pkgPath, "kcl.mod.lock")) + }() + + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + pathMainK := filepath.Join(pkgPath, "main.k") + + _, err := RunWithOpts( + opt.WithEntries([]string{pathMainK}), + opt.WithKclOption(kcl.WithWorkDir(pkgPath)), + opt.WithLogWriter(nil), + ) + + assert.Equal(t, err, nil) + os.Stdout = old + w.Close() + var buf bytes.Buffer + buf.ReadFrom(r) + + assert.Equal(t, buf.String(), "") +} diff --git a/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/kcl.mod b/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/kcl.mod new file mode 100644 index 00000000..8f27a88e --- /dev/null +++ b/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/kcl.mod @@ -0,0 +1,7 @@ +[package] +name = "dep_git_commit" +edition = "0.0.1" +version = "0.0.1" + +[dependencies] +catalog = { git = "https://github.com/KusionStack/catalog.git", commit = "a29e3db" } diff --git a/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/main.k b/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/main.k new file mode 100644 index 00000000..320a5761 --- /dev/null +++ b/pkg/api/test_data/test_run_pkg_in_path/test_run_with_no_log/main.k @@ -0,0 +1,45 @@ +import catalog.models.schema.v1 as ac +import catalog.models.schema.v1.workload as wl +import catalog.models.schema.v1.workload.container as c +import catalog.models.schema.v1.workload.container.probe as p +import catalog.models.schema.v1.monitoring as m + +ac.AppConfiguration { + workload: wl.Service { + containers: { + "nginx": c.Container { + image: "nginx:v1" + # Run the following command as defined + command: ["/bin/sh", "-c", "echo hi"] + # Extra arguments append to command defined above + args: ["/bin/sh", "-c", "echo hi"] + env: { + # An environment variable of name "env1" and value "VALUE" will be set + "env1": "VALUE" + # An environment variable of name "env2" and value of the key "key" in the + # secret named "sec-name" will be set. + "env2": "secret://sec-name/key" + } + # Run the command "/bin/sh -c echo hi", as defined above, in the directory "/tmp" + workingDir: "/tmp" + # Configure a HTTP readiness probe + readinessProbe: p.Probe { + probeHandler: p.Http { + url: "http://localhost:80" + } + initialDelaySeconds: 10 + } + } + } + # Set the replicas + replicas: 2 + } + # Add the monitoring configuration backed by Prometheus + monitoring: m.Prometheus{ + interval: "30s" + timeout: "15s" + path: "/metrics" + port: "web" + scheme: "http" + } +} \ No newline at end of file diff --git a/pkg/opt/opt.go b/pkg/opt/opt.go index 1780e976..65223839 100644 --- a/pkg/opt/opt.go +++ b/pkg/opt/opt.go @@ -26,13 +26,12 @@ type CompileOptions struct { } // MergeOptions will merge the input options. +// When combining multiple options, the 'WithLogWriter' should be set at the end. func MergeOptions(opts ...CompileOptions) CompileOptions { var opt = DefaultCompileOptions() for _, o := range opts { opt.Merge(*o.Option) - if o.writer != nil { - opt.writer = o.writer - } + opt.writer = o.writer if o.isVendor { opt.isVendor = o.isVendor @@ -79,6 +78,15 @@ func WithNoSumCheck(is bool) CompileOptions { return *opt } +// WithLogWriter will set the log writer of the compiler. +// When multiple log writers are set, the compiler will write to the last log writer. +// When combining multiple options, the log should be set at the end. +func WithLogWriter(writer io.Writer) CompileOptions { + var opt = DefaultCompileOptions() + opt.writer = writer + return *opt +} + // DefaultCompileOptions returns a default CompileOptions. func DefaultCompileOptions() *CompileOptions { return &CompileOptions{ From 7ccfee9f68de7d7663a339edcda7c54a16095fbd Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 29 Jan 2024 15:03:39 +0800 Subject: [PATCH 2/4] fix: make go lint happy Signed-off-by: zongz --- pkg/api/kpm_run_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/api/kpm_run_test.go b/pkg/api/kpm_run_test.go index 58f130b4..5b0ee572 100644 --- a/pkg/api/kpm_run_test.go +++ b/pkg/api/kpm_run_test.go @@ -239,7 +239,8 @@ func TestRunWithOptsWithNoLog(t *testing.T) { os.Stdout = old w.Close() var buf bytes.Buffer - buf.ReadFrom(r) + _, err = buf.ReadFrom(r) + assert.Equal(t, err, nil) assert.Equal(t, buf.String(), "") } From a68a89ed1984f1847a6e6522ac65e73bd2f1e7fa Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 29 Jan 2024 15:44:59 +0800 Subject: [PATCH 3/4] fix: fix CR comments Signed-off-by: zongz --- pkg/api/kpm_run.go | 10 ++++--- pkg/opt/opt.go | 67 ++++++++++++++------------------------------- pkg/opt/opt_test.go | 36 ------------------------ 3 files changed, 27 insertions(+), 86 deletions(-) diff --git a/pkg/api/kpm_run.go b/pkg/api/kpm_run.go index c2f63a95..3450f2cc 100644 --- a/pkg/api/kpm_run.go +++ b/pkg/api/kpm_run.go @@ -86,10 +86,12 @@ func RunWithOpt(opts *opt.CompileOptions) (*kcl.KCLResultList, error) { } // RunWithOpts will compile the kcl package with the compile options. -// Note: When combining multiple options, the 'WithLogWriter' should be set at the end. -func RunWithOpts(opts ...opt.CompileOptions) (*kcl.KCLResultList, error) { - mergedOpts := opt.MergeOptions(opts...) - return runPkgWithOpt(&mergedOpts) +func RunWithOpts(opts ...opt.Option) (*kcl.KCLResultList, error) { + mergedOpts := opt.DefaultCompileOptions() + for _, opt := range opts { + opt(mergedOpts) + } + return runPkgWithOpt(mergedOpts) } // getAbsInputPath will return the abs path of the file path described by '--input'. diff --git a/pkg/opt/opt.go b/pkg/opt/opt.go index 65223839..e6c48032 100644 --- a/pkg/opt/opt.go +++ b/pkg/opt/opt.go @@ -25,66 +25,41 @@ type CompileOptions struct { *kcl.Option } -// MergeOptions will merge the input options. -// When combining multiple options, the 'WithLogWriter' should be set at the end. -func MergeOptions(opts ...CompileOptions) CompileOptions { - var opt = DefaultCompileOptions() - for _, o := range opts { - opt.Merge(*o.Option) - opt.writer = o.writer - - if o.isVendor { - opt.isVendor = o.isVendor - } - - if o.hasSettingsYaml { - opt.hasSettingsYaml = o.hasSettingsYaml - } - - if o.noSumCheck { - opt.noSumCheck = o.noSumCheck - } - - opt.entries = append(opt.entries, o.entries...) - } - return *opt -} +type Option func(*CompileOptions) // WithKclOption will add a kcl option to the compiler. -func WithKclOption(opt kcl.Option) CompileOptions { - var opts = DefaultCompileOptions() - opts.Merge(opt) - return *opts +func WithKclOption(opt kcl.Option) Option { + return func(opts *CompileOptions) { + opts.Merge(opt) + } } // WithEntries will add entries to the compiler. -func WithEntries(entries []string) CompileOptions { - var opt = DefaultCompileOptions() - opt.entries = entries - return *opt +func WithEntries(entries []string) Option { + return func(opt *CompileOptions) { + opt.entries = append(opt.entries, entries...) + } } // WithEntry will add an entry to the compiler. -func WithVendor(isVendor bool) CompileOptions { - var opt = DefaultCompileOptions() - opt.isVendor = isVendor - return *opt +func WithVendor(isVendor bool) Option { + return func(opt *CompileOptions) { + opt.isVendor = isVendor + } } // WithNoSumCheck will set the 'no_sum_check' flag. -func WithNoSumCheck(is bool) CompileOptions { - var opt = DefaultCompileOptions() - opt.noSumCheck = is - return *opt +func WithNoSumCheck(is bool) Option { + return func(opt *CompileOptions) { + opt.noSumCheck = is + } } // WithLogWriter will set the log writer of the compiler. -// When multiple log writers are set, the compiler will write to the last log writer. -// When combining multiple options, the log should be set at the end. -func WithLogWriter(writer io.Writer) CompileOptions { - var opt = DefaultCompileOptions() - opt.writer = writer - return *opt +func WithLogWriter(writer io.Writer) Option { + return func(opt *CompileOptions) { + opt.writer = writer + } } // DefaultCompileOptions returns a default CompileOptions. diff --git a/pkg/opt/opt_test.go b/pkg/opt/opt_test.go index 54dfeb88..a04ea6a0 100644 --- a/pkg/opt/opt_test.go +++ b/pkg/opt/opt_test.go @@ -20,39 +20,3 @@ func TestWorkDirAsPkgPath(t *testing.T) { opts.SetEntries([]string{"override.k"}) assert.Equal(t, opts.Entries(), []string{"override.k"}) } - -func TestCompilationMerge(t *testing.T) { - opts := MergeOptions( - WithEntries([]string{"test.k"}), - ) - assert.Equal(t, opts.Entries(), []string{"test.k"}) - assert.Equal(t, opts.NoSumCheck(), false) - assert.Equal(t, opts.IsVendor(), false) - - opts = MergeOptions( - opts, - WithNoSumCheck(true), - ) - - assert.Equal(t, opts.Entries(), []string{"test.k"}) - assert.Equal(t, opts.NoSumCheck(), true) - assert.Equal(t, opts.IsVendor(), false) - - opts = MergeOptions( - opts, - WithVendor(true), - ) - - assert.Equal(t, opts.Entries(), []string{"test.k"}) - assert.Equal(t, opts.NoSumCheck(), true) - assert.Equal(t, opts.IsVendor(), true) - - opts = MergeOptions( - opts, - WithEntries([]string{"test1.k"}), - ) - - assert.Equal(t, opts.Entries(), []string{"test.k", "test1.k"}) - assert.Equal(t, opts.NoSumCheck(), true) - assert.Equal(t, opts.IsVendor(), true) -} From 7552f2ba9f47e87c541eedae9b026c28b91caef3 Mon Sep 17 00:00:00 2001 From: zongz Date: Mon, 29 Jan 2024 16:09:55 +0800 Subject: [PATCH 4/4] fix: fix CR comments Signed-off-by: zongz --- pkg/api/kpm_run_test.go | 2 +- pkg/opt/opt.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/api/kpm_run_test.go b/pkg/api/kpm_run_test.go index 5b0ee572..d94723fe 100644 --- a/pkg/api/kpm_run_test.go +++ b/pkg/api/kpm_run_test.go @@ -230,9 +230,9 @@ func TestRunWithOptsWithNoLog(t *testing.T) { pathMainK := filepath.Join(pkgPath, "main.k") _, err := RunWithOpts( + opt.WithLogWriter(nil), opt.WithEntries([]string{pathMainK}), opt.WithKclOption(kcl.WithWorkDir(pkgPath)), - opt.WithLogWriter(nil), ) assert.Equal(t, err, nil) diff --git a/pkg/opt/opt.go b/pkg/opt/opt.go index e6c48032..57e2a0ec 100644 --- a/pkg/opt/opt.go +++ b/pkg/opt/opt.go @@ -36,29 +36,29 @@ func WithKclOption(opt kcl.Option) Option { // WithEntries will add entries to the compiler. func WithEntries(entries []string) Option { - return func(opt *CompileOptions) { - opt.entries = append(opt.entries, entries...) + return func(opts *CompileOptions) { + opts.entries = append(opts.entries, entries...) } } // WithEntry will add an entry to the compiler. func WithVendor(isVendor bool) Option { - return func(opt *CompileOptions) { - opt.isVendor = isVendor + return func(opts *CompileOptions) { + opts.isVendor = isVendor } } // WithNoSumCheck will set the 'no_sum_check' flag. func WithNoSumCheck(is bool) Option { - return func(opt *CompileOptions) { - opt.noSumCheck = is + return func(opts *CompileOptions) { + opts.noSumCheck = is } } // WithLogWriter will set the log writer of the compiler. func WithLogWriter(writer io.Writer) Option { - return func(opt *CompileOptions) { - opt.writer = writer + return func(opts *CompileOptions) { + opts.writer = writer } }