FAB-9763 Fix container control inversion

The core/container package directly references the
core/container/dockercontroller and core/container/inproccontroller
packages.  Consequently, because these packages cannot refer back to the
core/container package, there is an awkward core/container/api package.
This CR improves the initialization path and tests so that this
indirection is no longer necessary.

Change-Id: I5cf8e617b09f7d399fe5d5e5f6239fdda1197bc4
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

Author: Jason Yellick

core/chaincode/chaincode_support_test.go

@@ -35,6 +35,8 @@ import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/config"
 	"github.com/hyperledger/fabric/core/container"
+	"github.com/hyperledger/fabric/core/container/dockercontroller"
+	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	"github.com/hyperledger/fabric/core/ledger"
 	"github.com/hyperledger/fabric/core/ledger/ledgermgmt"
 	cmp "github.com/hyperledger/fabric/core/mocks/peer"
@@ -177,7 +179,12 @@ func initMockPeer(chainIDs ...string) (*ChaincodeSupport, error) {
 		certGenerator,
 		&ccprovider.CCInfoFSImpl{},
 		aclmgmt.GetACLProvider(),
-		container.NewVMController(),
+		container.NewVMController(
+			map[string]container.VMProvider{
+				dockercontroller.ContainerType: dockercontroller.NewProvider(),
+				inproccontroller.ContainerType: inproccontroller.NewProvider(),
+			},
+		),
 	)
 	SideEffectInitialize(chaincodeSupport)
 	chaincodeSupport.SetSysCCProvider(sccp)

core/chaincode/container_runtime.go

@@ -18,14 +18,16 @@ import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/container"
 	"github.com/hyperledger/fabric/core/container/ccintf"
+	"github.com/hyperledger/fabric/core/container/dockercontroller"
+	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	pb "github.com/hyperledger/fabric/protos/peer"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
 // Processor processes vm and container requests.
 type Processor interface {
-	Process(ctxt context.Context, vmtype string, req container.VMCReqIntf) error
+	Process(ctxt context.Context, vmtype string, req container.VMCReq) error
 }
 
 // CertGenerator generates client certificates for chaincode.
@@ -112,9 +114,9 @@ func (c *ContainerRuntime) Stop(ctxt context.Context, cccid *ccprovider.CCContex
 
 func getVMType(cds *pb.ChaincodeDeploymentSpec) string {
 	if cds.ExecEnv == pb.ChaincodeDeploymentSpec_SYSTEM {
-		return container.SYSTEM
+		return inproccontroller.ContainerType
 	}
-	return container.DOCKER
+	return dockercontroller.ContainerType
 }
 
 const (

core/chaincode/container_runtime_test.go

@@ -16,6 +16,8 @@ import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/container"
 	"github.com/hyperledger/fabric/core/container/ccintf"
+	"github.com/hyperledger/fabric/core/container/dockercontroller"
+	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	pb "github.com/hyperledger/fabric/protos/peer"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -162,8 +164,8 @@ func TestContainerRuntimeStart(t *testing.T) {
 		execEnv pb.ChaincodeDeploymentSpec_ExecutionEnvironment
 		vmType  string
 	}{
-		{pb.ChaincodeDeploymentSpec_DOCKER, container.DOCKER},
-		{pb.ChaincodeDeploymentSpec_SYSTEM, container.SYSTEM},
+		{pb.ChaincodeDeploymentSpec_DOCKER, dockercontroller.ContainerType},
+		{pb.ChaincodeDeploymentSpec_SYSTEM, inproccontroller.ContainerType},
 	}
 
 	for _, tc := range tests {
@@ -245,8 +247,8 @@ func TestContainerRuntimeStop(t *testing.T) {
 		execEnv pb.ChaincodeDeploymentSpec_ExecutionEnvironment
 		vmType  string
 	}{
-		{pb.ChaincodeDeploymentSpec_DOCKER, container.DOCKER},
-		{pb.ChaincodeDeploymentSpec_SYSTEM, container.SYSTEM},
+		{pb.ChaincodeDeploymentSpec_DOCKER, dockercontroller.ContainerType},
+		{pb.ChaincodeDeploymentSpec_SYSTEM, inproccontroller.ContainerType},
 	}
 
 	for _, tc := range tests {

core/chaincode/exectransaction_test.go

@@ -35,6 +35,8 @@ import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/config"
 	"github.com/hyperledger/fabric/core/container"
+	"github.com/hyperledger/fabric/core/container/dockercontroller"
+	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	"github.com/hyperledger/fabric/core/ledger"
 	"github.com/hyperledger/fabric/core/ledger/ledgerconfig"
 	"github.com/hyperledger/fabric/core/ledger/ledgermgmt"
@@ -125,7 +127,12 @@ func initPeer(chainIDs ...string) (net.Listener, *ChaincodeSupport, func(), erro
 		certGenerator,
 		&ccprovider.CCInfoFSImpl{},
 		aclmgmt.GetACLProvider(),
-		container.NewVMController(),
+		container.NewVMController(
+			map[string]container.VMProvider{
+				dockercontroller.ContainerType: dockercontroller.NewProvider(),
+				inproccontroller.ContainerType: inproccontroller.NewProvider(),
+			},
+		),
 	)
 	chaincodeSupport.SetSysCCProvider(sccp)
 	SideEffectInitialize(chaincodeSupport)

core/chaincode/mock/processor.go

@@ -19,6 +19,8 @@ import (
 	"github.com/hyperledger/fabric/core/common/ccprovider"
 	"github.com/hyperledger/fabric/core/common/sysccprovider"
 	"github.com/hyperledger/fabric/core/container"
+	"github.com/hyperledger/fabric/core/container/dockercontroller"
+	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	"github.com/hyperledger/fabric/core/peer"
 	"github.com/hyperledger/fabric/core/scc"
 	pb "github.com/hyperledger/fabric/protos/peer"
@@ -132,7 +134,12 @@ func initSysCCTests() (*oldSysCCInfo, net.Listener, *ChaincodeSupport, error) {
 		certGenerator,
 		&ccprovider.CCInfoFSImpl{},
 		aclmgmt.GetACLProvider(),
-		container.NewVMController(),
+		container.NewVMController(
+			map[string]container.VMProvider{
+				dockercontroller.ContainerType: dockercontroller.NewProvider(),
+				inproccontroller.ContainerType: inproccontroller.NewProvider(),
+			},
+		),
 	)
 	pb.RegisterChaincodeSupportServer(grpcServer, chaincodeSupport)
 

core/chaincode/systemchaincode_test.go

@@ -7,7 +7,6 @@ package container_test
 
 import (
 	"github.com/hyperledger/fabric/core/container"
-	"github.com/hyperledger/fabric/core/container/api"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
@@ -21,17 +20,17 @@ type vmProvider interface {
 
 //go:generate counterfeiter -o mock/vm.go --fake-name VM . vm
 type vm interface {
-	api.VM
+	container.VM
 }
 
 //go:generate counterfeiter -o mock/vm_req.go --fake-name VMCReq . vmcReq
 type vmcReq interface {
-	container.VMCReqIntf
+	container.VMCReq
 }
 
 //go:generate counterfeiter -o mock/builder.go --fake-name Builder . builder
 type builder interface {
-	api.Builder
+	container.Builder
 }
 
 func TestContainer(t *testing.T) {

core/container/api/core.go

@@ -115,4 +115,34 @@ var _ = Describe("Container", func() {
 			})
 		})
 	})
+
+	Describe("VMController", func() {
+		var (
+			ctxt         = context.Background()
+			vmProvider   *mock.VMProvider
+			vmController *container.VMController
+			vmcReq       *mock.VMCReq
+		)
+
+		BeforeEach(func() {
+			vmProvider = &mock.VMProvider{}
+			vmController = container.NewVMController(map[string]container.VMProvider{
+				"FakeProvider": vmProvider,
+			})
+			vmProvider.NewVMReturns(&mock.VM{})
+			vmcReq = &mock.VMCReq{}
+		})
+
+		Describe("Process", func() {
+			It("Panics if there is no underlying VM provider", func() {
+				Expect(func() { vmController.Process(ctxt, "Unknown-Type", nil) }).To(Panic())
+				Expect(vmProvider.NewVMCallCount()).To(Equal(0))
+			})
+			It("Returns no error if the underlying VM provider is successful", func() {
+				err := vmController.Process(ctxt, "FakeProvider", vmcReq)
+				Expect(vmProvider.NewVMCallCount()).To(Equal(1))
+				Expect(err).NotTo(HaveOccurred())
+			})
+		})
+	})
 })

core/container/container_suite_test.go

@@ -8,21 +8,32 @@ package container
 
 import (
 	"fmt"
+	"io"
 	"sync"
 
 	"golang.org/x/net/context"
 
 	"github.com/hyperledger/fabric/common/flogging"
 	"github.com/hyperledger/fabric/core/chaincode/platforms"
-	"github.com/hyperledger/fabric/core/container/api"
 	"github.com/hyperledger/fabric/core/container/ccintf"
-	"github.com/hyperledger/fabric/core/container/dockercontroller"
-	"github.com/hyperledger/fabric/core/container/inproccontroller"
 	pb "github.com/hyperledger/fabric/protos/peer"
 )
 
 type VMProvider interface {
-	NewVM() api.VM
+	NewVM() VM
+}
+
+type Builder interface {
+	Build() (io.Reader, error)
+}
+
+//VM is an abstract virtual image for supporting arbitrary virual machines
+type VM interface {
+	Deploy(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, reader io.Reader) error
+	Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, filesToUpload map[string][]byte, builder Builder) error
+	Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error
+	Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error
+	GetVMName(ccID ccintf.CCID, format func(string) (string, error)) (string, error)
 }
 
 type refCountedLock struct {
@@ -44,23 +55,14 @@ type VMController struct {
 var vmLogger = flogging.MustGetLogger("container")
 
 // NewVMController creates a new instance of VMController
-func NewVMController() *VMController {
+func NewVMController(vmProviders map[string]VMProvider) *VMController {
 	return &VMController{
 		containerLocks: make(map[string]*refCountedLock),
-		vmProviders: map[string]VMProvider{
-			DOCKER: dockercontroller.NewProvider(),
-			SYSTEM: inproccontroller.NewProvider(),
-		},
+		vmProviders:    vmProviders,
 	}
 }
 
-//constants for supported containers
-const (
-	DOCKER = "Docker"
-	SYSTEM = "System"
-)
-
-func (vmc *VMController) newVM(typ string) api.VM {
+func (vmc *VMController) newVM(typ string) VM {
 	v, ok := vmc.vmProviders[typ]
 	if !ok {
 		vmLogger.Panicf("Programming error: unsupported VM type: %s", typ)
@@ -103,25 +105,25 @@ func (vmc *VMController) unlockContainer(id string) {
 	vmc.Unlock()
 }
 
-//VMCReqIntf - all requests should implement this interface.
+//VMCReq - all requests should implement this interface.
 //The context should be passed and tested at each layer till we stop
 //note that we'd stop on the first method on the stack that does not
 //take context
-type VMCReqIntf interface {
-	Do(ctxt context.Context, v api.VM) error
+type VMCReq interface {
+	Do(ctxt context.Context, v VM) error
 	GetCCID() ccintf.CCID
 }
 
 //StartContainerReq - properties for starting a container.
 type StartContainerReq struct {
 	ccintf.CCID
-	Builder       api.Builder
+	Builder       Builder
 	Args          []string
 	Env           []string
 	FilesToUpload map[string][]byte
 }
 
-func (si StartContainerReq) Do(ctxt context.Context, v api.VM) error {
+func (si StartContainerReq) Do(ctxt context.Context, v VM) error {
 	return v.Start(ctxt, si.CCID, si.Args, si.Env, si.FilesToUpload, si.Builder)
 }
 
@@ -139,7 +141,7 @@ type StopContainerReq struct {
 	Dontremove bool
 }
 
-func (si StopContainerReq) Do(ctxt context.Context, v api.VM) error {
+func (si StopContainerReq) Do(ctxt context.Context, v VM) error {
 	return v.Stop(ctxt, si.CCID, si.Timeout, si.Dontkill, si.Dontremove)
 }
 
@@ -155,7 +157,7 @@ func (si StopContainerReq) GetCCID() ccintf.CCID {
 //context can be cancelled. VMCProcess will try to cancel calling functions if it can
 //For instance docker clients api's such as BuildImage are not cancelable.
 //In all cases VMCProcess will wait for the called go routine to return
-func (vmc *VMController) Process(ctxt context.Context, vmtype string, req VMCReqIntf) error {
+func (vmc *VMController) Process(ctxt context.Context, vmtype string, req VMCReq) error {
 	v := vmc.newVM(vmtype)
 	if v == nil {
 		return fmt.Errorf("Unknown VM type %s", vmtype)