diff --git a/core/container/externalbuilders/externalbuilders.go b/core/container/externalbuilders/externalbuilders.go index 83e28e518bc..c7082282eee 100644 --- a/core/container/externalbuilders/externalbuilders.go +++ b/core/container/externalbuilders/externalbuilders.go @@ -15,6 +15,7 @@ import ( "os/exec" "path/filepath" "regexp" + "time" "github.com/hyperledger/fabric/common/flogging" "github.com/hyperledger/fabric/core/chaincode/persistence" @@ -75,9 +76,10 @@ func (d *Detector) CachedBuild(ccid string) (*Instance, error) { for _, builder := range d.Builders { if builder.Name == buildInfo.BuilderName { return &Instance{ - PackageID: ccid, - Builder: builder, - BldDir: filepath.Join(durablePath, "bld"), + PackageID: ccid, + Builder: builder, + BldDir: filepath.Join(durablePath, "bld"), + TermTimeout: 5 * time.Second, }, nil } } @@ -157,9 +159,10 @@ func (d *Detector) Build(ccid string, md *persistence.ChaincodePackageMetadata, } return &Instance{ - PackageID: ccid, - Builder: builder, - BldDir: durableBldDir, + PackageID: ccid, + Builder: builder, + BldDir: durableBldDir, + TermTimeout: 5 * time.Second, }, nil } diff --git a/core/container/externalbuilders/instance.go b/core/container/externalbuilders/instance.go index 9309a2acd2e..1bf947fbfb1 100644 --- a/core/container/externalbuilders/instance.go +++ b/core/container/externalbuilders/instance.go @@ -8,16 +8,19 @@ package externalbuilders import ( "os/exec" + "syscall" + "time" "github.com/hyperledger/fabric/core/container/ccintf" "github.com/pkg/errors" ) type Instance struct { - PackageID string - BldDir string - Builder *Builder - Session *Session + PackageID string + BldDir string + Builder *Builder + Session *Session + TermTimeout time.Duration } func (i *Instance) Start(peerConnection *ccintf.PeerConnection) error { @@ -29,14 +32,37 @@ func (i *Instance) Start(peerConnection *ccintf.PeerConnection) error { return nil } +// Stop signals the process to terminate with SIGTERM. If the process doesn't +// terminate within TermTimeout, the process is killed with SIGKILL. func (i *Instance) Stop() error { - return errors.Errorf("stop is not implemented for external builders yet") + if i.Session == nil { + return errors.Errorf("instance has not been started") + } + + done := make(chan struct{}) + go func() { i.Wait(); close(done) }() + + i.Session.Signal(syscall.SIGTERM) + select { + case <-time.After(i.TermTimeout): + i.Session.Signal(syscall.SIGKILL) + case <-done: + return nil + } + + select { + case <-time.After(5 * time.Second): + return errors.Errorf("failed to stop instance '%s'", i.PackageID) + case <-done: + return nil + } } func (i *Instance) Wait() (int, error) { if i.Session == nil { - return 0, errors.Errorf("instance was not successfully started") + return -1, errors.Errorf("instance was not successfully started") } + err := i.Session.Wait() err = errors.Wrapf(err, "builder '%s' run failed", i.Builder.Name) if exitErr, ok := errors.Cause(err).(*exec.ExitError); ok { diff --git a/core/container/externalbuilders/instance_test.go b/core/container/externalbuilders/instance_test.go index ac76aa5e8f6..af3589ffb2b 100644 --- a/core/container/externalbuilders/instance_test.go +++ b/core/container/externalbuilders/instance_test.go @@ -7,6 +7,9 @@ SPDX-License-Identifier: Apache-2.0 package externalbuilders_test import ( + "os/exec" + "time" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "go.uber.org/zap" @@ -57,9 +60,47 @@ var _ = Describe("Instance", func() { }) Describe("Stop", func() { - It("statically returns an error", func() { - err := instance.Stop() - Expect(err).To(MatchError("stop is not implemented for external builders yet")) + It("terminates the process", func() { + cmd := exec.Command("sleep", "90") + sess, err := externalbuilders.Start(logger, cmd) + Expect(err).NotTo(HaveOccurred()) + instance.Session = sess + instance.TermTimeout = time.Minute + + errCh := make(chan error) + go func() { errCh <- instance.Session.Wait() }() + Consistently(errCh).ShouldNot(Receive()) + + err = instance.Stop() + Expect(err).ToNot(HaveOccurred()) + Eventually(errCh).Should(Receive(MatchError("signal: terminated"))) + }) + + Context("when the process doesn't respond to SIGTERM within TermTimeout", func() { + It("kills the process with malice", func() { + cmd := exec.Command("testdata/ignoreterm.sh") + sess, err := externalbuilders.Start(logger, cmd) + Expect(err).NotTo(HaveOccurred()) + + instance.Session = sess + instance.TermTimeout = time.Second + + errCh := make(chan error) + go func() { errCh <- instance.Session.Wait() }() + Consistently(errCh).ShouldNot(Receive()) + + err = instance.Stop() + Expect(err).ToNot(HaveOccurred()) + Eventually(errCh).Should(Receive(MatchError("signal: killed"))) + }) + }) + + Context("when the instance session has not been started", func() { + It("returns an error", func() { + instance.Session = nil + err := instance.Stop() + Expect(err).To(MatchError("instance has not been started")) + }) }) }) @@ -82,7 +123,7 @@ var _ = Describe("Instance", func() { Expect(code).To(Equal(0)) }) - When("run exits with a non-zero status", func() { + Context("when run exits with a non-zero status", func() { BeforeEach(func() { instance.Builder.Location = "testdata/failbuilder" instance.Builder.Name = "failbuilder" @@ -98,5 +139,14 @@ var _ = Describe("Instance", func() { Expect(code).To(Equal(1)) }) }) + + Context("when the instance session has not been started", func() { + It("returns an error", func() { + instance.Session = nil + exitCode, err := instance.Wait() + Expect(err).To(MatchError("instance was not successfully started")) + Expect(exitCode).To(Equal(-1)) + }) + }) }) }) diff --git a/core/container/externalbuilders/testdata/ignoreterm.sh b/core/container/externalbuilders/testdata/ignoreterm.sh new file mode 100755 index 00000000000..ad25eb7a7c9 --- /dev/null +++ b/core/container/externalbuilders/testdata/ignoreterm.sh @@ -0,0 +1,11 @@ +#!/bin/bash +# +# Copyright IBM Corp. All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +trap "echo Ignoring SIGTERM" SIGTERM + +while true; do + sleep 0.1 +done