Standardize default exposed ports in the node chart

[PierreBesson]

We want to make it easier to reason about the exposed ports for substrate nodes (especially collators that run 2 nodes in 1). So from internal discussions at Parity we brainstormed the following table. The logic is to reuse conventions that arose organically while minimizing confusion (eg. it is very hard to differentiate 30334 and 30344 at a glance).

To achieve this we propose to shift port numbers by -1000 for the secondary chain (ie. relay-chain for the collator). Note that most of the times those ports don't need to be exposed.

Type	Primary	Secondary
p2p_tcp	30333	29333
p2p_ws	30444	29444
prom	9615	8615
rpc	9933	8933
rpc_ws	9944	8944

[ArshamTeymouri]

The .Values.node.perNodeServices.relayP2pService can correspond to both primary and secondary chain configs, depending on .Values.node.isParachain, making it impossible to have default values in values.yaml.

I think it's best to stick with the relaychain and parachain naming conventions if you agree:

Type	Relaychain	Parachain
p2p_tcp	30333	29333
p2p_ws	30444	29444
prom	9615	8615
rpc	9933	8933
rpc_ws	9944	8944

[PierreBesson]

No it's not best because when you deploy a parachain, you don't want to have these strange ports for your main chain. Actually the embedded relay-chain node will be optional in the future so it make sense that those will be shifted by -1000 so we keep our usual ports that have their function.

The confusion was caused by us reusing our existing relay-chain scripts to deploy parachains and it caused tremendous confusion for users that want to connect to their parachains and don't understand why it would use different ports than a regular substrate chain.

[ArshamTeymouri]

I agree, my comment was more about setting the ports based on being relaychain or parachain, rather than primary or secondary as a relaychain could be primary or secondary depending on .Values.node.isParachain.

So IIUC this would be the port numbers, right?

Type	Relaychain	Parachain
p2p_tcp	29333	30333
p2p_ws	29444	30444
prom	8615	9615
rpc	8933	9933
rpc_ws	8944	9944

[PierreBesson]

I agree, my comment was more about setting the ports based on being relaychain or parachain, rather than primary or secondary as a relaychain could be primary or secondary depending on .Values.node.isParachain.

You are completely correct that but this puts into question having default values for those ports in the values.yaml. So for now we can use old default ports. 30333 (tcp relay), 30334 (ws-relay), 30334 (tcp para), 30334 (ws-para). Overiddes will be needed for resolving conflicts for now.

[BulatSaif]

The confusion is caused by the fact that we use different approaches in ansible role, in helm-chart and default CLI settings of the application.

The port number is just a habit, if we use "8944" for the parachain and "9944" for the relaychain in the collection node, it will simplify the ansible and helm diagrams, but still cause confusion if we have different ports in the CLI default values.

I suggest moving this issue to the cumulus repository.

[PierreBesson]

I think what makes things harder is that I have made a big mistake and didn't follow initial logic of the helm-chart for implementing the p2p services. I have added relayP2pService: and paraP2pService blocks but in the helm chart there should be no concept of "relay" or "para" configuration (as it is too confusing) but only "primary" and "secondary".

So this:

perNodeServices:
    relayP2pService:
      enabled: false
      externalTrafficPolicy: Cluster
      type: NodePort # or ClusterIP or LoadBalancer
      port: 30333
      annotations: {}
      externalDns:
        enabled: false
        hostname: example.com
        ttl: 300
      # If enabled, additionally expose WebSocket port. Useful for bootnodes
      ws:
        enabled: false
        port: 30334
    ## If enabled, create service to expose parachain P2P
    ##
    paraP2pService:
      enabled: false
      externalTrafficPolicy: Cluster
      type: NodePort # or ClusterIP, LoadBalancer
      port: 30334
      annotations: {}
      externalDns:
        enabled: false
        hostname: example.com
        ttl: 300
      ws:
        enabled: false
        port: 30334

Should be IMO changed to this:

perNodeServices:
   tcpP2pService: # this refers to the relay-chain service for relay-chain node or para-chain service for parachain node
      enabled: false
      externalTrafficPolicy: Cluster
      type: NodePort # or ClusterIP, LoadBalancer
      port: 30333
      annotations: {}
      externalDns:
        enabled: false
        hostname: example.com
        ttl: 300
    wsP2pService:
        enabled: false
        port: 30444
collatorRelayChain:
    perNodeServices:
       p2pService: # this refers to the relay-chain service for collator node
           enabled: false
           externalTrafficPolicy: Cluster
           type: NodePort # or ClusterIP, LoadBalancer
           port: 29333
      wsP2pService:
        enabled: false
        port: 29444

Please 👍 or 👎 if you agree with this or not ?