Scale from Zero not using multiple matches about deployments-k8s HOT 8 CLOSED

@edwarnicke Previously we fixed another problem that you pointed. As I remember it was related to proccesing clientInfo labels. We fixed the problem in https://github.com/networkservicemesh/deployments-k8s/pull/1427/files#diff-01c4adc6e8ef21f0e9bd8be14010662eea9b27225b3a51ded0e39cdb5d0614f4R15

It would be nice to see the expected network service for this scenario. As I understand you mean that it should work with ns:

---
apiVersion: networkservicemesh.io/v1
kind: NetworkService
metadata:
  name: autoscale-icmp-responder
  namespace: nsm-system
spec:
  payload: ETHERNET
  name: autoscale-icmp-responder
  matches:
    - source_selector:
      routes:
        - destination_selector:
            app: nse-icmp-responder
            nodeName: "{{.nodeName}}"

Could you please say more about expectations?

from deployments-k8s.

In discover server we select first suitable match and then unconditionally end search.

So if we were to separate endpoints in the scale-from-zero example to 2 matches, then only first of the matches will ever be used, because of behavior of discover.

We were considering making discover fall through matches that don't resolve to any endpoints, to make it behave as advertised in the scale-from-zero proposal. After some considerations we rejected this change for the following reason:
Right now cluster admin can deploy network service with matches X and Y, and be sure that if a request matches X then it will never-ever go to route from Y.
But if we were to skip matches without live endpoints, then if endpoint(s) from match X are temporary unavailable, then we would go to match Y, and there is nothing that cluster admin can do about this. So, matches would become unreliable.

So we decided to place both routes in one match.
I believe that this is how matches were intended to be used. Am I wrong here?

from deployments-k8s.

roundrobin should be round robining among candidates.

discover discovers candidates. Round robin selects among them.

If roundrobin is functioning correctly... then the current approach will half the time pick the supplier, even when there's a topological match available.

Right now cluster admin can deploy network service with matches X and Y, and be sure that if a request matches X then it will never-ever go to route from Y.

Only returning the candidates from the first match matched is necessary to correctly implement composition.

But if we were to skip matches without live endpoints, then if endpoint(s) from match X are temporary unavailable, then we would go to match Y, and there is nothing that cluster admin can do about this. So, matches would become unreliable.

This makes matches less, not more, reliable, because it may result in connecting clients to inappropriate clients.

The central issues are that

• For Composition to work correctly, we need to return the candidates from the first match that matches. Even if there are none. Even if none are available.
• For Scale from Zero to work correctly, we need to be able to 'fallthrough' if there are no candidates for a match.

I had proposed (I think to Vlad, when he was looking deeply at these things) that the correct approach here was to add a
fallthrough property to a match that defaulted to false. If a match matches and has no candidates, then we continue to the next match if and only if the 'fallthrough' property is true for that match.

from deployments-k8s.

@edwarnicke Could you, please, confirm, before i've started changing code, that i understand correctly and we want matches in scale-from-zero example to be like:

---
apiVersion: networkservicemesh.io/v1
kind: NetworkService
metadata:
  name: autoscale-icmp-responder
  namespace: nsm-system
spec:
  payload: ETHERNET
  name: autoscale-icmp-responder
  matches:
    - source_selector:
      routes:
        - destination_selector:
            app: nse-icmp-responder
            nodeName: "{{.nodeName}}"
    - source_selector:
      routes:
        - destination_selector:
            app: icmp-responder-supplier 
    - fallthrough: true

from deployments-k8s.

@Mixaster995 It seems to me you have missed fallthrough

from deployments-k8s.

@denis-tingaikin thanks, modified question

from deployments-k8s.

More like:

---
apiVersion: networkservicemesh.io/v1
kind: NetworkService
metadata:
  name: autoscale-icmp-responder
  namespace: nsm-system
spec:
  payload: ETHERNET
  name: autoscale-icmp-responder
  matches:
    - source_selector:
      fallthrough: true
      routes:
        - destination_selector:
            app: nse-icmp-responder
            nodeName: "{{.nodeName}}"
    - source_selector:
      routes:
        - destination_selector:
            app: icmp-responder-supplier

from deployments-k8s.

@edwarnicke Currently NS for scale from zero looks this https://github.com/networkservicemesh/deployments-k8s/blob/main/examples/features/scale-from-zero/autoscale-netsvc.yaml#L1-L19

Be free to reopen issue if we missed something.

from deployments-k8s.