Skip to content

Commit

Permalink
FABN-864 service discovery usage improvements
Browse files Browse the repository at this point in the history
- Change initialization of gateway so that channels no longer
need to be defined in the CCP.
- ‘asLocalhost’ option now defaults to ‘true’ and is overridable
by an environment variable

Change-Id: I36c2a1b8d87c4b4f4a5e25c9d966f190486cec13
Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
  • Loading branch information
andrew-coleman committed Nov 13, 2018
1 parent 4348438 commit 65a8da9
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 37 deletions.
2 changes: 1 addition & 1 deletion build/tasks/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ gulp.task('run-tape-e2e', ['docker-ready'],
'test/integration/fabric-ca-certificate-service-tests.js',
'test/integration/fabric-ca-services-tests.js',
'test/integration/nodechaincode/e2e.js',
'test/integration/e2e.js',
'test/integration/network-e2e/e2e.js',
'test/integration/network-e2e/e2e-hsm.js',
'test/integration/e2e.js',
'test/integration/signTransactionOffline.js',
'test/integration/query.js',
'test/integration/client.js',
Expand Down
1 change: 1 addition & 0 deletions fabric-client/config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"grpc-wait-for-ready-timeout": 3000,
"initialize-with-discovery": false,
"discovery-as-localhost": true,
"discovery-cache-life": 300000,
"discovery-protocol": "grpcs",
"endorsement-handler": "fabric-client/lib/impl/DiscoveryEndorsementHandler.js",
Expand Down
11 changes: 10 additions & 1 deletion fabric-client/lib/Channel.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const Channel = class {
this._last_discover_timestamp = null;
this._discovery_peer = null;
this._use_discovery = sdk_utils.getConfigSetting('initialize-with-discovery', false);
this._as_localhost = sdk_utils.getConfigSetting('discovery-as-localhost', true);
this._endorsement_handler = null; // will be setup during initialization
this._commit_handler = null;

Expand Down Expand Up @@ -205,6 +206,14 @@ const Channel = class {
throw new Error('Request parameter "discover" must be boolean');
}
}
if (typeof request.asLocalhost !== 'undefined') {
if (typeof request.asLocalhost === 'boolean') {
logger.debug('%s - user requested discovery as localhost %s', method, request.asLocalhost);
this._as_localhost = request.asLocalhost;
} else {
throw new Error('Request parameter "asLocalhost" must be boolean');
}
}
if (request.endorsementHandler) {
logger.debug('%s - user requested endorsementHandler %s', method, request.endorsementHandler);
endorsement_handler_path = request.endorsementHandler;
Expand Down Expand Up @@ -1445,7 +1454,7 @@ const Channel = class {
let t_hostname = hostname;

// endpoints may be running in containers on the local system
if (request && request.asLocalhost) {
if (this._as_localhost) {
t_hostname = 'localhost';
}

Expand Down
9 changes: 6 additions & 3 deletions fabric-network/lib/gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ class Gateway {
strategy: EventStrategies.MSPID_SCOPE_ALLFORTX
},
discovery: {
enabled: true,
asLocalhost: false
enabled: true
}
};
}
Expand Down Expand Up @@ -230,7 +229,11 @@ class Gateway {
}

logger.debug('getNetwork: create network object and initialize');
const channel = this.client.getChannel(networkName);
let channel = this.client.getChannel(networkName, false);
if (channel === null) {
// not found in the in-memory cache or the CCP
channel = this.client.newChannel(networkName);
}
const newNetwork = new Network(this, channel);
await newNetwork._initialize(this.options.discovery);
this.networks.set(networkName, newNetwork);
Expand Down
17 changes: 12 additions & 5 deletions fabric-network/lib/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,14 @@ class Network {
// TODO: should sort peer list to the identity org initializing the channel.
// TODO: Candidate to push to low level node-sdk.

const ledgerPeers = this.channel.getPeers().filter((cPeer) => {
return cPeer.isInRole(FabricConstants.NetworkConfig.LEDGER_QUERY_ROLE);
});
let ledgerPeers;
if (discovery.enabled) {
ledgerPeers = this.gateway.getClient().getPeersForOrg();
} else {
ledgerPeers = this.channel.getPeers().filter((cPeer) => {
return cPeer.isInRole(FabricConstants.NetworkConfig.LEDGER_QUERY_ROLE);
});
}

if (ledgerPeers.length === 0) {
const msg = 'no suitable peers available to initialize from';
Expand All @@ -99,9 +104,11 @@ class Network {
try {
const initOptions = {
target: ledgerPeers[ledgerPeerIndex],
discover: discovery.enabled,
asLocalhost: discovery.asLocalhost
discover: discovery.enabled
};
if (typeof discovery.asLocalhost !== 'undefined') {
initOptions.asLocalhost = discovery.asLocalhost;
}

await this.channel.initialize(initOptions);

Expand Down
13 changes: 13 additions & 0 deletions fabric-network/test/gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ describe('Gateway', () => {
mockNetwork = sinon.createStubInstance(Network);
gateway.networks.set('foo', mockNetwork);
gateway.client = mockClient;
gateway.options.discovery.enabled = false;

mockInternalChannel = sinon.createStubInstance(InternalChannel);
const mockPeer1 = sinon.createStubInstance(Peer);
Expand Down Expand Up @@ -450,6 +451,18 @@ describe('Gateway', () => {
network2.channel.should.equal(mockInternalChannel);
gateway.networks.size.should.equal(2);
});

it('should create a channel object if not defined in the ccp', async () => {
mockClient.getChannel.withArgs('bar').returns(null);
mockClient.newChannel.withArgs('bar').returns(mockInternalChannel);
gateway.getCurrentIdentity = sinon.stub().returns({_mspId: 'MSP01'});

const network2 = await gateway.getNetwork('bar');
network2.should.be.instanceof(Network);
network2.gateway.should.equal(gateway);
network2.channel.should.equal(mockInternalChannel);
gateway.networks.size.should.equal(2);
});
});

describe('#disconnect', () => {
Expand Down
19 changes: 14 additions & 5 deletions fabric-network/test/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ describe('Network', () => {
}
});

mockGateway.getClient.returns(mockClient);
mockClient.getPeersForOrg.returns([mockPeer1, mockPeer2]);

network = new Network(mockGateway, mockChannel);

});
Expand Down Expand Up @@ -99,6 +102,12 @@ describe('Network', () => {
sinon.assert.calledOnce(mockChannel.initialize);
});

it('should initialize the network using the first peer with discovery', async () => {
mockChannel.initialize.resolves();
await network._initializeInternalChannel({enabled:true});
sinon.assert.calledOnce(mockChannel.initialize);
});

it('should try other peers if initialization fails', async () => {
network.initialized = false;
// create a real mock
Expand All @@ -117,15 +126,15 @@ describe('Network', () => {
mockChannel.initialize.onCall(2).rejects(new Error('connect failed again'));
let error;
try {
await network._initializeInternalChannel({enabled:true, asLocalhost: true});
await network._initializeInternalChannel({enabled:false, asLocalhost: true});
} catch (_error) {
error = _error;
}
error.should.match(/connect failed again/);
sinon.assert.calledThrice(mockChannel.initialize);
sinon.assert.calledWith(mockChannel.initialize.firstCall, {target: mockPeer1, discover: true, asLocalhost: true});
sinon.assert.calledWith(mockChannel.initialize.secondCall, {target: mockPeer3, discover: true, asLocalhost: true});
sinon.assert.calledWith(mockChannel.initialize.thirdCall, {target: mockPeer4, discover: true, asLocalhost: true});
sinon.assert.calledWith(mockChannel.initialize.firstCall, {target: mockPeer1, discover: false, asLocalhost: true});
sinon.assert.calledWith(mockChannel.initialize.secondCall, {target: mockPeer3, discover: false, asLocalhost: true});
sinon.assert.calledWith(mockChannel.initialize.thirdCall, {target: mockPeer4, discover: false, asLocalhost: true});
});

it('should fail if there are no LEDGER_QUERY_ROLE peers', async () => {
Expand All @@ -137,7 +146,7 @@ describe('Network', () => {
mockPeer5.isInRole.withArgs(FABRIC_CONSTANTS.NetworkConfig.LEDGER_QUERY_ROLE).returns(false);
peerArray = [mockPeer1, mockPeer2, mockPeer3, mockPeer4, mockPeer5];
mockChannel.getPeers.returns(peerArray);
return network._initializeInternalChannel()
return network._initializeInternalChannel({discover: false})
.should.be.rejectedWith(/no suitable peers available to initialize from/);
});
});
Expand Down
8 changes: 0 additions & 8 deletions test/fixtures/network-discovery.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,6 @@
"client":{
"organization":"Org1"
},
"channels":{
"mychannel":{
"peers":{
"peer0.org1.example.com":{
}
}
}
},
"organizations":{
"Org1":{
"mspid":"Org1MSP",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/network-e2e/e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ require('../e2e/create-channel.js');
require('../e2e/join-channel.js');
require('./install-chaincode.js');
require('./instantiate-chaincode.js');
require('./updateAnchorPeers');
// require('./updateAnchorPeers');
require('./invoke.js');
require('./query.js');
14 changes: 3 additions & 11 deletions test/integration/network-e2e/invoke.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ async function tlsSetup() {
}

async function createContract(t, gateway, gatewayOptions) {
const profile = gatewayOptions.useDiscovery ? ccpDiscovery : ccp;
const useDiscovery = !(gatewayOptions.discovery && gatewayOptions.discovery.enabled === false);
const profile = useDiscovery ? ccpDiscovery : ccp;
await gateway.connect(JSON.parse(profile.toString()), gatewayOptions);
t.pass('Connected to the gateway');

Expand Down Expand Up @@ -86,10 +87,7 @@ test('\n\n***** Network End-to-end flow: invoke transaction to move money using
const contract = await createContract(t, gateway, {
wallet: inMemoryWallet,
identity: 'User1@org1.example.com',
clientTlsIdentity: 'tlsId',
discovery: {
asLocalHost: true
}
clientTlsIdentity: 'tlsId'
});

const transaction = contract.createTransaction('move');
Expand Down Expand Up @@ -393,9 +391,6 @@ test('\n\n***** Network End-to-end flow: invoke transaction to move money using
clientTlsIdentity: 'tlsId',
eventHandlerOptions: {
strategy: DefaultEventHandlerStrategies.NETWORK_SCOPE_ALLFORTX
},
discovery: {
asLocalHost: true
}
});

Expand Down Expand Up @@ -524,9 +519,6 @@ test('\n\n***** Network End-to-end flow: invoke transaction to move money using
clientTlsIdentity: 'tlsId',
eventHandlerOptions: {
strategy: DefaultEventHandlerStrategies.NETWORK_SCOPE_ANYFORTX
},
discovery: {
asLocalHost: true
}
});

Expand Down
4 changes: 2 additions & 2 deletions test/integration/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ test(' ---->>>>> Query channel working <<<<<-----', (t) => {
const block_num = Number(processed_transaction.transactionEnvelope.payload.data.actions['0']
.payload.action.proposal_response_payload.extension.results.ns_rwset['0']
.rwset.reads[1].version.block_num.toString());
if (parseInt(block_num) > 7) {
if (parseInt(block_num) >= 7) {
t.pass('Successfully test for read set block num');
} else {
t.fail('Failed test for read set block num - block_num > 7 ::' + block_num);
t.fail('Failed test for read set block num - block_num >= 7 ::' + block_num);
}

// the "target peer" must be a peer in the same org as the app
Expand Down

0 comments on commit 65a8da9

Please sign in to comment.