diff --git a/go/action_kit_commons/diskfill/utils.go b/go/action_kit_commons/diskfill/utils.go index 7aab54e9..9540eced 100644 --- a/go/action_kit_commons/diskfill/utils.go +++ b/go/action_kit_commons/diskfill/utils.go @@ -40,7 +40,7 @@ func createBundle(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Op runc.RefreshNamespaces(ctx, sidecar.TargetProcess.Namespaces, specs.PIDNamespace) - if err := bundle.EditSpec(ctx, + if err := bundle.EditSpec( runc.WithHostname(containerId), runc.WithAnnotations(map[string]string{ "com.steadybit.sidecar": "true", @@ -56,7 +56,7 @@ func createBundle(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Op Options: []string{"noexec", "nosuid", "nodev", "rprivate"}, }), ); err != nil { - return nil, fmt.Errorf("failed to create config.json: %w", err) + return nil, err } success = true diff --git a/go/action_kit_commons/go.mod b/go/action_kit_commons/go.mod index cd363434..1d7a28f6 100644 --- a/go/action_kit_commons/go.mod +++ b/go/action_kit_commons/go.mod @@ -7,15 +7,16 @@ require ( github.com/opencontainers/runtime-spec v1.2.0 github.com/rs/zerolog v1.32.0 github.com/stretchr/testify v1.9.0 + golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/kr/pretty v0.3.0 // indirect + github.com/kr/pretty v0.3.1 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/rogpeppe/go-internal v1.11.0 // indirect + github.com/rogpeppe/go-internal v1.12.0 // indirect github.com/stretchr/objx v0.5.2 // indirect golang.org/x/sys v0.20.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect diff --git a/go/action_kit_commons/go.sum b/go/action_kit_commons/go.sum index aa01903c..0c5a0f11 100644 --- a/go/action_kit_commons/go.sum +++ b/go/action_kit_commons/go.sum @@ -5,10 +5,9 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8= github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= -github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -21,12 +20,13 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE7dzrbT927iTk= github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= 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/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= -github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= -github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.32.0 h1:keLypqrlIjaFsbmJOBdB/qvyF8KEtCWHwobLp5l/mQ0= github.com/rs/zerolog v1.32.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= @@ -34,17 +34,15 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 h1:vr/HnozRka3pE4EsMEg1lgkXJkTFJCVUX+S/ZT6wYzM= +golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go/action_kit_commons/network/dig_runner.go b/go/action_kit_commons/network/dig_runner.go index d7f73b89..159d54d1 100644 --- a/go/action_kit_commons/network/dig_runner.go +++ b/go/action_kit_commons/network/dig_runner.go @@ -48,7 +48,6 @@ func (r *RuncDigRunner) Run(ctx context.Context, arg []string, stdin io.Reader) runc.RefreshNamespaces(ctx, r.Sidecar.TargetProcess.Namespaces, specs.NetworkNamespace) if err = bundle.EditSpec( - ctx, runc.WithHostname(fmt.Sprintf("dig-%s", id)), runc.WithAnnotations(map[string]string{ "com.steadybit.sidecar": "true", diff --git a/go/action_kit_commons/network/list_interfaces.go b/go/action_kit_commons/network/list_interfaces.go index 7ef411d3..fac80c6e 100644 --- a/go/action_kit_commons/network/list_interfaces.go +++ b/go/action_kit_commons/network/list_interfaces.go @@ -54,15 +54,13 @@ func ListInterfaces(ctx context.Context, r runc.Runc, sidecar SidecarOpts) ([]In runc.RefreshNamespaces(ctx, sidecar.TargetProcess.Namespaces, specs.NetworkNamespace) if err = bundle.EditSpec( - ctx, runc.WithHostname(fmt.Sprintf("ip-link-show-%s", id)), runc.WithAnnotations(map[string]string{ "com.steadybit.sidecar": "true", }), runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)), runc.WithCapabilities("CAP_NET_ADMIN"), - runc.WithProcessArgs("ip", "-json", "link", "show"), - ); err != nil { + runc.WithProcessArgs("ip", "-json", "link", "show")); err != nil { return nil, err } diff --git a/go/action_kit_commons/network/network.go b/go/action_kit_commons/network/network.go index fb159a0f..e1a05947 100644 --- a/go/action_kit_commons/network/network.go +++ b/go/action_kit_commons/network/network.go @@ -156,7 +156,6 @@ func executeIpCommands(ctx context.Context, r runc.Runc, sidecar SidecarOpts, fa processArgs := []string{"ip", "-family", string(family), "-force", "-batch", "-"} if err = bundle.EditSpec( - ctx, runc.WithHostname(fmt.Sprintf("ip-%s", id)), runc.WithAnnotations(map[string]string{"com.steadybit.sidecar": "true"}), runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)), @@ -212,7 +211,6 @@ func executeTcCommands(ctx context.Context, r runc.Runc, sidecar SidecarOpts, cm processArgs := []string{"tc", "-force", "-batch", "-"} if err = bundle.EditSpec( - ctx, runc.WithHostname(fmt.Sprintf("tc-%s", id)), runc.WithAnnotations(map[string]string{"com.steadybit.sidecar": "true"}), runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)), diff --git a/go/action_kit_commons/network/network_test.go b/go/action_kit_commons/network/network_test.go index a4b32544..4063e1f8 100644 --- a/go/action_kit_commons/network/network_test.go +++ b/go/action_kit_commons/network/network_test.go @@ -100,8 +100,8 @@ type MockBundle struct { id string } -func (m *MockBundle) EditSpec(ctx context.Context, editors ...runc.SpecEditor) error { - args := m.Called(ctx, editors) +func (m *MockBundle) EditSpec(editors ...runc.SpecEditor) error { + args := m.Called(editors) return args.Error(0) } diff --git a/go/action_kit_commons/runc/runc.go b/go/action_kit_commons/runc/runc.go index 9834b453..26fdadec 100644 --- a/go/action_kit_commons/runc/runc.go +++ b/go/action_kit_commons/runc/runc.go @@ -4,6 +4,7 @@ package runc import ( + "bufio" "bytes" "context" "encoding/json" @@ -12,12 +13,14 @@ import ( "github.com/kelseyhightower/envconfig" "github.com/opencontainers/runtime-spec/specs-go" "github.com/rs/zerolog/log" + "golang.org/x/exp/slices" "io" "os" "os/exec" "path/filepath" "runtime/trace" "strconv" + "strings" "syscall" "time" ) @@ -32,7 +35,7 @@ type Runc interface { } type ContainerBundle interface { - EditSpec(ctx context.Context, editors ...SpecEditor) error + EditSpec(editors ...SpecEditor) error MountFromProcess(ctx context.Context, fromPid int, fromPath, mountpoint string) error CopyFileFromProcess(ctx context.Context, pid int, fromPath, toPath string) error Path() string @@ -76,6 +79,9 @@ var ( ) func NewRunc(cfg Config) Runc { + if errors.Is(checkForCgroup2Nsdelegate(), ErrCgroup2NsdelegateOptionUsed) { + log.Warn().Err(ErrCgroup2NsdelegateOptionUsed).Msg("cgroup2 mount option nsdelegate is set. This may lead to unexpected errors.") + } return &defaultRunc{cfg: cfg} } @@ -233,9 +239,8 @@ func (r *defaultRunc) Kill(ctx context.Context, id string, signal syscall.Signal type SpecEditor func(spec *specs.Spec) -func (b *containerBundle) EditSpec(ctx context.Context, editors ...SpecEditor) error { - defer trace.StartRegion(ctx, "runc.EditSpec").End() - spec, err := readSpec(filepath.Join(b.path, "config.json")) +func (b *containerBundle) EditSpec(editors ...SpecEditor) error { + spec, err := b.readSpec() if err != nil { return err } @@ -245,23 +250,18 @@ func (b *containerBundle) EditSpec(ctx context.Context, editors ...SpecEditor) e for _, fn := range editors { fn(spec) } - err = writeSpec(filepath.Join(b.path, "config.json"), spec) - log.Trace().Str("bundle", b.path).Interface("createSpec", spec).Msg("written runc createSpec") - return err -} -func (r *defaultRunc) createSpec(ctx context.Context, bundle string) error { - defer trace.StartRegion(ctx, "runc.Spec").End() - log.Trace().Str("bundle", bundle).Msg("creating container createSpec") - output, err := r.command(ctx, "spec", "--bundle", bundle).CombinedOutput() - if err != nil { - return fmt.Errorf("%w: %s", err, output) + if err := checkForCgroup2NsdelegateConflict(spec); err != nil { + return err } - return nil + + err = b.writeSpec(spec) + log.Trace().Str("bundle", b.path).Interface("createSpec", spec).Msg("written runc createSpec") + return err } -func readSpec(file string) (*specs.Spec, error) { - content, err := os.ReadFile(file) +func (b *containerBundle) readSpec() (*specs.Spec, error) { + content, err := os.ReadFile(filepath.Join(b.path, "config.json")) if err != nil { return nil, err } @@ -274,13 +274,22 @@ func readSpec(file string) (*specs.Spec, error) { return &spec, nil } - -func writeSpec(file string, spec *specs.Spec) error { +func (b *containerBundle) writeSpec(spec *specs.Spec) error { content, err := json.MarshalIndent(spec, "", "\t") if err != nil { return err } - return os.WriteFile(file, content, 0644) + return os.WriteFile(filepath.Join(b.path, "config.json"), content, 0644) +} + +func (r *defaultRunc) createSpec(ctx context.Context, bundle string) error { + defer trace.StartRegion(ctx, "runc.Spec").End() + log.Trace().Str("bundle", bundle).Msg("creating container createSpec") + output, err := r.command(ctx, "spec", "--bundle", bundle).CombinedOutput() + if err != nil { + return fmt.Errorf("%w: %s", err, output) + } + return nil } func (r *defaultRunc) command(ctx context.Context, args ...string) *exec.Cmd { @@ -428,3 +437,53 @@ func WithNamespace(ns LinuxNamespace) SpecEditor { spec.Linux.Namespaces = append(spec.Linux.Namespaces, ns) } } + +var getMountOptions = defaultMountOptions + +var ErrCgroup2NsdelegateOptionUsed = errors.New("cgroup2 nsdelegate mount option conflicts with cgroup path in spec") + +func checkForCgroup2NsdelegateConflict(spec *specs.Spec) error { + if spec == nil || spec.Linux == nil || spec.Linux.CgroupsPath == "" { + return nil + } + + return checkForCgroup2Nsdelegate() +} + +func checkForCgroup2Nsdelegate() error { + opts, err := getMountOptions("/sys/fs/cgroup", "cgroup2") + if err != nil { + return err + } + + if slices.Contains(opts, "nsdelegate") { + return ErrCgroup2NsdelegateOptionUsed + } + return nil +} + +func defaultMountOptions(file, vfstype string) ([]string, error) { + f, err := os.Open("/proc/mounts") + if err != nil { + return nil, err + } + defer func() { _ = f.Close() }() + + return getMountOptionsFromReader(f, file, vfstype), nil +} + +func getMountOptionsFromReader(r io.Reader, file string, vfstype string) []string { + scanner := bufio.NewScanner(r) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + if len(fields) >= 4 { + fsFile := fields[1] + fsVfstype := fields[2] + fsMntops := fields[3] + if fsFile == file && fsVfstype == vfstype { + return strings.Split(fsMntops, ",") + } + } + } + return nil +} diff --git a/go/action_kit_commons/runc/runc_test.go b/go/action_kit_commons/runc/runc_test.go index a497ec5b..36ac9047 100644 --- a/go/action_kit_commons/runc/runc_test.go +++ b/go/action_kit_commons/runc/runc_test.go @@ -1,7 +1,11 @@ package runc import ( + "fmt" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" "reflect" + "strings" "testing" "time" ) @@ -68,3 +72,70 @@ func Test_unmarshalGuarded(t *testing.T) { }) } } + +func Test_getMountOptionsFromReader(t *testing.T) { + r := strings.NewReader(` +/dev/vda1 /etc/hosts ext4 rw,relatime 0 0 +cgroup2 /sys/fs/cgroup cgroup2 rw,nosuid,nodev,noexec,relatime 0 0 +tmpfs /run/docker.sock tmpfs rw,nosuid,nodev,size=762488k,nr_inodes=819200,mode=755 0 0 +`) + opts := getMountOptionsFromReader(r, "/sys/fs/cgroup", "cgroup2") + assert.Equal(t, []string{"rw", "nosuid", "nodev", "noexec", "relatime"}, opts) + + opts = getMountOptionsFromReader(r, "/not/in/there", "cgroup2") + assert.Empty(t, opts) +} + +func TestCheckForCgroup2Nsdelegate(t *testing.T) { + specWithCgroupPath := specs.Spec{ + Linux: &specs.Linux{ + CgroupsPath: "/some", + }, + } + specDefault := specs.Spec{} + mountOptsDefault := []string{"rw", "nosuid", "nodev", "noexec", "relatime"} + mountOptsNsDelegate := []string{"rw", "nosuid", "nodev", "noexec", "relatime", "nsdelegate"} + + tests := []struct { + name string + spec specs.Spec + mountOpts []string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "should not error when no cgroup2 is used", + spec: specWithCgroupPath, + mountOpts: nil, + wantErr: assert.NoError, + }, + { + name: "should not error when no CGroupPath is set", + spec: specDefault, + mountOpts: mountOptsNsDelegate, + wantErr: assert.NoError, + }, + { + name: "should not error when no nsdelegate is used", + spec: specWithCgroupPath, + mountOpts: mountOptsDefault, + wantErr: assert.NoError, + }, + { + name: "should error when nsdelegate is used", + spec: specWithCgroupPath, + mountOpts: mountOptsNsDelegate, + wantErr: assert.Error, + }, + } + + defer func() { getMountOptions = defaultMountOptions }() + for _, tt := range tests { + getMountOptions = func(file, fstype string) ([]string, error) { + return tt.mountOpts, nil + } + + t.Run(tt.name, func(t *testing.T) { + tt.wantErr(t, checkForCgroup2NsdelegateConflict(&tt.spec), fmt.Sprintf("CheckForCgroup2Nsdelegate(%+v, %+v)", tt.spec, tt.mountOpts)) + }) + } +} diff --git a/go/action_kit_commons/stress/stress.go b/go/action_kit_commons/stress/stress.go index 729d82f6..f92ba093 100644 --- a/go/action_kit_commons/stress/stress.go +++ b/go/action_kit_commons/stress/stress.go @@ -100,7 +100,7 @@ func New(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Opts) (*Str runc.RefreshNamespaces(ctx, sidecar.TargetProcess.Namespaces, specs.PIDNamespace) processArgs := append([]string{"stress-ng"}, opts.Args()...) - if err := bundle.EditSpec(ctx, + if err := bundle.EditSpec( runc.WithHostname(containerId), runc.WithAnnotations(map[string]string{ "com.steadybit.sidecar": "true", @@ -116,7 +116,7 @@ func New(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Opts) (*Str Options: []string{"noexec", "nosuid", "nodev", "rprivate"}, }), ); err != nil { - return nil, fmt.Errorf("failed to create config.json: %w", err) + return nil, err } success = true diff --git a/go/action_kit_test/client/client.go b/go/action_kit_test/client/client.go index eb9c777a..8c69020a 100644 --- a/go/action_kit_test/client/client.go +++ b/go/action_kit_test/client/client.go @@ -15,6 +15,7 @@ import ( "github.com/steadybit/extension-kit/extconversion" "golang.org/x/text/cases" "golang.org/x/text/language" + "strings" "sync" "time" ) @@ -239,7 +240,7 @@ func (c *clientImpl) prepareAction(action action_kit_api.ActionDescription, targ logMessages(executionId, prepareResult.Messages) if prepareResult.Error != nil { - return nil, duration, fmt.Errorf("action failed: %v", *prepareResult.Error) + return nil, duration, toError(prepareResult.Error) } return prepareResult.State, duration, nil @@ -260,7 +261,7 @@ func (c *clientImpl) startAction(action action_kit_api.ActionDescription, execut logMessages(executionId, startResult.Messages) if startResult.Error != nil { - return state, fmt.Errorf("action failed: %v", *startResult.Error) + return state, toError(startResult.Error) } if startResult.State != nil { @@ -302,7 +303,7 @@ func (c *clientImpl) actionStatus(ctx context.Context, action action_kit_api.Act state = *statusResult.State } if statusResult.Error != nil { - return state, fmt.Errorf("action failed: %v", *statusResult.Error) + return state, toError(statusResult.Error) } log.Info().Str("actionId", action.Id).Bool("completed", statusResult.Completed).Msg("Action status") @@ -313,27 +314,44 @@ func (c *clientImpl) actionStatus(ctx context.Context, action action_kit_api.Act } } +func toError(err *action_kit_api.ActionKitError) error { + if err == nil { + return nil + } + var sb strings.Builder + if err.Status != nil { + sb.WriteString("[") + sb.WriteString(string(*err.Status)) + sb.WriteString("] ") + } + sb.WriteString(err.Title) + if err.Detail != nil { + sb.WriteString(": ") + sb.WriteString(*err.Detail) + } + return fmt.Errorf(sb.String()) +} + func (c *clientImpl) stopAction(action action_kit_api.ActionDescription, executionId uuid.UUID, state action_kit_api.ActionState, metrics func(metrics []action_kit_api.Metric), messages func(messages []action_kit_api.Message)) error { stopBody := action_kit_api.StopActionRequestBody{ ExecutionId: executionId, State: state, } var stopResult action_kit_api.StopResult - err := c.executeWithBodyAndValidate(*action.Stop, stopBody, &stopResult, "StopResult") - if err != nil { + if err := c.executeWithBodyAndValidate(*action.Stop, stopBody, &stopResult, "StopResult"); err != nil { return fmt.Errorf("failed to stop action: %w", err) } logMessages(executionId, stopResult.Messages) - if stopResult.Metrics != nil { + if metrics != nil && stopResult.Metrics != nil { metrics(*stopResult.Metrics) } - if stopResult.Messages != nil { + if messages != nil && stopResult.Messages != nil { messages(*stopResult.Messages) } if stopResult.Error != nil { - return fmt.Errorf("action failed: %v", *stopResult.Error) + return toError(stopResult.Error) } return nil } diff --git a/go/action_kit_test/e2e/extension_factory.go b/go/action_kit_test/e2e/extension_factory.go index 57daa2fb..c6467c12 100644 --- a/go/action_kit_test/e2e/extension_factory.go +++ b/go/action_kit_test/e2e/extension_factory.go @@ -26,8 +26,9 @@ type HelmExtensionFactory struct { func (h *HelmExtensionFactory) CreateImage() error { cmd := exec.Command("make", "container") cmd.Dir = ".." - cmd.Stdout = &prefixWriter{prefix: "⚒️", w: os.Stdout} - cmd.Stderr = &prefixWriter{prefix: "⚒️", w: os.Stdout} + stdout := &prefixWriter{prefix: []byte("⚒️"), w: os.Stdout} + cmd.Stdout = stdout + cmd.Stderr = stdout start := time.Now() if err := cmd.Run(); err != nil { diff --git a/go/action_kit_test/e2e/minikube.go b/go/action_kit_test/e2e/minikube.go index 90c9af5d..bbbf811b 100644 --- a/go/action_kit_test/e2e/minikube.go +++ b/go/action_kit_test/e2e/minikube.go @@ -31,6 +31,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "sync" "testing" @@ -55,8 +56,8 @@ type Minikube struct { func newMinikube(runtime Runtime, driver string) *Minikube { profile := "e2e-" + string(runtime) - stdout := prefixWriter{prefix: "🧊", w: os.Stdout} - stderr := prefixWriter{prefix: "🧊", w: os.Stderr} + stdout := prefixWriter{prefix: []byte("🧊 "), w: os.Stdout} + stderr := prefixWriter{prefix: []byte("🧊 "), w: os.Stderr} return &Minikube{ Runtime: runtime, @@ -319,21 +320,44 @@ func createKubernetesClient(context string) (*kubernetes.Clientset, *rest.Config } type prefixWriter struct { - prefix string - w io.Writer + prefix []byte + w io.Writer + notStartWithPrefix bool + m sync.Mutex } -func (w *prefixWriter) Write(p []byte) (n int, err error) { - lines := strings.Split(strings.TrimSuffix(string(p), "\n"), "\n") - count := 0 - for _, line := range lines { - c, err := fmt.Fprintf(w.w, "%s%s\n", w.prefix, line) - count += c +func (p *prefixWriter) Write(buf []byte) (n int, err error) { + p.m.Lock() + defer p.m.Unlock() + + if !p.notStartWithPrefix { + p.notStartWithPrefix = true + _, err := p.w.Write([]byte(p.prefix)) if err != nil { - return count, err + return 0, err + } + } + + remainder := buf + for { + var c int + if j := slices.Index(remainder, '\n'); j >= 0 { + c, err = p.w.Write(remainder[:j+1]) + if j+1 < len(remainder) { + _, err = p.w.Write(p.prefix) + } else { + p.notStartWithPrefix = false + } + remainder = remainder[j+1:] + } else { + c, err = p.w.Write(remainder) + remainder = nil + } + n += c + if len(remainder) == 0 || err != nil { + return } } - return len(p), nil } type ServiceClient struct { @@ -641,7 +665,7 @@ func (m *Minikube) TailLog(ctx context.Context, pod metav1.Object) { defer func() { _ = reader.Close() }() scanner := bufio.NewScanner(reader) for scanner.Scan() { - fmt.Printf("📦%s\n", scanner.Text()) + fmt.Printf("📦 %s\n", scanner.Text()) } }