Implement Stop on the externalbuilders.Instance

FAB-16108

Change-Id: Ibd9b684998f68b59f1da736f0f4d42c2336110d8
Signed-off-by: Matthew Sykes <[email protected]>

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
 }
 

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 {

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))
+			})
+		})
 	})
 })

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