From 4939b2084f56a3190e17629c5dd30ca8bb98a357 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 22 Feb 2024 15:03:20 -0500 Subject: [PATCH 1/3] Rework existing commands to have consistent structure * Rework command subpackages to not export every function/variable * Avoid use of global variables for state * Remove usage of viper (for now); instead pass context directory via the command's context * Make each subcommand have the same structure: cmd.go defines the command, .go defines the logic for running the command. --- cmd/root.go | 90 ++++++++++----------------- cmd/root_test.go | 46 -------------- go.mod | 35 +++-------- go.sum | 73 +++------------------- pkg/cmd/build/build.go | 117 +++--------------------------------- pkg/cmd/build/build_test.go | 81 ------------------------- pkg/cmd/build/cmd.go | 93 ++++++++++++++++++++++++++++ pkg/cmd/build/cmd_test.go | 44 ++++++++++++++ pkg/cmd/export/cmd.go | 108 +++++++++++++++------------------ pkg/cmd/export/export.go | 18 +++--- pkg/cmd/list/cmd.go | 44 ++++++-------- pkg/cmd/login/login.go | 2 +- pkg/cmd/pull/cmd.go | 48 ++++++--------- pkg/cmd/pull/pull.go | 2 +- pkg/cmd/push/cmd.go | 40 +++++------- pkg/cmd/version/version.go | 2 +- pkg/lib/constants/consts.go | 3 + 17 files changed, 311 insertions(+), 535 deletions(-) delete mode 100644 cmd/root_test.go delete mode 100644 pkg/cmd/build/build_test.go create mode 100644 pkg/cmd/build/cmd.go create mode 100644 pkg/cmd/build/cmd_test.go diff --git a/cmd/root.go b/cmd/root.go index dbd1f173..a4a2dc3a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -4,6 +4,12 @@ Copyright © 2024 Jozu.com package cmd import ( + "context" + "fmt" + "os" + "os/user" + "path/filepath" + "kitops/pkg/cmd/build" "kitops/pkg/cmd/export" "kitops/pkg/cmd/list" @@ -11,22 +17,9 @@ import ( "kitops/pkg/cmd/pull" "kitops/pkg/cmd/push" "kitops/pkg/cmd/version" - "os" - "os/user" - "path/filepath" + "kitops/pkg/lib/constants" "github.com/spf13/cobra" - "github.com/spf13/viper" -) - -type ( - RootOptions struct { - ConfigHome string - } - - RootFlags struct { - ConfigHome string - } ) var ( @@ -34,68 +27,51 @@ var ( longDesc = `KitOps is a tool to manage AI and ML models` ) -// rootCmd represents the base command when called without any subcommands -var rootCmd = newRootCmd() - -func init() { - rootCmd.AddCommand(build.NewCmdBuild()) - rootCmd.AddCommand(login.NewCmdLogin()) - rootCmd.AddCommand(pull.PullCommand()) - rootCmd.AddCommand(push.PushCommand()) - rootCmd.AddCommand(list.ListCommand()) - rootCmd.AddCommand(export.ExportCommand()) - rootCmd.AddCommand(version.NewCmdVersion()) +type rootFlags struct { + configHome string } -func newRootCmd() *cobra.Command { - flags := &RootFlags{} +func RunCommand() *cobra.Command { + flags := &rootFlags{} + cmd := &cobra.Command{ Use: "kit", Short: shortDesc, Long: longDesc, PersistentPreRun: func(cmd *cobra.Command, args []string) { - options, err := flags.ToOptions() - if err != nil { - panic(err) - } - err = options.Complete() - if err != nil { - panic(err) + configHome := flags.configHome + if configHome == "" { + currentUser, err := user.Current() + if err != nil { + fmt.Printf("Failed to resolve default storage path '$HOME/%s: could not get current user", constants.DefaultConfigSubdir) + } + configHome = filepath.Join(currentUser.HomeDir, constants.DefaultConfigSubdir) } + ctx := context.WithValue(cmd.Context(), constants.ConfigKey{}, configHome) + cmd.SetContext(ctx) }, } - flags.addFlags(cmd) - return cmd -} -func (f *RootFlags) addFlags(cmd *cobra.Command) { - cmd.PersistentFlags().StringVar(&f.ConfigHome, "config", "", "config file (default is $HOME/.kitops)") - viper.BindPFlag("config", cmd.PersistentFlags().Lookup("config")) -} + addSubcommands(cmd) + cmd.PersistentFlags().StringVar(&flags.configHome, "config", "", fmt.Sprintf("config file (default is $HOME/%s)", constants.DefaultConfigSubdir)) -func (f *RootFlags) ToOptions() (*RootOptions, error) { - return &RootOptions{ - ConfigHome: f.ConfigHome, - }, nil + return cmd } -func (o *RootOptions) Complete() error { - if o.ConfigHome == "" { - currentUser, err := user.Current() - if err != nil { - return err - } - configpath := filepath.Join(currentUser.HomeDir, ".kitops") - viper.Set("config", configpath) - o.ConfigHome = configpath - } - return nil +func addSubcommands(rootCmd *cobra.Command) { + rootCmd.AddCommand(build.BuildCommand()) + rootCmd.AddCommand(login.LoginCommand()) + rootCmd.AddCommand(pull.PullCommand()) + rootCmd.AddCommand(push.PushCommand()) + rootCmd.AddCommand(list.ListCommand()) + rootCmd.AddCommand(export.ExportCommand()) + rootCmd.AddCommand(version.VersionCommand()) } // Execute adds all child commands to the root command and sets flags appropriately. // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { - err := rootCmd.Execute() + err := RunCommand().Execute() if err != nil { os.Exit(1) } diff --git a/cmd/root_test.go b/cmd/root_test.go deleted file mode 100644 index 02cd4631..00000000 --- a/cmd/root_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package cmd - -import ( - "os/user" - "path/filepath" - "testing" - - "github.com/spf13/viper" - "github.com/stretchr/testify/assert" -) - -func TestRootCmd(t *testing.T) { - t.Run("ToOptions", func(t *testing.T) { - t.Run("WithConfigHome", func(t *testing.T) { - f := &RootFlags{ - ConfigHome: "/path/to/config", - } - options, err := f.ToOptions() - assert.NoError(t, err) - assert.Equal(t, "/path/to/config", options.ConfigHome) - }) - }) - t.Run("Complete", func(t *testing.T) { - t.Run("WithConfigHome", func(t *testing.T) { - o := &RootOptions{ - ConfigHome: "/path/to/config", - } - err := o.Complete() - assert.NoError(t, err) - assert.Equal(t, "/path/to/config", o.ConfigHome) - }) - - t.Run("WithoutConfigHome", func(t *testing.T) { - currentUser, err := user.Current() - assert.NoError(t, err) - o := &RootOptions{} - - err = o.Complete() - assert.NoError(t, err) - assert.Equal(t, filepath.Join(currentUser.HomeDir, ".kitops"), o.ConfigHome) - assert.Equal(t, filepath.Join(currentUser.HomeDir, ".kitops"), viper.GetString("config")) - - }) - }) - -} diff --git a/go.mod b/go.mod index e073ae9c..96b85a4c 100644 --- a/go.mod +++ b/go.mod @@ -4,39 +4,18 @@ go 1.21.6 require ( github.com/opencontainers/go-digest v1.0.0 - github.com/opencontainers/image-spec v1.1.0-rc5 + github.com/opencontainers/image-spec v1.1.0 github.com/spf13/cobra v1.8.0 - oras.land/oras-go/v2 v2.3.1 + github.com/stretchr/testify v1.8.4 + oras.land/oras-go/v2 v2.4.0 sigs.k8s.io/yaml v1.4.0 ) require ( - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/fsnotify/fsnotify v1.7.0 // indirect - github.com/hashicorp/hcl v1.0.0 // indirect - github.com/magiconair/properties v1.8.7 // indirect - github.com/mitchellh/mapstructure v1.5.0 // indirect - github.com/pelletier/go-toml/v2 v2.1.0 // indirect - github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/sagikazarmark/locafero v0.4.0 // indirect - github.com/sagikazarmark/slog-shim v0.1.0 // indirect - github.com/sourcegraph/conc v0.3.0 // indirect - github.com/spf13/afero v1.11.0 // indirect - github.com/spf13/cast v1.6.0 // indirect - github.com/subosito/gotenv v1.6.0 // indirect - go.uber.org/atomic v1.9.0 // indirect - go.uber.org/multierr v1.9.0 // indirect - golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect - golang.org/x/sys v0.15.0 // indirect - golang.org/x/text v0.14.0 // indirect - gopkg.in/ini.v1 v1.67.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect -) - -require ( + github.com/davecgh/go-spew v1.1.1 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/spf13/viper v1.18.2 - github.com/stretchr/testify v1.8.4 - golang.org/x/sync v0.5.0 // indirect + golang.org/x/sync v0.6.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 37a5bd7f..7122d066 100644 --- a/go.sum +++ b/go.sum @@ -1,85 +1,30 @@ github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= -github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= -github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= -github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= -github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= -github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= -github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= -github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= -github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= -github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= -github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= -github.com/opencontainers/image-spec v1.1.0-rc5 h1:Ygwkfw9bpDvs+c9E34SdgGOj41dX/cbdlwvlWt0pnFI= -github.com/opencontainers/image-spec v1.1.0-rc5/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= -github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4= -github.com/pelletier/go-toml/v2 v2.1.0/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc= +github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= +github.com/opencontainers/image-spec v1.1.0/go.mod h1:W4s4sFTMaBeK1BQLXbG4AdM2szdn85PY75RI83NrTrM= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= -github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= -github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sagikazarmark/locafero v0.4.0 h1:HApY1R9zGo4DBgr7dqsTH/JJxLTTsOt7u6keLGt6kNQ= -github.com/sagikazarmark/locafero v0.4.0/go.mod h1:Pe1W6UlPYUk/+wc/6KFhbORCfqzgYEpgQ3O5fPuL3H4= -github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6gto+ugjYE= -github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWRIczQRv+GVI1AkeQ= -github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= -github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0= -github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8= -github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY= -github.com/spf13/cast v1.6.0 h1:GEiTHELF+vaR5dhz3VqZfFSzZjYbgeKDpBxQVS4GYJ0= -github.com/spf13/cast v1.6.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/spf13/viper v1.18.2 h1:LUXCnvUvSM6FXAsj6nnfc8Q2tp1dIgUfY9Kc8GsSOiQ= -github.com/spf13/viper v1.18.2/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMVB+yk= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= -github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= -go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= -go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= -go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= -golang.org/x/exp v0.0.0-20230905200255-921286631fa9 h1:GoHiUyI/Tp2nVkLI2mCxVkOjsbSXD66ic0XW0js0R9g= -golang.org/x/exp v0.0.0-20230905200255-921286631fa9/go.mod h1:S2oDrQGGwySpoQPVqRShND87VCbxmc6bL1Yd2oYrm6k= -golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= -golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= -golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= +golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= +golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= -gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -oras.land/oras-go/v2 v2.3.1 h1:lUC6q8RkeRReANEERLfH86iwGn55lbSWP20egdFHVec= -oras.land/oras-go/v2 v2.3.1/go.mod h1:5AQXVEu1X/FKp1F9DMOb5ZItZBOa0y5dha0yCm4NR9c= +oras.land/oras-go/v2 v2.4.0 h1:i+Wt5oCaMHu99guBD0yuBjdLvX7Lz8ukPbwXdR7uBMs= +oras.land/oras-go/v2 v2.4.0/go.mod h1:osvtg0/ClRq1KkydMAEu/IxFieyjItcsQ4ut4PPF+f8= sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= diff --git a/pkg/cmd/build/build.go b/pkg/cmd/build/build.go index 9dd36835..992ff57c 100644 --- a/pkg/cmd/build/build.go +++ b/pkg/cmd/build/build.go @@ -1,95 +1,19 @@ -/* -Copyright © 2024 Jozu.com -*/ package build import ( "fmt" - "os" - "path" - "kitops/pkg/artifact" "kitops/pkg/lib/constants" "kitops/pkg/lib/filesystem" "kitops/pkg/lib/storage" - - "github.com/spf13/cobra" - "github.com/spf13/viper" - "oras.land/oras-go/v2/registry" -) - -var ( - shortDesc = `Build a model` - longDesc = `Build a model TODO` + "os" + "path" ) -type BuildFlags struct { - ModelFile string - FullTagRef string -} - -type BuildOptions struct { - ModelFile string - ContextDir string - configHome string - storageHome string - modelRef *registry.Reference - extraRefs []string -} - -func NewCmdBuild() *cobra.Command { - buildFlags := NewBuildFlags() - - cmd := &cobra.Command{ - Use: "build", - Short: shortDesc, - Long: longDesc, - Run: func(cmd *cobra.Command, args []string) { - options, err := buildFlags.ToOptions() - if err != nil { - fmt.Println(err) - return - } - err = options.Complete(cmd, args) - if err != nil { - fmt.Println(err) - return - } - err = options.Validate() - if err != nil { - fmt.Println(err) - return - } - err = options.RunBuild() - if err != nil { - fmt.Println(err) - return - } - }, - } - buildFlags.AddFlags(cmd) - return cmd -} - -func (options *BuildOptions) Complete(cmd *cobra.Command, argsIn []string) error { - options.ContextDir = argsIn[0] - if options.ModelFile == "" { - options.ModelFile = path.Join(options.ContextDir, constants.DefaultModelFileName) - } - options.configHome = viper.GetString("config") - fmt.Println("config: ", options.configHome) - options.storageHome = storage.StorageHome(options.configHome) - return nil -} - -func (o *BuildOptions) Validate() error { - return nil -} - -func (options *BuildOptions) RunBuild() error { +func RunBuild(options *buildOptions) error { fmt.Println("build called") // 1. Read the model file - modelfile, err := os.Open(options.ModelFile) + modelfile, err := os.Open(options.modelFile) if err != nil { return err } @@ -105,7 +29,7 @@ func (options *BuildOptions) RunBuild() error { // 2. package the Code for _, code := range kitfile.Code { - codePath, err := filesystem.VerifySubpath(options.ContextDir, code.Path) + codePath, err := filesystem.VerifySubpath(options.contextDir, code.Path) if err != nil { return err } @@ -117,7 +41,7 @@ func (options *BuildOptions) RunBuild() error { } // 3. package the DataSets for _, dataset := range kitfile.DataSets { - datasetPath, err := filesystem.VerifySubpath(options.ContextDir, dataset.Path) + datasetPath, err := filesystem.VerifySubpath(options.contextDir, dataset.Path) if err != nil { return err } @@ -129,8 +53,7 @@ func (options *BuildOptions) RunBuild() error { } // 4. package the TrainedModel - - modelPath, err := filesystem.VerifySubpath(options.ContextDir, kitfile.Model.Path) + modelPath, err := filesystem.VerifySubpath(options.contextDir, kitfile.Model.Path) if err != nil { return err } @@ -164,29 +87,3 @@ func (options *BuildOptions) RunBuild() error { return nil } - -func (o *BuildFlags) ToOptions() (*BuildOptions, error) { - options := &BuildOptions{} - if o.ModelFile != "" { - options.ModelFile = o.ModelFile - } - if o.FullTagRef != "" { - modelRef, extraRefs, err := storage.ParseReference(o.FullTagRef) - if err != nil { - return nil, err - } - options.modelRef = modelRef - options.extraRefs = extraRefs - } - return options, nil -} - -func (flags *BuildFlags) AddFlags(cmd *cobra.Command) { - cmd.Flags().StringVarP(&flags.ModelFile, "file", "f", "", "Path to the model file") - cmd.Flags().StringVarP(&flags.FullTagRef, "tag", "t", "", "Tag for the model. Example: -t registry/repository:tag1,tag2") - cmd.Args = cobra.ExactArgs(1) -} - -func NewBuildFlags() *BuildFlags { - return &BuildFlags{} -} diff --git a/pkg/cmd/build/build_test.go b/pkg/cmd/build/build_test.go deleted file mode 100644 index 2f49a9c7..00000000 --- a/pkg/cmd/build/build_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package build - -import ( - "kitops/pkg/lib/constants" - "path" - "testing" - - "github.com/spf13/cobra" - - "github.com/stretchr/testify/assert" -) - -func TestNewCmdBuild(t *testing.T) { - cmd := NewCmdBuild() - - assert.NotNil(t, cmd) - assert.Equal(t, "build", cmd.Use) - assert.Equal(t, shortDesc, cmd.Short) - assert.Equal(t, longDesc, cmd.Long) -} - -func TestBuildOptions_Complete(t *testing.T) { - options := &BuildOptions{} - cmd := &cobra.Command{} - args := []string{"arg1"} - - err := options.Complete(cmd, args) - - assert.NoError(t, err) - assert.Equal(t, args[0], options.ContextDir) - assert.Equal(t, path.Join(options.ContextDir, constants.DefaultModelFileName), options.ModelFile) -} - -func TestBuildOptions_Validate(t *testing.T) { - options := &BuildOptions{ - ModelFile: "Kitfile", - ContextDir: "/path/to/context", - } - - err := options.Validate() - - assert.NoError(t, err) -} - -func TestBuildOptions_RunBuild(t *testing.T) { - t.Skip("Skipping test for now") - options := &BuildOptions{ - ModelFile: "Kitfile", - ContextDir: "/path/to/context", - } - - err := options.RunBuild() - - assert.NoError(t, err) -} - -func TestBuildFlags_ToOptions(t *testing.T) { - flags := &BuildFlags{ - ModelFile: "Kitfile", - } - - options, err := flags.ToOptions() - - assert.NoError(t, err) - assert.Equal(t, flags.ModelFile, options.ModelFile) -} - -func TestBuildFlags_AddFlags(t *testing.T) { - flags := &BuildFlags{} - cmd := &cobra.Command{} - - flags.AddFlags(cmd) - - assert.NotNil(t, cmd.Flags().Lookup("file")) -} - -func TestNewBuildFlags(t *testing.T) { - flags := NewBuildFlags() - - assert.NotNil(t, flags) -} diff --git a/pkg/cmd/build/cmd.go b/pkg/cmd/build/cmd.go new file mode 100644 index 00000000..3ca68f07 --- /dev/null +++ b/pkg/cmd/build/cmd.go @@ -0,0 +1,93 @@ +/* +Copyright © 2024 Jozu.com +*/ +package build + +import ( + "context" + "fmt" + "path" + + "kitops/pkg/lib/constants" + "kitops/pkg/lib/storage" + + "github.com/spf13/cobra" + "oras.land/oras-go/v2/registry" +) + +var ( + shortDesc = `Build a model` + longDesc = `Build a model TODO` +) + +type buildFlags struct { + modelFile string + fullTagRef string +} + +type buildOptions struct { + modelFile string + contextDir string + configHome string + storageHome string + modelRef *registry.Reference + extraRefs []string +} + +func BuildCommand() *cobra.Command { + flags := &buildFlags{} + + cmd := &cobra.Command{ + Use: "build", + Short: shortDesc, + Long: longDesc, + Run: runCommand(flags), + } + + cmd.Flags().StringVarP(&flags.modelFile, "file", "f", "", "Path to the model file") + cmd.Flags().StringVarP(&flags.fullTagRef, "tag", "t", "", "Tag for the model. Example: -t registry/repository:tag1,tag2") + cmd.Args = cobra.ExactArgs(1) + return cmd +} + +func runCommand(flags *buildFlags) func(cmd *cobra.Command, args []string) { + return func(cmd *cobra.Command, args []string) { + opts := &buildOptions{} + err := opts.complete(cmd.Context(), flags, args) + if err != nil { + fmt.Println(err) + return + } + err = RunBuild(opts) + if err != nil { + fmt.Println(err) + return + } + } +} + +func (opts *buildOptions) complete(ctx context.Context, flags *buildFlags, args []string) error { + opts.contextDir = args[0] + + opts.modelFile = flags.modelFile + if opts.modelFile == "" { + opts.modelFile = path.Join(opts.contextDir, constants.DefaultModelFileName) + } + + configHome, ok := ctx.Value(constants.ConfigKey{}).(string) + if !ok { + return fmt.Errorf("default config path not set on command context") + } + opts.configHome = configHome + opts.storageHome = storage.StorageHome(opts.configHome) + + if flags.fullTagRef != "" { + modelRef, extraRefs, err := storage.ParseReference(flags.fullTagRef) + if err != nil { + return fmt.Errorf("failed to parse reference %s: %w", flags.fullTagRef, err) + } + opts.modelRef = modelRef + opts.extraRefs = extraRefs + } + return nil +} diff --git a/pkg/cmd/build/cmd_test.go b/pkg/cmd/build/cmd_test.go new file mode 100644 index 00000000..877cae29 --- /dev/null +++ b/pkg/cmd/build/cmd_test.go @@ -0,0 +1,44 @@ +package build + +import ( + "context" + "kitops/pkg/lib/constants" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewCmdBuild(t *testing.T) { + cmd := BuildCommand() + + assert.NotNil(t, cmd) + assert.Equal(t, "build", cmd.Use) + assert.Equal(t, shortDesc, cmd.Short) + assert.Equal(t, longDesc, cmd.Long) +} + +func TestBuildOptions_Complete(t *testing.T) { + options := &buildOptions{} + flags := &buildFlags{} + ctx := context.WithValue(context.Background(), constants.ConfigKey{}, "/home/user/.kitops") + args := []string{"arg1"} + + err := options.complete(ctx, flags, args) + + assert.NoError(t, err) + assert.Equal(t, args[0], options.contextDir) + assert.Equal(t, filepath.Join(args[0], constants.DefaultModelFileName), options.modelFile) +} + +func TestBuildOptions_RunBuild(t *testing.T) { + t.Skip("Skipping test for now") + options := &buildOptions{ + modelFile: "Kitfile", + contextDir: "/path/to/context", + } + + err := RunBuild(options) + + assert.NoError(t, err) +} diff --git a/pkg/cmd/export/cmd.go b/pkg/cmd/export/cmd.go index cb1460b7..0a291281 100644 --- a/pkg/cmd/export/cmd.go +++ b/pkg/cmd/export/cmd.go @@ -4,10 +4,10 @@ import ( "context" "errors" "fmt" + "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" "github.com/spf13/cobra" - "github.com/spf13/viper" "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/oci" "oras.land/oras-go/v2/errdef" @@ -20,40 +20,39 @@ const ( longDesc = `Export model from registry TODO` ) -var ( - flags *ExportFlags - opts *ExportOptions -) - -type ExportFlags struct { - Overwrite bool - UseHTTP bool - ExportConfig bool - ExportModels bool - ExportDatasets bool - ExportCode bool - ExportDir string +type exportFlags struct { + overwrite bool + useHTTP bool + exportConfig bool + exportModels bool + exportDatasets bool + exportCode bool + exportDir string } -type ExportOptions struct { +type exportOptions struct { configHome string storageHome string exportDir string overwrite bool - exportConf ExportConf + exportConf exportConf modelRef *registry.Reference usehttp bool } -type ExportConf struct { - ExportConfig bool - ExportModels bool - ExportCode bool - ExportDatasets bool +type exportConf struct { + exportConfig bool + exportModels bool + exportCode bool + exportDatasets bool } -func (opts *ExportOptions) complete(args []string) error { - opts.configHome = viper.GetString("config") +func (opts *exportOptions) complete(ctx context.Context, flags *exportFlags, args []string) error { + configHome, ok := ctx.Value(constants.ConfigKey{}).(string) + if !ok { + return fmt.Errorf("default config path not set on command context") + } + opts.configHome = configHome opts.storageHome = storage.StorageHome(opts.configHome) modelRef, extraTags, err := storage.ParseReference(args[0]) if err != nil { @@ -63,63 +62,54 @@ func (opts *ExportOptions) complete(args []string) error { return fmt.Errorf("can not export multiple tags") } opts.modelRef = modelRef - opts.overwrite = flags.Overwrite - opts.usehttp = flags.UseHTTP - opts.exportDir = flags.ExportDir - - if !flags.ExportConfig && !flags.ExportModels && !flags.ExportCode && !flags.ExportDatasets { - opts.exportConf.ExportConfig = true - opts.exportConf.ExportModels = true - opts.exportConf.ExportCode = true - opts.exportConf.ExportDatasets = true + opts.overwrite = flags.overwrite + opts.usehttp = flags.useHTTP + opts.exportDir = flags.exportDir + + if !flags.exportConfig && !flags.exportModels && !flags.exportCode && !flags.exportDatasets { + opts.exportConf.exportConfig = true + opts.exportConf.exportModels = true + opts.exportConf.exportCode = true + opts.exportConf.exportDatasets = true } else { - opts.exportConf.ExportConfig = flags.ExportConfig - opts.exportConf.ExportModels = flags.ExportModels - opts.exportConf.ExportCode = flags.ExportCode - opts.exportConf.ExportDatasets = flags.ExportDatasets + opts.exportConf.exportConfig = flags.exportConfig + opts.exportConf.exportModels = flags.exportModels + opts.exportConf.exportCode = flags.exportCode + opts.exportConf.exportDatasets = flags.exportDatasets } return nil } -func (opts *ExportOptions) validate() error { - return nil -} - func ExportCommand() *cobra.Command { - opts = &ExportOptions{} - flags = &ExportFlags{} + flags := &exportFlags{} cmd := &cobra.Command{ Use: "export", Short: shortDesc, Long: longDesc, - Run: runCommand(opts), + Run: runCommand(flags), } cmd.Args = cobra.ExactArgs(1) - cmd.Flags().StringVarP(&flags.ExportDir, "dir", "d", "", "Directory to export into. Will be created if it does not exist") - cmd.Flags().BoolVarP(&flags.Overwrite, "overwrite", "o", false, "Overwrite existing files and directories in the export dir") - cmd.Flags().BoolVar(&flags.ExportConfig, "config", false, "Export only config file") - cmd.Flags().BoolVar(&flags.ExportModels, "models", false, "Export only models") - cmd.Flags().BoolVar(&flags.ExportCode, "code", false, "Export only code") - cmd.Flags().BoolVar(&flags.ExportDatasets, "datasets", false, "Export only datasets") - cmd.Flags().BoolVar(&flags.UseHTTP, "http", false, "Use plain HTTP when connecting to remote registries") + cmd.Flags().StringVarP(&flags.exportDir, "dir", "d", "", "Directory to export into. Will be created if it does not exist") + cmd.Flags().BoolVarP(&flags.overwrite, "overwrite", "o", false, "Overwrite existing files and directories in the export dir") + cmd.Flags().BoolVar(&flags.exportConfig, "config", false, "Export only config file") + cmd.Flags().BoolVar(&flags.exportModels, "models", false, "Export only models") + cmd.Flags().BoolVar(&flags.exportCode, "code", false, "Export only code") + cmd.Flags().BoolVar(&flags.exportDatasets, "datasets", false, "Export only datasets") + cmd.Flags().BoolVar(&flags.useHTTP, "http", false, "Use plain HTTP when connecting to remote registries") return cmd } -func runCommand(opts *ExportOptions) func(*cobra.Command, []string) { +func runCommand(flags *exportFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - if err := opts.complete(args); err != nil { + opts := &exportOptions{} + if err := opts.complete(cmd.Context(), flags, args); err != nil { fmt.Printf("Failed to process arguments: %s", err) return } - err := opts.validate() - if err != nil { - fmt.Println(err) - return - } store, err := getStoreForRef(cmd.Context(), opts) if err != nil { @@ -132,7 +122,7 @@ func runCommand(opts *ExportOptions) func(*cobra.Command, []string) { exportTo = "current directory" } fmt.Printf("Exporting to %s\n", exportTo) - err = ExportModel(cmd.Context(), store, opts.modelRef, opts) + err = exportModel(cmd.Context(), store, opts.modelRef, opts) if err != nil { fmt.Println(err) return @@ -140,7 +130,7 @@ func runCommand(opts *ExportOptions) func(*cobra.Command, []string) { } } -func getStoreForRef(ctx context.Context, opts *ExportOptions) (oras.Target, error) { +func getStoreForRef(ctx context.Context, opts *exportOptions) (oras.Target, error) { localStore, err := oci.New(storage.LocalStorePath(opts.storageHome, opts.modelRef)) if err != nil { return nil, fmt.Errorf("failed to read local storage: %s\n", err) diff --git a/pkg/cmd/export/export.go b/pkg/cmd/export/export.go index 99e62159..fbe90fa9 100644 --- a/pkg/cmd/export/export.go +++ b/pkg/cmd/export/export.go @@ -21,7 +21,7 @@ import ( "sigs.k8s.io/yaml" ) -func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference, options *ExportOptions) error { +func exportModel(ctx context.Context, store oras.Target, ref *registry.Reference, options *exportOptions) error { manifestDesc, err := store.Resolve(ctx, ref.Reference) if err != nil { return fmt.Errorf("failed to resolve local reference: %w", err) @@ -31,8 +31,8 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference return fmt.Errorf("failed to read local model: %s", err) } - if options.exportConf.ExportConfig { - if err := ExportConfig(config, options.exportDir, options.overwrite); err != nil { + if options.exportConf.exportConfig { + if err := exportConfig(config, options.exportDir, options.overwrite); err != nil { return err } } @@ -44,7 +44,7 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference layerDir := "" switch layerDesc.MediaType { case constants.ModelLayerMediaType: - if !options.exportConf.ExportModels { + if !options.exportConf.exportModels { continue } modelEntry := config.Model @@ -52,7 +52,7 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference fmt.Printf("Exporting model %s to %s\n", modelEntry.Name, layerDir) case constants.CodeLayerMediaType: - if !options.exportConf.ExportCode { + if !options.exportConf.exportCode { continue } codeEntry := config.Code[codeIdx] @@ -61,7 +61,7 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference codeIdx += 1 case constants.DataSetLayerMediaType: - if !options.exportConf.ExportDatasets { + if !options.exportConf.exportDatasets { continue } datasetEntry := config.DataSets[datasetIdx] @@ -69,7 +69,7 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference fmt.Printf("Exporting dataset %s to %s\n", datasetEntry.Name, layerDir) datasetIdx += 1 } - if err := ExportLayer(ctx, store, layerDesc, layerDir, options.overwrite); err != nil { + if err := exportLayer(ctx, store, layerDesc, layerDir, options.overwrite); err != nil { return err } } @@ -77,7 +77,7 @@ func ExportModel(ctx context.Context, store oras.Target, ref *registry.Reference return nil } -func ExportConfig(config *artifact.KitFile, exportDir string, overwrite bool) error { +func exportConfig(config *artifact.KitFile, exportDir string, overwrite bool) error { configPath := path.Join(exportDir, constants.DefaultModelFileName) if fi, exists := filesystem.PathExists(configPath); exists { if !overwrite { @@ -99,7 +99,7 @@ func ExportConfig(config *artifact.KitFile, exportDir string, overwrite bool) er return nil } -func ExportLayer(ctx context.Context, store content.Storage, desc ocispec.Descriptor, exportDir string, overwrite bool) error { +func exportLayer(ctx context.Context, store content.Storage, desc ocispec.Descriptor, exportDir string, overwrite bool) error { rc, err := store.Fetch(ctx, desc) if err != nil { return fmt.Errorf("failed get layer %s: %w", desc.Digest, err) diff --git a/pkg/cmd/list/cmd.go b/pkg/cmd/list/cmd.go index 44bb6dc8..6f0b66e1 100644 --- a/pkg/cmd/list/cmd.go +++ b/pkg/cmd/list/cmd.go @@ -1,14 +1,15 @@ package list import ( + "context" "fmt" + "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" "os" "path" "text/tabwriter" "github.com/spf13/cobra" - "github.com/spf13/viper" "oras.land/oras-go/v2/registry" ) @@ -17,24 +18,23 @@ const ( longDesc = `List model kits TODO` ) -var ( - flags *ListFlags - opts *ListOptions -) - -type ListFlags struct { - UseHTTP bool +type listFlags struct { + useHTTP bool } -type ListOptions struct { +type listOptions struct { configHome string storageHome string remoteRef *registry.Reference usehttp bool } -func (opts *ListOptions) complete(flags *ListFlags, args []string) error { - opts.configHome = viper.GetString("config") +func (opts *listOptions) complete(ctx context.Context, flags *listFlags, args []string) error { + configHome, ok := ctx.Value(constants.ConfigKey{}).(string) + if !ok { + return fmt.Errorf("default config path not set on command context") + } + opts.configHome = configHome opts.storageHome = path.Join(opts.configHome, "storage") if len(args) > 0 { remoteRef, extraTags, err := storage.ParseReference(args[0]) @@ -46,41 +46,33 @@ func (opts *ListOptions) complete(flags *ListFlags, args []string) error { } opts.remoteRef = remoteRef } - opts.usehttp = flags.UseHTTP - return nil -} - -func (opts *ListOptions) validate() error { + opts.usehttp = flags.useHTTP return nil } // ListCommand represents the models command func ListCommand() *cobra.Command { - flags = &ListFlags{} - opts = &ListOptions{} + flags := &listFlags{} cmd := &cobra.Command{ Use: "list [repository]", Short: shortDesc, Long: longDesc, - Run: RunCommand(opts), + Run: runCommand(flags), } cmd.Args = cobra.MaximumNArgs(1) - cmd.Flags().BoolVar(&flags.UseHTTP, "http", false, "Use plain HTTP when connecting to remote registries") + cmd.Flags().BoolVar(&flags.useHTTP, "http", false, "Use plain HTTP when connecting to remote registries") return cmd } -func RunCommand(options *ListOptions) func(*cobra.Command, []string) { +func runCommand(flags *listFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - if err := options.complete(flags, args); err != nil { + opts := &listOptions{} + if err := opts.complete(cmd.Context(), flags, args); err != nil { fmt.Printf("Failed to parse argument: %s", err) return } - if err := options.validate(); err != nil { - fmt.Println(err) - return - } var allInfoLines []string if opts.remoteRef == nil { diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 2b34739f..26334e92 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -9,7 +9,7 @@ import ( "github.com/spf13/cobra" ) -func NewCmdLogin() *cobra.Command { +func LoginCommand() *cobra.Command { cmd := &cobra.Command{ Use: "login", Short: "A brief description of your command", diff --git a/pkg/cmd/pull/cmd.go b/pkg/cmd/pull/cmd.go index 346830ca..35b73430 100644 --- a/pkg/cmd/pull/cmd.go +++ b/pkg/cmd/pull/cmd.go @@ -1,11 +1,12 @@ package pull import ( + "context" "fmt" + "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" "github.com/spf13/cobra" - "github.com/spf13/viper" "oras.land/oras-go/v2/content/oci" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote" @@ -16,25 +17,25 @@ const ( longDesc = `Pull model from registry TODO` ) -var ( - flags *PullFlags - opts *PullOptions -) - -type PullFlags struct { - UseHTTP bool +type pullFlags struct { + useHTTP bool } -type PullOptions struct { +type pullOptions struct { usehttp bool configHome string storageHome string modelRef *registry.Reference } -func (opts *PullOptions) complete(args []string) error { - opts.configHome = viper.GetString("config") +func (opts *pullOptions) complete(ctx context.Context, flags *pullFlags, args []string) error { + configHome, ok := ctx.Value(constants.ConfigKey{}).(string) + if !ok { + return fmt.Errorf("default config path not set on command context") + } + opts.configHome = configHome opts.storageHome = storage.StorageHome(opts.configHome) + modelRef, extraTags, err := storage.ParseReference(args[0]) if err != nil { return fmt.Errorf("failed to parse reference %s: %w", modelRef, err) @@ -46,40 +47,31 @@ func (opts *PullOptions) complete(args []string) error { return fmt.Errorf("reference cannot include multiple tags") } opts.modelRef = modelRef - opts.usehttp = flags.UseHTTP - return nil -} - -func (opts *PullOptions) validate() error { + opts.usehttp = flags.useHTTP return nil } func PullCommand() *cobra.Command { - opts = &PullOptions{} - flags = &PullFlags{} + flags := &pullFlags{} cmd := &cobra.Command{ Use: "pull", Short: shortDesc, Long: longDesc, - Run: runCommand(opts), + Run: runCommand(flags), } cmd.Args = cobra.ExactArgs(1) - cmd.Flags().BoolVar(&flags.UseHTTP, "http", false, "Use plain HTTP when connecting to remote registries") + cmd.Flags().BoolVar(&flags.useHTTP, "http", false, "Use plain HTTP when connecting to remote registries") return cmd } -func runCommand(opts *PullOptions) func(*cobra.Command, []string) { +func runCommand(flags *pullFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - if err := opts.complete(args); err != nil { + opts := &pullOptions{} + if err := opts.complete(cmd.Context(), flags, args); err != nil { fmt.Printf("Failed to process arguments: %s", err) } - err := opts.validate() - if err != nil { - fmt.Println(err) - return - } remoteRegistry, err := remote.NewRegistry(opts.modelRef.Registry) if err != nil { fmt.Println(err) @@ -97,7 +89,7 @@ func runCommand(opts *PullOptions) func(*cobra.Command, []string) { } fmt.Printf("Pulling %s\n", opts.modelRef.String()) - desc, err := PullModel(cmd.Context(), remoteRegistry, localStore, opts.modelRef) + desc, err := pullModel(cmd.Context(), remoteRegistry, localStore, opts.modelRef) if err != nil { fmt.Printf("Failed to pull: %s\n", err) return diff --git a/pkg/cmd/pull/pull.go b/pkg/cmd/pull/pull.go index 9b6de397..6e9a3bb0 100644 --- a/pkg/cmd/pull/pull.go +++ b/pkg/cmd/pull/pull.go @@ -17,7 +17,7 @@ import ( "oras.land/oras-go/v2/registry/remote" ) -func PullModel(ctx context.Context, remoteRegistry *remote.Registry, localStore *oci.Store, ref *registry.Reference) (ocispec.Descriptor, error) { +func pullModel(ctx context.Context, remoteRegistry *remote.Registry, localStore *oci.Store, ref *registry.Reference) (ocispec.Descriptor, error) { repo, err := remoteRegistry.Repository(ctx, ref.Repository) if err != nil { return ocispec.DescriptorEmptyJSON, fmt.Errorf("failed to read repository: %w", err) diff --git a/pkg/cmd/push/cmd.go b/pkg/cmd/push/cmd.go index e8c93aa4..f10ba7ae 100644 --- a/pkg/cmd/push/cmd.go +++ b/pkg/cmd/push/cmd.go @@ -1,11 +1,12 @@ package push import ( + "context" "fmt" + "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" "github.com/spf13/cobra" - "github.com/spf13/viper" "oras.land/oras-go/v2/content/oci" "oras.land/oras-go/v2/registry" "oras.land/oras-go/v2/registry/remote" @@ -16,24 +17,23 @@ const ( longDesc = `Push model to registry TODO` ) -var ( - flags *PushFlags - opts *PushOptions -) - -type PushFlags struct { +type pushFlags struct { UseHTTP bool } -type PushOptions struct { +type pushOptions struct { usehttp bool configHome string storageHome string modelRef *registry.Reference } -func (opts *PushOptions) complete(args []string) error { - opts.configHome = viper.GetString("config") +func (opts *pushOptions) complete(ctx context.Context, flags *pushFlags, args []string) error { + configHome, ok := ctx.Value(constants.ConfigKey{}).(string) + if !ok { + return fmt.Errorf("default config path not set on command context") + } + opts.configHome = configHome opts.storageHome = storage.StorageHome(opts.configHome) modelRef, extraTags, err := storage.ParseReference(args[0]) if err != nil { @@ -50,19 +50,14 @@ func (opts *PushOptions) complete(args []string) error { return nil } -func (opts *PushOptions) validate() error { - return nil -} - func PushCommand() *cobra.Command { - opts = &PushOptions{} - flags = &PushFlags{} + flags := &pushFlags{} cmd := &cobra.Command{ Use: "push", Short: shortDesc, Long: longDesc, - Run: runCommand(opts), + Run: runCommand(flags), } cmd.Args = cobra.ExactArgs(1) @@ -70,16 +65,13 @@ func PushCommand() *cobra.Command { return cmd } -func runCommand(opts *PushOptions) func(*cobra.Command, []string) { +func runCommand(flags *pushFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { - if err := opts.complete(args); err != nil { + opts := &pushOptions{} + if err := opts.complete(cmd.Context(), flags, args); err != nil { fmt.Printf("Failed to process arguments: %s", err) } - err := opts.validate() - if err != nil { - fmt.Println(err) - return - } + remoteRegistry, err := remote.NewRegistry(opts.modelRef.Registry) if err != nil { fmt.Println(err) diff --git a/pkg/cmd/version/version.go b/pkg/cmd/version/version.go index 42992bf1..a0d2641a 100644 --- a/pkg/cmd/version/version.go +++ b/pkg/cmd/version/version.go @@ -16,7 +16,7 @@ var ( GoVersion = runtime.Version() ) -func NewCmdVersion() *cobra.Command { +func VersionCommand() *cobra.Command { cmd := &cobra.Command{ Use: "version", diff --git a/pkg/lib/constants/consts.go b/pkg/lib/constants/consts.go index 5204d998..51d171a7 100644 --- a/pkg/lib/constants/consts.go +++ b/pkg/lib/constants/consts.go @@ -1,7 +1,10 @@ package constants +type ConfigKey struct{} + const ( DefaultModelFileName = "Kitfile" + DefaultConfigSubdir = ".kitops" // Media type for the model layer ModelLayerMediaType = "application/vnd.kitops.modelkit.model.v1.tar+gzip" From a39887a159f5c6d7b1d3fc15fa75a9704dbafb4e Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 22 Feb 2024 15:42:38 -0500 Subject: [PATCH 2/3] Collect all output into one package for more consistent handling Add pkg/output package to control logging and text output while running commands. Package includes a debug log level, which can be enabled by using the global flag '-v|--verbose'. Add verbose logging to commands where appropriate and slim down non-verbose log --- cmd/root.go | 11 +++++-- pkg/cmd/build/build.go | 8 ++--- pkg/cmd/build/cmd.go | 21 ++++++++++-- pkg/cmd/export/cmd.go | 19 +++++++---- pkg/cmd/export/export.go | 18 +++++++---- pkg/cmd/list/cmd.go | 18 +++++++---- pkg/cmd/login/login.go | 4 +-- pkg/cmd/pull/cmd.go | 21 +++++++----- pkg/cmd/push/cmd.go | 21 +++++++----- pkg/cmd/version/version.go | 4 +-- pkg/lib/storage/local.go | 16 ++++++--- pkg/output/logging.go | 66 ++++++++++++++++++++++++++++++++++++++ 12 files changed, 171 insertions(+), 56 deletions(-) create mode 100644 pkg/output/logging.go diff --git a/cmd/root.go b/cmd/root.go index a4a2dc3a..958b12d2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -18,6 +18,7 @@ import ( "kitops/pkg/cmd/push" "kitops/pkg/cmd/version" "kitops/pkg/lib/constants" + "kitops/pkg/output" "github.com/spf13/cobra" ) @@ -29,6 +30,7 @@ var ( type rootFlags struct { configHome string + verbose bool } func RunCommand() *cobra.Command { @@ -43,18 +45,21 @@ func RunCommand() *cobra.Command { if configHome == "" { currentUser, err := user.Current() if err != nil { - fmt.Printf("Failed to resolve default storage path '$HOME/%s: could not get current user", constants.DefaultConfigSubdir) + output.Fatalf("Failed to resolve default storage path '$HOME/%s: could not get current user", constants.DefaultConfigSubdir) } configHome = filepath.Join(currentUser.HomeDir, constants.DefaultConfigSubdir) } + if flags.verbose { + output.SetDebug(true) + } ctx := context.WithValue(cmd.Context(), constants.ConfigKey{}, configHome) cmd.SetContext(ctx) }, } addSubcommands(cmd) - cmd.PersistentFlags().StringVar(&flags.configHome, "config", "", fmt.Sprintf("config file (default is $HOME/%s)", constants.DefaultConfigSubdir)) - + cmd.PersistentFlags().StringVar(&flags.configHome, "config", "", fmt.Sprintf("Config file (default $HOME/%s)", constants.DefaultConfigSubdir)) + cmd.PersistentFlags().BoolVarP(&flags.verbose, "verbose", "v", false, "Include additional information in output (default false)") return cmd } diff --git a/pkg/cmd/build/build.go b/pkg/cmd/build/build.go index 992ff57c..8dc43c4e 100644 --- a/pkg/cmd/build/build.go +++ b/pkg/cmd/build/build.go @@ -1,17 +1,16 @@ package build import ( - "fmt" "kitops/pkg/artifact" "kitops/pkg/lib/constants" "kitops/pkg/lib/filesystem" "kitops/pkg/lib/storage" + "kitops/pkg/output" "os" "path" ) func RunBuild(options *buildOptions) error { - fmt.Println("build called") // 1. Read the model file modelfile, err := os.Open(options.modelFile) if err != nil { @@ -20,7 +19,6 @@ func RunBuild(options *buildOptions) error { defer modelfile.Close() kitfile := &artifact.KitFile{} if err = kitfile.LoadModel(modelfile); err != nil { - fmt.Println(err) return err } @@ -39,6 +37,7 @@ func RunBuild(options *buildOptions) error { } model.Layers = append(model.Layers, *layer) } + // 3. package the DataSets for _, dataset := range kitfile.DataSets { datasetPath, err := filesystem.VerifySubpath(options.contextDir, dataset.Path) @@ -73,7 +72,6 @@ func RunBuild(options *buildOptions) error { store := storage.NewLocalStore(modelStorePath, repo) manifestDesc, err := store.SaveModel(model, tag) if err != nil { - fmt.Println(err) return err } @@ -83,7 +81,7 @@ func RunBuild(options *buildOptions) error { } } - fmt.Println("Model saved: ", manifestDesc.Digest) + output.Infof("Model saved: %s", manifestDesc.Digest) return nil } diff --git a/pkg/cmd/build/cmd.go b/pkg/cmd/build/cmd.go index 3ca68f07..ba68b44d 100644 --- a/pkg/cmd/build/cmd.go +++ b/pkg/cmd/build/cmd.go @@ -7,9 +7,11 @@ import ( "context" "fmt" "path" + "strings" "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" + "kitops/pkg/output" "github.com/spf13/cobra" "oras.land/oras-go/v2/registry" @@ -55,12 +57,12 @@ func runCommand(flags *buildFlags) func(cmd *cobra.Command, args []string) { opts := &buildOptions{} err := opts.complete(cmd.Context(), flags, args) if err != nil { - fmt.Println(err) + output.Fatalf("Failed to process configuration: %s", err) return } err = RunBuild(opts) if err != nil { - fmt.Println(err) + output.Fatalf("Failed to build model kit: %s", err) return } } @@ -89,5 +91,20 @@ func (opts *buildOptions) complete(ctx context.Context, flags *buildFlags, args opts.modelRef = modelRef opts.extraRefs = extraRefs } + printConfig(opts) return nil } + +func printConfig(opts *buildOptions) { + output.Debugf("Using storage path: %s", opts.storageHome) + output.Debugf("Context dir: %s", opts.contextDir) + output.Debugf("Model file: %s", opts.modelFile) + if opts.modelRef != nil { + output.Debugf("Building %s", opts.modelRef.String()) + } else { + output.Debugln("No tag or reference specified") + } + if len(opts.extraRefs) > 0 { + output.Debugf("Additional tags: %s", strings.Join(opts.extraRefs, ", ")) + } +} diff --git a/pkg/cmd/export/cmd.go b/pkg/cmd/export/cmd.go index 0a291281..afc50a13 100644 --- a/pkg/cmd/export/cmd.go +++ b/pkg/cmd/export/cmd.go @@ -6,6 +6,7 @@ import ( "fmt" "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" + "kitops/pkg/output" "github.com/spf13/cobra" "oras.land/oras-go/v2" @@ -78,6 +79,7 @@ func (opts *exportOptions) complete(ctx context.Context, flags *exportFlags, arg opts.exportConf.exportDatasets = flags.exportDatasets } + printConfig(opts) return nil } @@ -107,25 +109,22 @@ func runCommand(flags *exportFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { opts := &exportOptions{} if err := opts.complete(cmd.Context(), flags, args); err != nil { - fmt.Printf("Failed to process arguments: %s", err) - return + output.Fatalf("Failed to process arguments: %s", err) } store, err := getStoreForRef(cmd.Context(), opts) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } exportTo := opts.exportDir if exportTo == "" { exportTo = "current directory" } - fmt.Printf("Exporting to %s\n", exportTo) + output.Infof("Exporting to %s", exportTo) err = exportModel(cmd.Context(), store, opts.modelRef, opts) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } } } @@ -163,3 +162,9 @@ func getStoreForRef(ctx context.Context, opts *exportOptions) (oras.Target, erro return repo, nil } + +func printConfig(opts *exportOptions) { + output.Debugf("Using storage path: %s", opts.storageHome) + output.Debugf("Overwrite: %t", opts.overwrite) + output.Debugf("Exporting %s", opts.modelRef.String()) +} diff --git a/pkg/cmd/export/export.go b/pkg/cmd/export/export.go index fbe90fa9..22f45438 100644 --- a/pkg/cmd/export/export.go +++ b/pkg/cmd/export/export.go @@ -10,6 +10,7 @@ import ( "kitops/pkg/lib/constants" "kitops/pkg/lib/filesystem" "kitops/pkg/lib/repo" + "kitops/pkg/output" "os" "path" "path/filepath" @@ -49,7 +50,7 @@ func exportModel(ctx context.Context, store oras.Target, ref *registry.Reference } modelEntry := config.Model layerDir = filepath.Join(options.exportDir, modelEntry.Path) - fmt.Printf("Exporting model %s to %s\n", modelEntry.Name, layerDir) + output.Infof("Exporting model %s to %s", modelEntry.Name, layerDir) case constants.CodeLayerMediaType: if !options.exportConf.exportCode { @@ -57,7 +58,7 @@ func exportModel(ctx context.Context, store oras.Target, ref *registry.Reference } codeEntry := config.Code[codeIdx] layerDir = filepath.Join(options.exportDir, codeEntry.Path) - fmt.Printf("Exporting code to %s\n", layerDir) + output.Infof("Exporting code to %s", layerDir) codeIdx += 1 case constants.DataSetLayerMediaType: @@ -66,13 +67,15 @@ func exportModel(ctx context.Context, store oras.Target, ref *registry.Reference } datasetEntry := config.DataSets[datasetIdx] layerDir = filepath.Join(options.exportDir, datasetEntry.Path) - fmt.Printf("Exporting dataset %s to %s\n", datasetEntry.Name, layerDir) + output.Infof("Exporting dataset %s to %s", datasetEntry.Name, layerDir) datasetIdx += 1 } if err := exportLayer(ctx, store, layerDesc, layerDir, options.overwrite); err != nil { return err } } + output.Debugf("Exported %d code layers", codeIdx) + output.Debugf("Exported %d dataset layers", datasetIdx) return nil } @@ -92,7 +95,7 @@ func exportConfig(config *artifact.KitFile, exportDir string, overwrite bool) er return fmt.Errorf("failed to export config: %w", err) } - fmt.Printf("Exporting config to %s\n", configPath) + output.Infof("Exporting config to %s", configPath) if err := os.WriteFile(configPath, configBytes, 0644); err != nil { return fmt.Errorf("failed to write config file: %w", err) } @@ -119,6 +122,7 @@ func exportLayer(ctx context.Context, store content.Storage, desc ocispec.Descri } else if !fi.IsDir() { return fmt.Errorf("failed to export: path %s exists and is not a directory", exportDir) } + output.Debugf("Directory %s already exists", exportDir) } if err := os.MkdirAll(exportDir, 0755); err != nil { return fmt.Errorf("failed to create directory %s: %w", exportDir, err) @@ -137,7 +141,6 @@ func extractTar(tr *tar.Reader, dir string, overwrite bool) error { return err } outPath := path.Join(dir, header.Name) - fmt.Printf("Extracting %s\n", outPath) switch header.Typeflag { case tar.TypeDir: @@ -148,8 +151,9 @@ func extractTar(tr *tar.Reader, dir string, overwrite bool) error { if !fi.IsDir() { return fmt.Errorf("path '%s' already exists and is not a directory", outPath) } + output.Debugf("Path %s already exists", outPath) } - fmt.Printf("Creating directory %s\n", outPath) + output.Debugf("Creating directory %s", outPath) if err := os.MkdirAll(outPath, header.FileInfo().Mode()); err != nil { return fmt.Errorf("failed to create directory %s: %w", outPath, err) } @@ -163,7 +167,7 @@ func extractTar(tr *tar.Reader, dir string, overwrite bool) error { return fmt.Errorf("path '%s' already exists and is not a regular file", outPath) } } - fmt.Printf("Extracting file %s\n", outPath) + output.Debugf("Extracting file %s", outPath) file, err := os.OpenFile(outPath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, header.FileInfo().Mode()) if err != nil { return fmt.Errorf("failed to create file %s: %w", outPath, err) diff --git a/pkg/cmd/list/cmd.go b/pkg/cmd/list/cmd.go index 6f0b66e1..33d09a3a 100644 --- a/pkg/cmd/list/cmd.go +++ b/pkg/cmd/list/cmd.go @@ -5,6 +5,7 @@ import ( "fmt" "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" + "kitops/pkg/output" "os" "path" "text/tabwriter" @@ -70,29 +71,25 @@ func runCommand(flags *listFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { opts := &listOptions{} if err := opts.complete(cmd.Context(), flags, args); err != nil { - fmt.Printf("Failed to parse argument: %s", err) - return + output.Fatalf("Failed to parse argument: %s", err) } var allInfoLines []string if opts.remoteRef == nil { lines, err := listLocalKits(opts.storageHome) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } allInfoLines = lines } else { lines, err := listRemoteKits(cmd.Context(), opts.remoteRef, opts.usehttp) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } allInfoLines = lines } printSummary(allInfoLines) - } } @@ -104,3 +101,10 @@ func printSummary(lines []string) { } tw.Flush() } + +func printConfig(opts *listOptions) { + output.Debugf("Using storage path: %s", opts.storageHome) + if opts.remoteRef != nil { + output.Debugf("Listing remote model kits in %s", opts.remoteRef.String()) + } +} diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 26334e92..8f62dbdf 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -4,7 +4,7 @@ Copyright © 2024 Jozu.com package login import ( - "fmt" + "kitops/pkg/output" "github.com/spf13/cobra" ) @@ -20,7 +20,7 @@ func LoginCommand() *cobra.Command { This application is a tool to generate the needed files to quickly create a Cobra application.`, Run: func(cmd *cobra.Command, args []string) { - fmt.Println("login called") + output.Infoln("login called") }, } diff --git a/pkg/cmd/pull/cmd.go b/pkg/cmd/pull/cmd.go index 35b73430..d8bcd1c4 100644 --- a/pkg/cmd/pull/cmd.go +++ b/pkg/cmd/pull/cmd.go @@ -5,6 +5,7 @@ import ( "fmt" "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" + "kitops/pkg/output" "github.com/spf13/cobra" "oras.land/oras-go/v2/content/oci" @@ -48,6 +49,8 @@ func (opts *pullOptions) complete(ctx context.Context, flags *pullFlags, args [] } opts.modelRef = modelRef opts.usehttp = flags.useHTTP + + printConfig(opts) return nil } @@ -70,12 +73,11 @@ func runCommand(flags *pullFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { opts := &pullOptions{} if err := opts.complete(cmd.Context(), flags, args); err != nil { - fmt.Printf("Failed to process arguments: %s", err) + output.Fatalf("Failed to process arguments: %s", err) } remoteRegistry, err := remote.NewRegistry(opts.modelRef.Registry) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } if opts.usehttp { remoteRegistry.PlainHTTP = true @@ -84,16 +86,19 @@ func runCommand(flags *pullFlags) func(*cobra.Command, []string) { localStorePath := storage.LocalStorePath(opts.storageHome, opts.modelRef) localStore, err := oci.New(localStorePath) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } - fmt.Printf("Pulling %s\n", opts.modelRef.String()) + output.Infof("Pulling %s", opts.modelRef.String()) desc, err := pullModel(cmd.Context(), remoteRegistry, localStore, opts.modelRef) if err != nil { - fmt.Printf("Failed to pull: %s\n", err) + output.Fatalf("Failed to pull: %s", err) return } - fmt.Printf("Pulled %s\n", desc.Digest) + output.Infof("Pulled %s", desc.Digest) } } + +func printConfig(opts *pullOptions) { + output.Debugf("Using storage path: %s", opts.storageHome) +} diff --git a/pkg/cmd/push/cmd.go b/pkg/cmd/push/cmd.go index f10ba7ae..fd73b3e4 100644 --- a/pkg/cmd/push/cmd.go +++ b/pkg/cmd/push/cmd.go @@ -5,6 +5,7 @@ import ( "fmt" "kitops/pkg/lib/constants" "kitops/pkg/lib/storage" + "kitops/pkg/output" "github.com/spf13/cobra" "oras.land/oras-go/v2/content/oci" @@ -47,6 +48,8 @@ func (opts *pushOptions) complete(ctx context.Context, flags *pushFlags, args [] } opts.modelRef = modelRef opts.usehttp = flags.UseHTTP + + printConfig(opts) return nil } @@ -69,13 +72,12 @@ func runCommand(flags *pushFlags) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { opts := &pushOptions{} if err := opts.complete(cmd.Context(), flags, args); err != nil { - fmt.Printf("Failed to process arguments: %s", err) + output.Fatalf("Failed to process arguments: %s", err) } remoteRegistry, err := remote.NewRegistry(opts.modelRef.Registry) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } if opts.usehttp { remoteRegistry.PlainHTTP = true @@ -84,16 +86,19 @@ func runCommand(flags *pushFlags) func(*cobra.Command, []string) { localStorePath := storage.LocalStorePath(opts.storageHome, opts.modelRef) localStore, err := oci.New(localStorePath) if err != nil { - fmt.Println(err) - return + output.Fatalln(err) } - fmt.Printf("Pushing %s\n", opts.modelRef.String()) + output.Infof("Pushing %s\n", opts.modelRef.String()) desc, err := PushModel(cmd.Context(), localStore, remoteRegistry, opts.modelRef) if err != nil { - fmt.Printf("Failed to push: %s\n", err) + output.Fatalf("Failed to push: %s\n", err) return } - fmt.Printf("Pushed %s\n", desc.Digest) + output.Infof("Pushed %s\n", desc.Digest) } } + +func printConfig(opts *pushOptions) { + output.Debugf("Using storage path: %s", opts.storageHome) +} diff --git a/pkg/cmd/version/version.go b/pkg/cmd/version/version.go index a0d2641a..abd64d87 100644 --- a/pkg/cmd/version/version.go +++ b/pkg/cmd/version/version.go @@ -1,7 +1,7 @@ package version import ( - "fmt" + "kitops/pkg/output" "runtime" "github.com/spf13/cobra" @@ -26,7 +26,7 @@ including the current version of the tool, the Git commit that the version was b the build time, and the version of Go it was compiled with. This can be useful for debugging or verifying that you are running the expected version of kit.`, Run: func(cmd *cobra.Command, args []string) { - fmt.Printf("Version: %s\nCommit: %s\nBuilt: %s\nGo version: %s\n", Version, GitCommit, BuildTime, GoVersion) + output.Infof("Version: %s\nCommit: %s\nBuilt: %s\nGo version: %s\n", Version, GitCommit, BuildTime, GoVersion) }, } return cmd diff --git a/pkg/lib/storage/local.go b/pkg/lib/storage/local.go index 9a020413..3bb2d347 100644 --- a/pkg/lib/storage/local.go +++ b/pkg/lib/storage/local.go @@ -7,6 +7,7 @@ import ( "fmt" "kitops/pkg/artifact" "kitops/pkg/lib/constants" + "kitops/pkg/output" "os" "path/filepath" @@ -119,14 +120,14 @@ func (store *LocalStore) saveContentLayer(layer *artifact.ModelLayer) (ocispec.D return ocispec.DescriptorEmptyJSON, err } if exists { - fmt.Println("Model layer already saved: ", desc.Digest) + output.Infof("Model layer already saved: %s", desc.Digest) } else { // Does not exist in storage, need to push err = store.storage.Push(ctx, desc, buf) if err != nil { return ocispec.DescriptorEmptyJSON, err } - fmt.Println("Saved model layer: ", desc.Digest) + output.Infof("Saved model layer: %s", desc.Digest) } return desc, nil @@ -154,6 +155,9 @@ func (store *LocalStore) saveConfigFile(model *artifact.KitFile) (ocispec.Descri if err != nil { return ocispec.DescriptorEmptyJSON, err } + output.Infof("Saved configuration: %s", desc.Digest) + } else { + output.Infof("Configuration already exists in storage: %s", desc.Digest) } return desc, nil @@ -168,9 +172,7 @@ func (store *LocalStore) saveModelManifest(layerDescs []ocispec.Descriptor, conf Annotations: map[string]string{}, } // Add the layers to the manifest - for _, layerDesc := range layerDescs { - manifest.Layers = append(manifest.Layers, layerDesc) - } + manifest.Layers = append(manifest.Layers, layerDescs...) manifestBytes, err := json.Marshal(manifest) if err != nil { @@ -191,6 +193,9 @@ func (store *LocalStore) saveModelManifest(layerDescs []ocispec.Descriptor, conf if err != nil { return nil, err } + output.Infof("Saved manifest to storage: %s", desc.Digest) + } else { + output.Infof("Manifest already exists in storage: %s", desc.Digest) } if tag != "" { @@ -200,6 +205,7 @@ func (store *LocalStore) saveModelManifest(layerDescs []ocispec.Descriptor, conf if err := store.storage.Tag(context.Background(), desc, tag); err != nil { return nil, fmt.Errorf("failed to tag manifest: %w", err) } + output.Debugf("Added tag to manifest: %s", tag) } return &desc, nil diff --git a/pkg/output/logging.go b/pkg/output/logging.go new file mode 100644 index 00000000..23ead23a --- /dev/null +++ b/pkg/output/logging.go @@ -0,0 +1,66 @@ +package output + +import ( + "fmt" + "os" + "strings" +) + +var ( + printDebug = false +) + +func SetDebug(debug bool) { + printDebug = debug +} + +func Infoln(s any) { + fmt.Println(s) +} + +func Infof(s string, args ...any) { + // Avoid printing incomplete lines + if !strings.HasSuffix(s, "\n") { + s = s + "\n" + } + fmt.Printf(s, args...) +} + +func Errorln(s any) { + fmt.Fprintln(os.Stderr, s) +} + +func Errorf(s string, args ...any) { + // Avoid printing incomplete lines + if !strings.HasSuffix(s, "\n") { + s = s + "\n" + } + fmt.Fprintf(os.Stderr, s, args...) +} + +func Fatalln(s any) { + Errorln(s) + os.Exit(1) +} + +func Fatalf(s string, args ...any) { + Errorf(s, args...) + os.Exit(1) +} + +func Debugln(s any) { + if printDebug { + fmt.Println(s) + } +} + +func Debugf(s string, args ...any) { + if !printDebug { + return + } + // Avoid printing incomplete lines + if !strings.HasSuffix(s, "\n") { + s = s + "\n" + } + fmt.Printf(s, args...) +} From 635ceb0cd401f324dd91aa8d0f1dd7523c3111d2 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 23 Feb 2024 11:42:54 -0500 Subject: [PATCH 3/3] Propagate command context instead of using context.Background() --- pkg/cmd/build/build.go | 7 ++++--- pkg/cmd/build/cmd.go | 2 +- pkg/cmd/build/cmd_test.go | 2 +- pkg/cmd/list/cmd.go | 2 +- pkg/cmd/list/list.go | 18 +++++++++--------- pkg/cmd/list/list_test.go | 3 ++- pkg/lib/storage/local.go | 24 ++++++++++-------------- pkg/lib/storage/store.go | 4 ++-- pkg/lib/testing/testing.go | 4 ++-- 9 files changed, 32 insertions(+), 34 deletions(-) diff --git a/pkg/cmd/build/build.go b/pkg/cmd/build/build.go index 8dc43c4e..29827137 100644 --- a/pkg/cmd/build/build.go +++ b/pkg/cmd/build/build.go @@ -1,6 +1,7 @@ package build import ( + "context" "kitops/pkg/artifact" "kitops/pkg/lib/constants" "kitops/pkg/lib/filesystem" @@ -10,7 +11,7 @@ import ( "path" ) -func RunBuild(options *buildOptions) error { +func RunBuild(ctx context.Context, options *buildOptions) error { // 1. Read the model file modelfile, err := os.Open(options.modelFile) if err != nil { @@ -70,13 +71,13 @@ func RunBuild(options *buildOptions) error { tag = options.modelRef.Reference } store := storage.NewLocalStore(modelStorePath, repo) - manifestDesc, err := store.SaveModel(model, tag) + manifestDesc, err := store.SaveModel(ctx, model, tag) if err != nil { return err } for _, tag := range options.extraRefs { - if err := store.TagModel(*manifestDesc, tag); err != nil { + if err := store.TagModel(ctx, *manifestDesc, tag); err != nil { return err } } diff --git a/pkg/cmd/build/cmd.go b/pkg/cmd/build/cmd.go index ba68b44d..516664ed 100644 --- a/pkg/cmd/build/cmd.go +++ b/pkg/cmd/build/cmd.go @@ -60,7 +60,7 @@ func runCommand(flags *buildFlags) func(cmd *cobra.Command, args []string) { output.Fatalf("Failed to process configuration: %s", err) return } - err = RunBuild(opts) + err = RunBuild(cmd.Context(), opts) if err != nil { output.Fatalf("Failed to build model kit: %s", err) return diff --git a/pkg/cmd/build/cmd_test.go b/pkg/cmd/build/cmd_test.go index 877cae29..db510025 100644 --- a/pkg/cmd/build/cmd_test.go +++ b/pkg/cmd/build/cmd_test.go @@ -38,7 +38,7 @@ func TestBuildOptions_RunBuild(t *testing.T) { contextDir: "/path/to/context", } - err := RunBuild(options) + err := RunBuild(context.Background(), options) assert.NoError(t, err) } diff --git a/pkg/cmd/list/cmd.go b/pkg/cmd/list/cmd.go index 33d09a3a..7c88b9e1 100644 --- a/pkg/cmd/list/cmd.go +++ b/pkg/cmd/list/cmd.go @@ -76,7 +76,7 @@ func runCommand(flags *listFlags) func(*cobra.Command, []string) { var allInfoLines []string if opts.remoteRef == nil { - lines, err := listLocalKits(opts.storageHome) + lines, err := listLocalKits(cmd.Context(), opts.storageHome) if err != nil { output.Fatalln(err) } diff --git a/pkg/cmd/list/list.go b/pkg/cmd/list/list.go index 7db9d76a..77a8de38 100644 --- a/pkg/cmd/list/list.go +++ b/pkg/cmd/list/list.go @@ -24,7 +24,7 @@ const ( listTableFmt = "%s\t%s\t%s\t%s\t%s\t%s\t" ) -func listLocalKits(storageRoot string) ([]string, error) { +func listLocalKits(ctx context.Context, storageRoot string) ([]string, error) { storeDirs, err := findRepos(storageRoot) if err != nil { return nil, err @@ -34,7 +34,7 @@ func listLocalKits(storageRoot string) ([]string, error) { for _, storeDir := range storeDirs { store := storage.NewLocalStore(storageRoot, storeDir) - infolines, err := listKits(store) + infolines, err := listKits(ctx, store) if err != nil { return nil, err } @@ -43,7 +43,7 @@ func listLocalKits(storageRoot string) ([]string, error) { return allInfoLines, nil } -func listKits(store storage.Store) ([]string, error) { +func listKits(ctx context.Context, store storage.Store) ([]string, error) { index, err := store.ParseIndexJson() if err != nil { return nil, err @@ -51,14 +51,14 @@ func listKits(store storage.Store) ([]string, error) { var infolines []string for _, manifestDesc := range index.Manifests { - manifest, err := getManifest(store, manifestDesc) + manifest, err := getManifest(ctx, store, manifestDesc) if err != nil { return nil, err } if manifest.Config.MediaType != constants.ModelConfigMediaType { continue } - manifestConf, err := readManifestConfig(store, manifest) + manifestConf, err := readManifestConfig(ctx, store, manifest) if err != nil { return nil, err } @@ -69,8 +69,8 @@ func listKits(store storage.Store) ([]string, error) { return infolines, nil } -func getManifest(store storage.Store, manifestDesc ocispec.Descriptor) (*ocispec.Manifest, error) { - manifestBytes, err := store.Fetch(context.Background(), manifestDesc) +func getManifest(ctx context.Context, store storage.Store, manifestDesc ocispec.Descriptor) (*ocispec.Manifest, error) { + manifestBytes, err := store.Fetch(ctx, manifestDesc) if err != nil { return nil, fmt.Errorf("failed to read manifest %s: %w", manifestDesc.Digest, err) } @@ -81,8 +81,8 @@ func getManifest(store storage.Store, manifestDesc ocispec.Descriptor) (*ocispec return manifest, nil } -func readManifestConfig(store storage.Store, manifest *ocispec.Manifest) (*artifact.KitFile, error) { - configBytes, err := store.Fetch(context.Background(), manifest.Config) +func readManifestConfig(ctx context.Context, store storage.Store, manifest *ocispec.Manifest) (*artifact.KitFile, error) { + configBytes, err := store.Fetch(ctx, manifest.Config) if err != nil { return nil, fmt.Errorf("failed to read config: %w", err) } diff --git a/pkg/cmd/list/list_test.go b/pkg/cmd/list/list_test.go index 7acc331b..01abe00e 100644 --- a/pkg/cmd/list/list_test.go +++ b/pkg/cmd/list/list_test.go @@ -1,6 +1,7 @@ package list import ( + "context" "fmt" "kitops/pkg/artifact" "kitops/pkg/lib/constants" @@ -155,7 +156,7 @@ func TestListKits(t *testing.T) { Index: tt.index, Repo: tt.repo, } - summaryLines, err := listKits(testStore) + summaryLines, err := listKits(context.Background(), testStore) if tt.expectErrRegexp != "" { // Should be error assert.Empty(t, summaryLines, "Should not output summary on error") diff --git a/pkg/lib/storage/local.go b/pkg/lib/storage/local.go index 3bb2d347..d38cf8c1 100644 --- a/pkg/lib/storage/local.go +++ b/pkg/lib/storage/local.go @@ -43,33 +43,33 @@ func NewLocalStore(storeRoot, repo string) Store { } } -func (store *LocalStore) SaveModel(model *artifact.Model, tag string) (*ocispec.Descriptor, error) { - configDesc, err := store.saveConfigFile(model.Config) +func (store *LocalStore) SaveModel(ctx context.Context, model *artifact.Model, tag string) (*ocispec.Descriptor, error) { + configDesc, err := store.saveConfigFile(ctx, model.Config) if err != nil { return nil, err } var layerDescs []ocispec.Descriptor for _, layer := range model.Layers { - layerDesc, err := store.saveContentLayer(&layer) + layerDesc, err := store.saveContentLayer(ctx, &layer) if err != nil { return nil, err } layerDescs = append(layerDescs, layerDesc) } - manifestDesc, err := store.saveModelManifest(layerDescs, configDesc, tag) + manifestDesc, err := store.saveModelManifest(ctx, layerDescs, configDesc, tag) if err != nil { return nil, err } return manifestDesc, nil } -func (store *LocalStore) TagModel(manifestDesc ocispec.Descriptor, tag string) error { +func (store *LocalStore) TagModel(ctx context.Context, manifestDesc ocispec.Descriptor, tag string) error { if err := validateTag(tag); err != nil { return err } - if err := store.storage.Tag(context.Background(), manifestDesc, tag); err != nil { + if err := store.storage.Tag(ctx, manifestDesc, tag); err != nil { return fmt.Errorf("failed to tag manifest: %w", err) } @@ -99,9 +99,7 @@ func (store *LocalStore) GetRepository() string { return store.repo } -func (store *LocalStore) saveContentLayer(layer *artifact.ModelLayer) (ocispec.Descriptor, error) { - ctx := context.Background() - +func (store *LocalStore) saveContentLayer(ctx context.Context, layer *artifact.ModelLayer) (ocispec.Descriptor, error) { buf := &bytes.Buffer{} err := layer.Apply(buf) if err != nil { @@ -133,8 +131,7 @@ func (store *LocalStore) saveContentLayer(layer *artifact.ModelLayer) (ocispec.D return desc, nil } -func (store *LocalStore) saveConfigFile(model *artifact.KitFile) (ocispec.Descriptor, error) { - ctx := context.Background() +func (store *LocalStore) saveConfigFile(ctx context.Context, model *artifact.KitFile) (ocispec.Descriptor, error) { modelBytes, err := model.MarshalToJSON() if err != nil { return ocispec.DescriptorEmptyJSON, err @@ -163,8 +160,7 @@ func (store *LocalStore) saveConfigFile(model *artifact.KitFile) (ocispec.Descri return desc, nil } -func (store *LocalStore) saveModelManifest(layerDescs []ocispec.Descriptor, config ocispec.Descriptor, tag string) (*ocispec.Descriptor, error) { - ctx := context.Background() +func (store *LocalStore) saveModelManifest(ctx context.Context, layerDescs []ocispec.Descriptor, config ocispec.Descriptor, tag string) (*ocispec.Descriptor, error) { manifest := ocispec.Manifest{ Versioned: specs.Versioned{SchemaVersion: 2}, Config: config, @@ -202,7 +198,7 @@ func (store *LocalStore) saveModelManifest(layerDescs []ocispec.Descriptor, conf if err := validateTag(tag); err != nil { return nil, err } - if err := store.storage.Tag(context.Background(), desc, tag); err != nil { + if err := store.storage.Tag(ctx, desc, tag); err != nil { return nil, fmt.Errorf("failed to tag manifest: %w", err) } output.Debugf("Added tag to manifest: %s", tag) diff --git a/pkg/lib/storage/store.go b/pkg/lib/storage/store.go index fc87ccc1..9f342b90 100644 --- a/pkg/lib/storage/store.go +++ b/pkg/lib/storage/store.go @@ -11,8 +11,8 @@ import ( ) type Store interface { - SaveModel(model *artifact.Model, tag string) (*ocispec.Descriptor, error) - TagModel(manifestDesc ocispec.Descriptor, tag string) error + SaveModel(ctx context.Context, model *artifact.Model, tag string) (*ocispec.Descriptor, error) + TagModel(ctx context.Context, manifestDesc ocispec.Descriptor, tag string) error GetRepository() string ParseIndexJson() (*ocispec.Index, error) Fetch(context.Context, ocispec.Descriptor) ([]byte, error) diff --git a/pkg/lib/testing/testing.go b/pkg/lib/testing/testing.go index ce095bf1..fd58477a 100644 --- a/pkg/lib/testing/testing.go +++ b/pkg/lib/testing/testing.go @@ -71,12 +71,12 @@ func (s *TestStore) ParseIndexJson() (*ocispec.Index, error) { return nil, TestingNotFoundError } -func (*TestStore) TagModel(ocispec.Descriptor, string) error { +func (*TestStore) TagModel(context.Context, ocispec.Descriptor, string) error { return fmt.Errorf("tag model is not implemented for testing") } // SaveModel is not yet implemented! -func (*TestStore) SaveModel(*artifact.Model, string) (*ocispec.Descriptor, error) { +func (*TestStore) SaveModel(context.Context, *artifact.Model, string) (*ocispec.Descriptor, error) { return nil, fmt.Errorf("save model is not implemented for testing") }