Operators

[AurelienGasser]

Fix #38

Companion PRs :

• Update values for operator hlf-k8s compatibility substra-backend#227
• remove statusCheckDeadlineSeconds patch substra-tests#101

[AurelienGasser]

cc @natct10 @chrisalexandrepena This is where we're going to be working on the operators

[AurelienGasser]

@ClementGautier Thanks for the first review. I added more doc, could you please give this another look?

[ClementGautier]

It's a good enough for now I think. I'd like the README to describe more the operational processes than how it works internally (even if this information is useful for debugging purpose) for example like this:

# foo
Introduction

## Configuration
List of all variables, the expected value format and what it does.

## Usage
### Add an organization to the system channel
### Add an organization to the application channel
### Update the chaincode

Could you at least describe how to perform a chaincode upgrade ? I guess I need to add the chaincode so it's installed and modify the appChannel.chaincodeVersion but I don't know if that works out of the box.

[AlexandrePicosson]

This is nice and add scalability to this component.

[AurelienGasser]

@ClementGautier @AlexandrePicosson Thanks for your review!

This is ready for another review.

Regarding the chaincode upgrade instructions, I created an issue as it's out of scope for this PR: #50

[ClementGautier]

Good job ! I approve like this. Last question: this chart is, basically, an umbrella chart. As we forked the charts for the orderer and the peers: is it planned to move some logic into the sub-charts ? I mean some resources could be put in hlf-ord, some other on hlf-peer and some other into hlf-ca. It could reduce some complexity in the code (at least the condition which define if we deploy a peer or an orderer).

[AurelienGasser]

@ClementGautier Thanks for the review.

is it planned to move some logic into the sub-charts ?

There's no plan that I'm aware of, but +1 on the idea.

(at least the condition which define if we deploy a peer or an orderer)

Could you please expand on that? I'm not sure what you mean.

[AurelienGasser]

Side note: we have enabled the github flag "don't dismiss reviews when there's a new commit" on other Substra repos, is there any reason not to do it on this repo too?

[ClementGautier]

is there any reason not to do it on this repo too

No, it's legacy I guess, there was no guideline at the time I configured the branch protection. (btw I'm not admin anymore and can't change it myself)

Could you please expand on that? I'm not sure what you mean.

Sure, I just meant that it would result in less code to move those operators on there respective sub-chart as it would make some templating conditions useless. For example this one : {{- if and .Values.orderer.enabled .Values.ca.enabled }}

[AurelienGasser]

move those operators on there respective sub-chart

Gotcha. Note for the future, cc @Kelvin-M