Skip to content

Commit 7bc73ea

Browse files
author
Jason Yellick
committed
FAB-9806 Remove GetVMName filter func
The interface for container.VM has a 'GetVMName(string, func(string) (string, error)) (string, error)' definition. However, no callers outside of the dockercontroller package passes in a value for the function of anything other than 'nil'. This internal implementation detail of dockercontroller does not need to be exposed outside of its package, so this CR removes it. Change-Id: I96351cf1559c8366e9b11a78466d497881acd395 Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
1 parent 0a7719b commit 7bc73ea

File tree

5 files changed

+64
-101
lines changed

5 files changed

+64
-101
lines changed

‎core/container/controller.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type VM interface {
3333
Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, filesToUpload map[string][]byte, builder Builder) error
3434
Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error
3535
Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error
36-
GetVMName(ccID ccintf.CCID, format func(string) (string, error)) (string, error)
36+
GetVMName(ccID ccintf.CCID) string
3737
}
3838

3939
type refCountedLock struct {
@@ -165,13 +165,9 @@ func (vmc *VMController) Process(ctxt context.Context, vmtype string, req VMCReq
165165

166166
c := make(chan error)
167167
go func() {
168-
id, err := v.GetVMName(req.GetCCID(), nil)
169-
if err != nil {
170-
c <- err
171-
return
172-
}
168+
id := v.GetVMName(req.GetCCID())
173169
vmc.lockContainer(id)
174-
err = req.Do(ctxt, v)
170+
err := req.Do(ctxt, v)
175171
vmc.unlockContainer(id)
176172
c <- err
177173
}()

‎core/container/dockercontroller/dockercontroller.go

Lines changed: 26 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func (vm *DockerVM) createContainer(ctxt context.Context, client dockerClient,
190190

191191
func (vm *DockerVM) deployImage(client dockerClient, ccid ccintf.CCID,
192192
args []string, env []string, reader io.Reader) error {
193-
id, err := vm.GetVMName(ccid, formatImageName)
193+
id, err := vm.GetVMNameForDocker(ccid)
194194
if err != nil {
195195
return err
196196
}
@@ -235,7 +235,7 @@ func (vm *DockerVM) Deploy(ctxt context.Context, ccid ccintf.CCID,
235235
//Start starts a container using a previously created docker image
236236
func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
237237
args []string, env []string, filesToUpload map[string][]byte, builder container.Builder) error {
238-
imageName, err := vm.GetVMName(ccid, formatImageName)
238+
imageName, err := vm.GetVMNameForDocker(ccid)
239239
if err != nil {
240240
return err
241241
}
@@ -246,10 +246,7 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
246246
return err
247247
}
248248

249-
containerName, err := vm.GetVMName(ccid, nil)
250-
if err != nil {
251-
return err
252-
}
249+
containerName := vm.GetVMName(ccid)
253250

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

@@ -405,10 +402,7 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
405402

406403
//Stop stops a running chaincode
407404
func (vm *DockerVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error {
408-
id, err := vm.GetVMName(ccid, nil)
409-
if err != nil {
410-
return err
411-
}
405+
id := vm.GetVMName(ccid)
412406

413407
client, err := vm.getClientFnc()
414408
if err != nil {
@@ -451,7 +445,7 @@ func (vm *DockerVM) stopInternal(ctxt context.Context, client dockerClient,
451445

452446
//Destroy destroys an image
453447
func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error {
454-
id, err := vm.GetVMName(ccid, formatImageName)
448+
id, err := vm.GetVMNameForDocker(ccid)
455449
if err != nil {
456450
return err
457451
}
@@ -477,41 +471,22 @@ func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
477471
// GetVMName generates the VM name from peer information. It accepts a format
478472
// function parameter to allow different formatting based on the desired use of
479473
// the name.
480-
func (vm *DockerVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
481-
name := ccid.GetName()
482-
483-
if vm.NetworkID != "" && vm.PeerID != "" {
484-
name = fmt.Sprintf("%s-%s-%s", vm.NetworkID, vm.PeerID, name)
485-
} else if vm.NetworkID != "" {
486-
name = fmt.Sprintf("%s-%s", vm.NetworkID, name)
487-
} else if vm.PeerID != "" {
488-
name = fmt.Sprintf("%s-%s", vm.PeerID, name)
489-
}
490-
491-
if format != nil {
492-
formattedName, err := format(name)
493-
if err != nil {
494-
return formattedName, err
495-
}
496-
name = formattedName
497-
}
498-
474+
func (vm *DockerVM) GetVMName(ccid ccintf.CCID) string {
499475
// replace any invalid characters with "-" (either in network id, peer id, or in the
500476
// entire name returned by any format function)
501-
name = vmRegExp.ReplaceAllString(name, "-")
502-
503-
return name, nil
477+
return vmRegExp.ReplaceAllString(vm.preFormatImageName(ccid), "-")
504478
}
505479

506-
// formatImageName formats the docker image from peer information. This is
480+
// GetVMNameForDocker formats the docker image from peer information. This is
507481
// needed to keep image (repository) names unique in a single host, multi-peer
508482
// environment (such as a development environment). It computes the hash for the
509483
// supplied image name and then appends it to the lowercase image name to ensure
510484
// uniqueness.
511-
func formatImageName(name string) (string, error) {
485+
func (vm *DockerVM) GetVMNameForDocker(ccid ccintf.CCID) (string, error) {
486+
name := vm.preFormatImageName(ccid)
512487
hash := hex.EncodeToString(util.ComputeSHA256([]byte(name)))
513-
name = vmRegExp.ReplaceAllString(name, "-")
514-
imageName := strings.ToLower(fmt.Sprintf("%s-%s", name, hash))
488+
saniName := vmRegExp.ReplaceAllString(name, "-")
489+
imageName := strings.ToLower(fmt.Sprintf("%s-%s", saniName, hash))
515490

516491
// Check that name complies with Docker's repository naming rules
517492
if !imageRegExp.MatchString(imageName) {
@@ -521,3 +496,17 @@ func formatImageName(name string) (string, error) {
521496

522497
return imageName, nil
523498
}
499+
500+
func (vm *DockerVM) preFormatImageName(ccid ccintf.CCID) string {
501+
name := ccid.GetName()
502+
503+
if vm.NetworkID != "" && vm.PeerID != "" {
504+
name = fmt.Sprintf("%s-%s-%s", vm.NetworkID, vm.PeerID, name)
505+
} else if vm.NetworkID != "" {
506+
name = fmt.Sprintf("%s-%s", vm.NetworkID, name)
507+
} else if vm.PeerID != "" {
508+
name = fmt.Sprintf("%s-%s", vm.PeerID, name)
509+
}
510+
511+
return name
512+
}

‎core/container/dockercontroller/dockercontroller_test.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -241,72 +241,69 @@ type testCase struct {
241241
name string
242242
vm *DockerVM
243243
ccid ccintf.CCID
244-
formatFunc func(string) (string, error)
245244
expectedOutput string
246245
}
247246

248-
func TestGetVMName(t *testing.T) {
247+
func TestGetVMNameForDocker(t *testing.T) {
249248
tc := []testCase{
250249
{
251250
name: "mycc",
252251
vm: &DockerVM{NetworkID: "dev", PeerID: "peer0"},
253252
ccid: ccintf.CCID{Name: "mycc", Version: "1.0"},
254-
formatFunc: formatImageName,
255253
expectedOutput: fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-mycc-1.0")))),
256254
},
257255
{
258256
name: "mycc-nonetworkid",
259257
vm: &DockerVM{PeerID: "peer1"},
260258
ccid: ccintf.CCID{Name: "mycc", Version: "1.0"},
261-
formatFunc: formatImageName,
262259
expectedOutput: fmt.Sprintf("%s-%s", "peer1-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("peer1-mycc-1.0")))),
263260
},
264261
{
265262
name: "myCC-UCids",
266263
vm: &DockerVM{NetworkID: "Dev", PeerID: "Peer0"},
267264
ccid: ccintf.CCID{Name: "myCC", Version: "1.0"},
268-
formatFunc: formatImageName,
269265
expectedOutput: fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("Dev-Peer0-myCC-1.0")))),
270266
},
271267
{
272268
name: "myCC-idsWithSpecialChars",
273269
vm: &DockerVM{NetworkID: "Dev$dev", PeerID: "Peer*0"},
274270
ccid: ccintf.CCID{Name: "myCC", Version: "1.0"},
275-
formatFunc: formatImageName,
276271
expectedOutput: fmt.Sprintf("%s-%s", "dev-dev-peer-0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("Dev$dev-Peer*0-myCC-1.0")))),
277272
},
278273
{
279274
name: "mycc-nopeerid",
280275
vm: &DockerVM{NetworkID: "dev"},
281276
ccid: ccintf.CCID{Name: "mycc", Version: "1.0"},
282-
formatFunc: formatImageName,
283277
expectedOutput: fmt.Sprintf("%s-%s", "dev-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-mycc-1.0")))),
284278
},
285279
{
286280
name: "myCC-LCids",
287281
vm: &DockerVM{NetworkID: "dev", PeerID: "peer0"},
288282
ccid: ccintf.CCID{Name: "myCC", Version: "1.0"},
289-
formatFunc: formatImageName,
290283
expectedOutput: fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-myCC-1.0")))),
291284
},
285+
}
286+
287+
for _, test := range tc {
288+
name, err := test.vm.GetVMNameForDocker(test.ccid)
289+
assert.Nil(t, err, "Expected nil error")
290+
assert.Equal(t, test.expectedOutput, name, "Unexpected output for test case name: %s", test.name)
291+
}
292+
293+
}
294+
295+
func TestGetVMName(t *testing.T) {
296+
tc := []testCase{
292297
{
293298
name: "myCC-preserveCase",
294299
vm: &DockerVM{NetworkID: "Dev", PeerID: "Peer0"},
295300
ccid: ccintf.CCID{Name: "myCC", Version: "1.0"},
296-
formatFunc: nil,
297-
expectedOutput: fmt.Sprintf("%s", "Dev-Peer0-myCC-1.0")},
298-
{
299-
name: "invalidCharsFormatFunction",
300-
vm: &DockerVM{NetworkID: "Dev", PeerID: "Peer0"},
301-
ccid: ccintf.CCID{Name: "myCC", Version: "1.0"},
302-
formatFunc: formatInvalidChars,
303-
expectedOutput: fmt.Sprintf("%s", "inv-lid-character--"),
301+
expectedOutput: fmt.Sprintf("%s", "Dev-Peer0-myCC-1.0"),
304302
},
305303
}
306304

307305
for _, test := range tc {
308-
name, err := test.vm.GetVMName(test.ccid, test.formatFunc)
309-
assert.Nil(t, err, "Expected nil error")
306+
name := test.vm.GetVMName(test.ccid)
310307
assert.Equal(t, test.expectedOutput, name, "Unexpected output for test case name: %s", test.name)
311308
}
312309

@@ -428,7 +425,3 @@ func (c *mockClient) RemoveContainer(opts docker.RemoveContainerOptions) error {
428425
}
429426
return nil
430427
}
431-
432-
func formatInvalidChars(name string) (string, error) {
433-
return "inv@lid*character$/", nil
434-
}

‎core/container/inproccontroller/inproccontroller.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (vm *InprocVM) Deploy(ctxt context.Context, ccid ccintf.CCID, args []string
121121
return fmt.Errorf(fmt.Sprintf("%s system chaincode does not contain chaincode instance", path))
122122
}
123123

124-
instName, _ := vm.GetVMName(ccid, nil)
124+
instName := vm.GetVMName(ccid)
125125
_, err := vm.getInstance(ctxt, ipctemplate, instName, args, env)
126126

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

193-
instName, _ := vm.GetVMName(ccid, nil)
193+
instName := vm.GetVMName(ccid)
194194

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

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

235-
instName, _ := vm.GetVMName(ccid, nil)
235+
instName := vm.GetVMName(ccid)
236236

237237
ipc := vm.registry.instRegistry[instName]
238238

@@ -260,14 +260,6 @@ func (vm *InprocVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
260260
// GetVMName ignores the peer and network name as it just needs to be unique in
261261
// process. It accepts a format function parameter to allow different
262262
// formatting based on the desired use of the name.
263-
func (vm *InprocVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
264-
name := ccid.GetName()
265-
if format != nil {
266-
formattedName, err := format(name)
267-
if err != nil {
268-
return formattedName, err
269-
}
270-
name = formattedName
271-
}
272-
return name, nil
263+
func (vm *InprocVM) GetVMName(ccid ccintf.CCID) string {
264+
return ccid.GetName()
273265
}

‎core/container/mock/vm.go

Lines changed: 15 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)