fix empty endpoints weight for AUTO_PASSTHROUGH gateway

[bincherry]

Please provide a description of this PR:

For AUTO_PASSTHROUGH gateway, typically istio-eastwestgateway, the endpoints of SNI-dnat cluster will be filtered by mtls checker. Yet the filtered LocalityLbEndpoints results have empty LoadBalancingWeight, that would cause LB error in sidecar.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[istio-policy-bot]

😊 Welcome @bincherry! This is either your first contribution to the Istio istio repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

[istio-testing]

Hi @bincherry. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[GregHanson]

/ok-to-test

[howardjohn]

Thanks for the PR! Can you give more details? How to replicate? What error? And we need tests covering this

[tedli]

The case is that, service call cross clusters through eastwest gateway with lb policy of consistency hash, result no healthy upstream. Because the locality weight in cluster load assignment is 0.

Ring hash logic in envoy code, read the locality weight:

https://github.com/envoyproxy/envoy/blob/652a89b6a76e6a59521a46d764b2bd52317421a0/source/common/upstream/thread_aware_lb_impl.cc#L54

https://github.com/envoyproxy/envoy/blob/652a89b6a76e6a59521a46d764b2bd52317421a0/source/common/upstream/ring_hash_lb.cc#L54

[bincherry]

Thanks for the PR! Can you give more details? How to replicate? What error? And we need tests covering this

Errors would happen in sidecar because of the empty load_balancing_weight of endpoints, which should be as least 1. For exmaple as described by tedli.

For now the empty load_balancing_weight will happen on every eastwestgateway, and we could get config_dump and check endpoints of SNI DNAT clusters to find it.

    {
     "endpoint_config": {
      "@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
      "cluster_name": "outbound_.8081_._.web.sky.svc.cluster.local",
      "endpoints": [
       {
        "locality": {},
        "lb_endpoints": [
         {
          "endpoint": {
           "address": {
            "socket_address": {
             "address": "172.30.40.144",
             "port_value": 8081
            }
           },
           "health_check_config": {}
          },
          "health_status": "HEALTHY",
          "metadata": {
           "filter_metadata": {
            "envoy.transport_socket_match": {
             "tlsMode": "istio"
            },
            "istio": {
             "workload": "web;sky;web;latest;cid-5a3b17793356"
            }
           }
          },
          "load_balancing_weight": 1
         }
        ],
        "load_balancing_weight": 0             ### HERE
       }
      ],
      "policy": {
       "overprovisioning_factor": 140
      }
     }
    },

[hzxuzhonghu]

cc @stevenctl

/ok-to-test

[bincherry]

/retest

[stevenctl]

Good catch, probably part of the fix for #38704

[stevenctl]

cc @howardjohn if you want to drop the change request

[tedli]

Hi all,

During we debug with this pr, we also found another issue istio/istio.io#11680, which was not documented. Then we give a pr to the doc repo.

Could someone help to assign reviewers to take a look?

stale

[howardjohn]

/retest-required