Skip to content

Commit

Permalink
[FAB-5329] Able to instantiate on a taken chaincode ID
Browse files Browse the repository at this point in the history 
This fix appends a hash of the Docker image name to the image name
to ensure a chaincode with different capitalization does not override
the container of an already instantiated chaincode. This problem was
introduced by the need to convert all Docker image names to lowercase.
The Docker container name will retain its capitalization as that
restriction only applies to the image name.

Change-Id: I854f80b2f0e0269d9bbc60725f82b7e5a804b6fd
Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
  • Loading branch information
@wlahti
wlahti committed Jul 26, 2017
1 parent d6b54c8 commit 36e5140
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 34 deletions.
4 changes: 2 additions & 2 deletions core/container/api/core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ import (
type BuildSpecFactory func() (io.Reader, error)
type PrelaunchFunc func() error

//abstract virtual image for supporting arbitrary virual machines
//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, builder BuildSpecFactory, preLaunchFunc PrelaunchFunc) 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) (string, error)
GetVMName(ccID ccintf.CCID, format func(string) (string, error)) (string, error)
}
2 changes: 1 addition & 1 deletion core/container/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func VMCProcess(ctxt context.Context, vmtype string, req VMCReqIntf) (interface{
go func() {
defer close(c)

id, err := v.GetVMName(req.getCCID())
id, err := v.GetVMName(req.getCCID(), nil)
if err != nil {
resp = VMCResp{Err: err}
return
Expand Down
72 changes: 47 additions & 25 deletions core/container/dockercontroller/dockercontroller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package dockercontroller

import (
"bytes"
"encoding/hex"
"fmt"
"io"
"strings"
Expand All @@ -29,6 +30,7 @@ import (

"github.com/fsouza/go-dockerclient"
"github.com/hyperledger/fabric/common/flogging"
"github.com/hyperledger/fabric/common/util"
container "github.com/hyperledger/fabric/core/container/api"
"github.com/hyperledger/fabric/core/container/ccintf"
cutil "github.com/hyperledger/fabric/core/container/util"
Expand All @@ -40,6 +42,8 @@ import (
var (
dockerLogger = flogging.MustGetLogger("dockercontroller")
hostConfig *docker.HostConfig
vmRegExp = regexp.MustCompile("[^a-zA-Z0-9-_.]")
imageRegExp = regexp.MustCompile("^[a-z0-9]+(([._-][a-z0-9]+)+)?$")
)

// getClient returns an instance that implements dockerClient interface
Expand Down Expand Up @@ -163,7 +167,7 @@ func (vm *DockerVM) createContainer(ctxt context.Context, client dockerClient,

func (vm *DockerVM) deployImage(client dockerClient, ccid ccintf.CCID,
args []string, env []string, reader io.Reader) error {
id, err := vm.GetVMName(ccid)
id, err := vm.GetVMName(ccid, formatImageName)
if err != nil {
return err
}
Expand Down Expand Up @@ -208,7 +212,7 @@ func (vm *DockerVM) Deploy(ctxt context.Context, ccid ccintf.CCID,
//Start starts a container using a previously created docker image
func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
args []string, env []string, builder container.BuildSpecFactory, prelaunchFunc container.PrelaunchFunc) error {
imageID, err := vm.GetVMName(ccid)
imageID, err := vm.GetVMName(ccid, formatImageName)
if err != nil {
return err
}
Expand All @@ -219,7 +223,11 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
return err
}

containerID := strings.Replace(imageID, ":", "_", -1)
containerID, err := vm.GetVMName(ccid, nil)
if err != nil {
return err
}

attachStdout := viper.GetBool("vm.docker.attachStdout")

//stop,force remove if necessary
Expand Down Expand Up @@ -349,7 +357,7 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,

//Stop stops a running chaincode
func (vm *DockerVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error {
id, err := vm.GetVMName(ccid)
id, err := vm.GetVMName(ccid, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -395,7 +403,7 @@ func (vm *DockerVM) stopInternal(ctxt context.Context, client dockerClient,

//Destroy destroys an image
func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error {
id, err := vm.GetVMName(ccid)
id, err := vm.GetVMName(ccid, formatImageName)
if err != nil {
return err
}
Expand All @@ -418,34 +426,48 @@ func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
return err
}

//GetVMName generates the docker image from peer information given the hashcode. This is needed to
//keep image name's unique in a single host, multi-peer environment (such as a development environment)
func (vm *DockerVM) GetVMName(ccid ccintf.CCID) (string, error) {
// GetVMName generates the VM name from peer information. It accepts a format
// function parameter to allow different formatting based on the desired use of
// the name.
func (vm *DockerVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
name := ccid.GetName()

//Convert to lowercase and replace any invalid characters with "-"
r := regexp.MustCompile("[^a-zA-Z0-9-_.]")

// replace any invalid characters with "-"
if ccid.NetworkID != "" && ccid.PeerID != "" {
name = strings.ToLower(
r.ReplaceAllString(
fmt.Sprintf("%s-%s-%s", ccid.NetworkID, ccid.PeerID, name), "-"))
name = vmRegExp.ReplaceAllString(
fmt.Sprintf("%s-%s-%s", ccid.NetworkID, ccid.PeerID, name), "-")
} else if ccid.NetworkID != "" {
name = strings.ToLower(
r.ReplaceAllString(
fmt.Sprintf("%s-%s", ccid.NetworkID, name), "-"))
name = vmRegExp.ReplaceAllString(
fmt.Sprintf("%s-%s", ccid.NetworkID, name), "-")
} else if ccid.PeerID != "" {
name = strings.ToLower(
r.ReplaceAllString(
fmt.Sprintf("%s-%s", ccid.PeerID, name), "-"))
name = vmRegExp.ReplaceAllString(
fmt.Sprintf("%s-%s", ccid.PeerID, name), "-")
}

// Check name complies with Docker's repository naming rules
r = regexp.MustCompile("^[a-z0-9]+(([._-][a-z0-9]+)+)?$")
if format != nil {
formattedName, err := format(name)
if err != nil {
return formattedName, err
}
// check to ensure format function didn't add any invalid characters
name = vmRegExp.ReplaceAllString(formattedName, "-")
}
return name, nil
}

if !r.MatchString(name) {
// formatImageName formats the docker image from peer information. This is
// needed to keep image (repository) names unique in a single host, multi-peer
// environment (such as a development environment). It computes the hash for the
// supplied image name and then appends it to the lowercase image name to ensure
// uniqueness.
func formatImageName(name string) (string, error) {
imageName := strings.ToLower(fmt.Sprintf("%s-%s", name, hex.EncodeToString(util.ComputeSHA256([]byte(name)))))

// Check that name complies with Docker's repository naming rules
if !imageRegExp.MatchString(imageName) {
dockerLogger.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", name)
return name, fmt.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", name)
return imageName, fmt.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", imageName)
}
return name, nil

return imageName, nil
}
38 changes: 38 additions & 0 deletions core/container/dockercontroller/dockercontroller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"bytes"
"compress/gzip"
"context"
"encoding/hex"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -226,6 +227,39 @@ func Test_Destroy(t *testing.T) {
testerr(t, err, true)
}

type testCase struct {
name string
ccid ccintf.CCID
formatFunc func(string) (string, error)
expectedOutput string
}

func TestGetVMName(t *testing.T) {
dvm := DockerVM{}
var tc []testCase

tc = append(tc,
testCase{"mycc", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, NetworkID: "dev", PeerID: "peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-mycc-1.0"))))},
testCase{"mycc-nonetworkid", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, PeerID: "peer1", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "peer1-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("peer1-mycc-1.0"))))},
testCase{"myCC", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("Dev-Peer0-myCC-1.0"))))},
testCase{"mycc-nopeerid", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, NetworkID: "dev", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-mycc-1.0"))))},
testCase{"myCC", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "dev", PeerID: "peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-myCC-1.0"))))},
testCase{"myCC-preserveCase", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, nil, fmt.Sprintf("%s", "Dev-Peer0-myCC-1.0")},
testCase{"invalidCharsFormatFunction", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, formatInvalidChars, fmt.Sprintf("%s", "inv-lid-character--")})

for _, test := range tc {
name, err := dvm.GetVMName(test.ccid, test.formatFunc)
assert.Nil(t, err, "Expected nil error")
assert.Equal(t, test.expectedOutput, name, "Unexpected output for test case name: %s", test.name)
}

}

func TestFormatImageName_invalidChars(t *testing.T) {
_, err := formatImageName("invalid*chars")
assert.NotNil(t, err, "Expected error")
}

func getCodeChainBytesInMem() io.Reader {
startTime := time.Now()
inputbuf := bytes.NewBuffer(nil)
Expand Down Expand Up @@ -322,3 +356,7 @@ func (c *mockClient) RemoveContainer(opts docker.RemoveContainerOptions) error {
}
return nil
}

func formatInvalidChars(name string) (string, error) {
return "inv@lid*character$/", nil
}
22 changes: 16 additions & 6 deletions core/container/inproccontroller/inproccontroller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (vm *InprocVM) Deploy(ctxt context.Context, ccid ccintf.CCID, args []string
return fmt.Errorf(fmt.Sprintf("%s system chaincode does not contain chaincode instance", path))
}

instName, _ := vm.GetVMName(ccid)
instName, _ := vm.GetVMName(ccid, nil)
_, err := vm.getInstance(ctxt, ipctemplate, instName, args, env)

//FUTURE ... here is where we might check code for safety
Expand Down Expand Up @@ -163,7 +163,7 @@ func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string,
return fmt.Errorf(fmt.Sprintf("%s not registered", path))
}

instName, _ := vm.GetVMName(ccid)
instName, _ := vm.GetVMName(ccid, nil)

ipc, err := vm.getInstance(ctxt, ipctemplate, instName, args, env)

Expand Down Expand Up @@ -211,7 +211,7 @@ func (vm *InprocVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, d
return fmt.Errorf("%s not registered", path)
}

instName, _ := vm.GetVMName(ccid)
instName, _ := vm.GetVMName(ccid, nil)

ipc := instRegistry[instName]

Expand All @@ -236,7 +236,17 @@ func (vm *InprocVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
return nil
}

//GetVMName ignores the peer and network name as it just needs to be unique in process
func (vm *InprocVM) GetVMName(ccid ccintf.CCID) (string, error) {
return ccid.GetName(), nil
// GetVMName ignores the peer and network name as it just needs to be unique in
// process. It accepts a format function parameter to allow different
// formatting based on the desired use of the name.
func (vm *InprocVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
name := ccid.GetName()
if format != nil {
formattedName, err := format(name)
if err != nil {
return formattedName, err
}
name = formattedName
}
return name, nil
}

0 comments on commit 36e5140

Please sign in to comment.