From 56459d6f253afffd3c4065555b669b51613feafe Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 21 Jan 2025 10:08:43 +0530
Subject: [PATCH 01/26] xds: generic xds client common configs
---
xds/clients/README | 8 ++
xds/clients/config.go | 121 +++++++++++++++++++++++++++++++
xds/clients/transport_builder.go | 52 +++++++++++++
3 files changed, 181 insertions(+)
create mode 100644 xds/clients/README
create mode 100644 xds/clients/config.go
create mode 100644 xds/clients/transport_builder.go
diff --git a/xds/clients/README b/xds/clients/README
new file mode 100644
index 000000000000..f8c685c479be
--- /dev/null
+++ b/xds/clients/README
@@ -0,0 +1,8 @@
+# NOTICE
+
+This directory contains implementation details of the xDS and LRS client but
+should not be used directly. Its contents are subject to change without notice
+and are not covered by any API stability guarantees.
+
+If you are looking for a way to interact with xDS, please refer to the
+public [xds documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
diff --git a/xds/clients/config.go b/xds/clients/config.go
new file mode 100644
index 000000000000..fb0b0965b9ec
--- /dev/null
+++ b/xds/clients/config.go
@@ -0,0 +1,121 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// Package clients provides the functionality to create xDS and LRS client
+// using possible options for resource decoding and transport.
+//
+// # Experimental
+//
+// Notice: This package is EXPERIMENTAL and may be changed or removed
+// in a later release.
+package clients
+
+import (
+ "fmt"
+ "slices"
+ "strings"
+
+ v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+ "google.golang.org/protobuf/proto"
+ "google.golang.org/protobuf/types/known/structpb"
+)
+
+// ServerConfig contains the configuration to connect to an xDS management
+// server.
+type ServerConfig struct {
+ ServerURI string
+ IgnoreResourceDeletion bool
+
+ Extensions any
+}
+
+// Equal reports whether sc and other `ServerConfig` objects are considered
+// equal.
+func (sc *ServerConfig) Equal(other *ServerConfig) bool {
+ switch {
+ case sc == nil && other == nil:
+ return true
+ case sc.ServerURI != other.ServerURI:
+ return false
+ }
+ return true
+}
+
+// String returns the string representation of the `ServerConfig`.
+func (sc *ServerConfig) String() string {
+ return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
+}
+
+// Authority provides the functionality required to communicate with
+// xDS management servers corresponding to an authority.
+type Authority struct {
+ XDSServers []ServerConfig
+
+ Extensions any
+}
+
+// Node is the representation of the client node of xDS Client.
+type Node struct {
+ ID string
+ Cluster string
+ Locality Locality
+ Metadata any
+ UserAgentName string
+ UserAgentVersion string
+
+ clientFeatures []string
+}
+
+// ToProto converts the `Node` object to its protobuf representation.
+func (n Node) ToProto() *v3corepb.Node {
+ return &v3corepb.Node{
+ Id: n.ID,
+ Cluster: n.Cluster,
+ Locality: func() *v3corepb.Locality {
+ if n.Locality.IsEmpty() {
+ return nil
+ }
+ return &v3corepb.Locality{
+ Region: n.Locality.Region,
+ Zone: n.Locality.Zone,
+ SubZone: n.Locality.SubZone,
+ }
+ }(),
+ Metadata: proto.Clone(n.Metadata.(*structpb.Struct)).(*structpb.Struct),
+ UserAgentName: n.UserAgentName,
+ UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
+ ClientFeatures: slices.Clone(n.clientFeatures),
+ }
+}
+
+// Locality is the representation of the locality field within `Node`.
+type Locality struct {
+ Region string
+ Zone string
+ SubZone string
+}
+
+// IsEmpty reports whether l is considered empty.
+func (l Locality) IsEmpty() bool {
+ return l.Equal(Locality{})
+}
+
+// Equal reports whether l and other `Locality` objects are considered equal.
+func (l Locality) Equal(other Locality) bool {
+ return l.Region == other.Region && l.Zone == other.Zone && l.SubZone == other.SubZone
+}
diff --git a/xds/clients/transport_builder.go b/xds/clients/transport_builder.go
new file mode 100644
index 000000000000..5acc68e8c953
--- /dev/null
+++ b/xds/clients/transport_builder.go
@@ -0,0 +1,52 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package clients
+
+import (
+ "context"
+)
+
+// TransportBuilder is an interface for building a new xDS transport.
+type TransportBuilder interface {
+ // Build creates a new xDS transport for the provided Server Config.
+ Build(ServerConfig ServerConfig) (Transport, error)
+}
+
+// Transport provides the functionality to communicate with an xDS server using
+// streaming calls.
+type Transport interface {
+ // NewStream creates a new streaming call to the xDS server for
+ // specified method name. The returned Streaming interface can be used
+ // to send and receive messages on the stream.
+ NewStream(context.Context, string) (Stream, error)
+
+ // Close closes the underlying connection and cleans up any resources used
+ // by the Transport.
+ Close() error
+}
+
+// Stream is an interface that provides a way to send and receive
+// messages on a stream. Messages are represented as a byte slice ([]byte).
+type Stream interface {
+ // Send sends the provided message on the stream.
+ Send([]byte) error
+
+ // Recv block until the next message is received on the stream.
+ Recv() ([]byte, error)
+}
From 1bb7909e9ed471f2fd5bfb8a960ec40722cbadeb Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Wed, 22 Jan 2025 10:00:59 +0530
Subject: [PATCH 02/26] re-push comments
---
xds/clients/README | 2 +-
xds/clients/config.go | 38 +++++++++++++++++++++++++++++++++++---
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/xds/clients/README b/xds/clients/README
index f8c685c479be..0bdae776edf6 100644
--- a/xds/clients/README
+++ b/xds/clients/README
@@ -1,6 +1,6 @@
# NOTICE
-This directory contains implementation details of the xDS and LRS client but
+This directory contains implementation details of the xDS and LRS client but
should not be used directly. Its contents are subject to change without notice
and are not covered by any API stability guarantees.
diff --git a/xds/clients/config.go b/xds/clients/config.go
index fb0b0965b9ec..2a0a1dca2bf5 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -38,9 +38,30 @@ import (
// ServerConfig contains the configuration to connect to an xDS management
// server.
type ServerConfig struct {
- ServerURI string
+ // ServerURI is the server url of the xDS management server.
+ ServerURI string
+
+ // IgnoreResourceDeletion is a server feature which if set to true,
+ // indicates that resource deletion errors can be ignored and cached
+ // resource data can be used.
+ //
+ // This will be removed in future once we implement gRFC A88
+ // and two new fields `FailOnDataErrors` and
+ // `ResourceTimerIsTransientError` will be introduced.
IgnoreResourceDeletion bool
+ // Extensions for `ServerConfig` can be populated with arbitrary data to be
+ // passed to the `TransportBuilder` and/or xDS Client's `ResourceType`
+ // implementations. This field can be used to provide additional
+ // configuration or context specific to the user's needs.
+ //
+ // The xDS and LRS clients itself does not interpret the contents of this
+ // field. It is the responsibility of the user's custom `TransportBuilder`
+ // and/or `ResourceType` implementations to handle and interpret these
+ // extensions.
+ //
+ // For example, a custom TransportBuilder might use this field to configure
+ // a specific security credentials.
Extensions any
}
@@ -61,11 +82,22 @@ func (sc *ServerConfig) String() string {
return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
}
-// Authority provides the functionality required to communicate with
-// xDS management servers corresponding to an authority.
+// Authority contains configuration for an xDS control plane authority.
type Authority struct {
+ // XDSServers contains the list of server configurations for this authority.
XDSServers []ServerConfig
+ // Extensions for `Authority` can be populated with arbitrary data to be
+ // passed to the xDS Client's user specific implementations. This field
+ // can be used to provide additional configuration or context specific to
+ // the user's needs.
+ //
+ // The xDS and LRS clients itself does not interpret the contents of this
+ // field. It is the responsibility of the user's implementations to handle
+ // and interpret these extensions.
+ //
+ // For example, a custom name resolver might use this field for the name of
+ // listener resource to subscribe to.
Extensions any
}
From 03ea00d00e02e6ad42d217e5a3ec4e109bbea07f Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Wed, 22 Jan 2025 10:04:31 +0530
Subject: [PATCH 03/26] improve ServerConfig equal
---
xds/clients/config.go | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/xds/clients/config.go b/xds/clients/config.go
index 2a0a1dca2bf5..2005c8aec64b 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -68,13 +68,10 @@ type ServerConfig struct {
// Equal reports whether sc and other `ServerConfig` objects are considered
// equal.
func (sc *ServerConfig) Equal(other *ServerConfig) bool {
- switch {
- case sc == nil && other == nil:
+ if sc == nil && other == nil {
return true
- case sc.ServerURI != other.ServerURI:
- return false
}
- return true
+ return sc.ServerURI != other.ServerURI
}
// String returns the string representation of the `ServerConfig`.
From bc8d7a716aba417bfd6cacbedcd5d9bddfec4afb Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Thu, 23 Jan 2025 12:49:54 +0530
Subject: [PATCH 04/26] easwar review round 1 on documentation
---
xds/clients/README | 9 ++-
xds/clients/config.go | 117 ++++++++++++++++++++-----------
xds/clients/transport_builder.go | 10 +--
3 files changed, 86 insertions(+), 50 deletions(-)
diff --git a/xds/clients/README b/xds/clients/README
index 0bdae776edf6..60ccd464231b 100644
--- a/xds/clients/README
+++ b/xds/clients/README
@@ -1,8 +1,7 @@
-# NOTICE
-
This directory contains implementation details of the xDS and LRS client but
-should not be used directly. Its contents are subject to change without notice
-and are not covered by any API stability guarantees.
+should not be used directly. This is a work in progress and its contents are
+subject to change without notice and are not covered by any API stability
+guarantees.
If you are looking for a way to interact with xDS, please refer to the
-public [xds documentation](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
+[example](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
diff --git a/xds/clients/config.go b/xds/clients/config.go
index 2005c8aec64b..65fd966d5574 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -16,13 +16,19 @@
*
*/
-// Package clients provides the functionality to create xDS and LRS client
-// using possible options for resource decoding and transport.
+// Package clients contains implementations of the xDS and LRS clients, to be
+// used by applications to communicate with xDS management servers.
+//
+// The xDS client enable users to create client instance with in-memory
+// configurations and register watches for named resource that can be received
+// on ADS stream.
+//
+// The LRS client allows to report load through LRS Stream.
//
// # Experimental
//
// Notice: This package is EXPERIMENTAL and may be changed or removed
-// in a later release.
+// in a later release. See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md)
package clients
import (
@@ -30,51 +36,65 @@ import (
"slices"
"strings"
- v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"
+
+ v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+ "github.com/google/go-cmp/cmp"
)
// ServerConfig contains the configuration to connect to an xDS management
// server.
type ServerConfig struct {
- // ServerURI is the server url of the xDS management server.
+ // ServerURI is the target URI of the xDS management server.
ServerURI string
// IgnoreResourceDeletion is a server feature which if set to true,
// indicates that resource deletion errors can be ignored and cached
// resource data can be used.
//
- // This will be removed in future once we implement gRFC A88
- // and two new fields `FailOnDataErrors` and
- // `ResourceTimerIsTransientError` will be introduced.
+ // This will be removed in the future once we implement gRFC A88
+ // and two new fields FailOnDataErrors and
+ // ResourceTimerIsTransientError will be introduced.
IgnoreResourceDeletion bool
- // Extensions for `ServerConfig` can be populated with arbitrary data to be
- // passed to the `TransportBuilder` and/or xDS Client's `ResourceType`
- // implementations. This field can be used to provide additional
- // configuration or context specific to the user's needs.
+ // Extensions can be populated with arbitrary data to be passed to the
+ // [TransportBuilder] and/or xDS Client's ResourceType implementations.
+ // This field can be used to provide additional configuration or context
+ // specific to the user's needs.
//
- // The xDS and LRS clients itself does not interpret the contents of this
- // field. It is the responsibility of the user's custom `TransportBuilder`
- // and/or `ResourceType` implementations to handle and interpret these
+ // The xDS and LRS clients itself do not interpret the contents of this
+ // field. It is the responsibility of the user's custom [TransportBuilder]
+ // and/or ResourceType implementations to handle and interpret these
// extensions.
//
- // For example, a custom TransportBuilder might use this field to configure
- // a specific security credentials.
+ // For example, a custom [TransportBuilder] might use this field to
+ // configure a specific security credentials.
Extensions any
}
-// Equal reports whether sc and other `ServerConfig` objects are considered
-// equal.
+// Equal returns true if sc and other are considered equal.
func (sc *ServerConfig) Equal(other *ServerConfig) bool {
- if sc == nil && other == nil {
+ switch {
+ case sc == nil && other == nil:
return true
+ case (sc != nil) != (other != nil):
+ return false
+ case sc.ServerURI != other.ServerURI:
+ return false
+ case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
+ return false
+ case !cmp.Equal(sc.Extensions, other.Extensions):
+ return false
}
- return sc.ServerURI != other.ServerURI
+ return true
}
-// String returns the string representation of the `ServerConfig`.
+// String returns a string representation of the [ServerConfig].
+//
+// NOTICE: This interface is intended mainly for logging/testing purposes and
+// that the user must not assume anything about the stability of the output
+// returned from this method.
func (sc *ServerConfig) String() string {
return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
}
@@ -84,12 +104,12 @@ type Authority struct {
// XDSServers contains the list of server configurations for this authority.
XDSServers []ServerConfig
- // Extensions for `Authority` can be populated with arbitrary data to be
- // passed to the xDS Client's user specific implementations. This field
- // can be used to provide additional configuration or context specific to
- // the user's needs.
+ // Extensions can be populated with arbitrary data to be passed to the xDS
+ // Client's user specific implementations. This field can be used to
+ // provide additional configuration or context specific to the user's
+ // needs.
//
- // The xDS and LRS clients itself does not interpret the contents of this
+ // The xDS and LRS clients itself do not interpret the contents of this
// field. It is the responsibility of the user's implementations to handle
// and interpret these extensions.
//
@@ -98,19 +118,31 @@ type Authority struct {
Extensions any
}
-// Node is the representation of the client node of xDS Client.
+// Node represents the node of the xDS client for management servers to
+// identify the application making the request.
type Node struct {
- ID string
- Cluster string
- Locality Locality
- Metadata any
- UserAgentName string
+ // ID is a string identifier of the node.
+ ID string
+ // Cluster is the name of the cluster the node belongs to.
+ Cluster string
+ // Locality is the location of the node including region, zone, sub-zone.
+ Locality Locality
+ // Metadata is any arbitrary values associated with the node to provide
+ // additional context.
+ Metadata any
+ // UserAgentName is the user agent name. It is typically set to a hardcoded
+ // constant such as grpc-go.
+ UserAgentName string
+ // UserAgentVersion is the user agent version. It is typically set to the
+ // version of the library.
UserAgentVersion string
-
- clientFeatures []string
+ // ClientFeatures is a list of features supported by this client. These are
+ // typically hardcoded within the xDS client, but may be overridden for
+ // testing purposes.
+ ClientFeatures []string
}
-// ToProto converts the `Node` object to its protobuf representation.
+// ToProto converts the [Node] object to its protobuf representation.
func (n Node) ToProto() *v3corepb.Node {
return &v3corepb.Node{
Id: n.ID,
@@ -128,14 +160,17 @@ func (n Node) ToProto() *v3corepb.Node {
Metadata: proto.Clone(n.Metadata.(*structpb.Struct)).(*structpb.Struct),
UserAgentName: n.UserAgentName,
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
- ClientFeatures: slices.Clone(n.clientFeatures),
+ ClientFeatures: slices.Clone(n.ClientFeatures),
}
}
-// Locality is the representation of the locality field within `Node`.
+// Locality represents the location of the xDS client node.
type Locality struct {
- Region string
- Zone string
+ // Region is the region of the xDS client node.
+ Region string
+ // Zone is the area within a region.
+ Zone string
+ // SubZone is the further subdivision within a sub-zone.
SubZone string
}
@@ -144,7 +179,7 @@ func (l Locality) IsEmpty() bool {
return l.Equal(Locality{})
}
-// Equal reports whether l and other `Locality` objects are considered equal.
+// Equal returns true if l and other are considered equal.
func (l Locality) Equal(other Locality) bool {
return l.Region == other.Region && l.Zone == other.Zone && l.SubZone == other.SubZone
}
diff --git a/xds/clients/transport_builder.go b/xds/clients/transport_builder.go
index 5acc68e8c953..3b4b4675bffb 100644
--- a/xds/clients/transport_builder.go
+++ b/xds/clients/transport_builder.go
@@ -22,9 +22,11 @@ import (
"context"
)
-// TransportBuilder is an interface for building a new xDS transport.
+// TransportBuilder is an interface to create communication channel to an xDS
+// management server.
type TransportBuilder interface {
- // Build creates a new xDS transport for the provided Server Config.
+ // Build creates a new [Transport] instance to the xDS server based on the
+ // provided Server Config.
Build(ServerConfig ServerConfig) (Transport, error)
}
@@ -32,7 +34,7 @@ type TransportBuilder interface {
// streaming calls.
type Transport interface {
// NewStream creates a new streaming call to the xDS server for
- // specified method name. The returned Streaming interface can be used
+ // specified RPC method name. The returned Streaming interface can be used
// to send and receive messages on the stream.
NewStream(context.Context, string) (Stream, error)
@@ -47,6 +49,6 @@ type Stream interface {
// Send sends the provided message on the stream.
Send([]byte) error
- // Recv block until the next message is received on the stream.
+ // Recv blocks until the next message is received on the stream.
Recv() ([]byte, error)
}
From 053ff95c0dcf01bdda52d14d938c3db6da323993 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Fri, 24 Jan 2025 11:03:17 +0530
Subject: [PATCH 05/26] easwar comments on docstrings
---
xds/clients/README | 7 +--
xds/clients/config.go | 96 +++++++++++++++++---------------
xds/clients/transport_builder.go | 17 +++---
3 files changed, 62 insertions(+), 58 deletions(-)
diff --git a/xds/clients/README b/xds/clients/README
index 60ccd464231b..d7dbc3f1cfa4 100644
--- a/xds/clients/README
+++ b/xds/clients/README
@@ -1,7 +1,6 @@
-This directory contains implementation details of the xDS and LRS client but
-should not be used directly. This is a work in progress and its contents are
-subject to change without notice and are not covered by any API stability
-guarantees.
+This directory holds internal code for the xDS and LRS clients and is not meant
+for direct use. It's under active development and may change significantly.
+There are no API stability guarantees at this time.
If you are looking for a way to interact with xDS, please refer to the
[example](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
diff --git a/xds/clients/config.go b/xds/clients/config.go
index 65fd966d5574..6e3cfcb4302b 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -16,19 +16,33 @@
*
*/
-// Package clients contains implementations of the xDS and LRS clients, to be
-// used by applications to communicate with xDS management servers.
+// Package clients provides implementations of the xDS and LRS clients,
+// enabling applications to communicate with xDS management servers and report
+// load.
//
-// The xDS client enable users to create client instance with in-memory
-// configurations and register watches for named resource that can be received
-// on ADS stream.
+// xDS Client
//
-// The LRS client allows to report load through LRS Stream.
+// The xDS client allows applications to:
+// - Create client instances with in-memory configurations.
+// - Register watches for named resources.
+// - Receive resources via the ADS (Aggregated Discovery Service) stream.
+//
+// This enables applications to dynamically discover and configure resources
+// such as listeners, routes, clusters, and endpoints from an xDS management
+// server.
+//
+// # LRS Client
+//
+// The LRS (Load Reporting Service) client allows applications to report load
+// data to an LRS server via the LRS stream. This data can be used for
+// monitoring, traffic management, and other purposes.
//
// # Experimental
//
-// Notice: This package is EXPERIMENTAL and may be changed or removed
-// in a later release. See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md)
+// NOTICE: This package is EXPERIMENTAL and may be changed or removed
+// in a later release.
+//
+// See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md).
package clients
import (
@@ -40,11 +54,9 @@ import (
"google.golang.org/protobuf/types/known/structpb"
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
- "github.com/google/go-cmp/cmp"
)
-// ServerConfig contains the configuration to connect to an xDS management
-// server.
+// ServerConfig holds settings for connecting to an xDS management server.
type ServerConfig struct {
// ServerURI is the target URI of the xDS management server.
ServerURI string
@@ -63,10 +75,9 @@ type ServerConfig struct {
// This field can be used to provide additional configuration or context
// specific to the user's needs.
//
- // The xDS and LRS clients itself do not interpret the contents of this
- // field. It is the responsibility of the user's custom [TransportBuilder]
- // and/or ResourceType implementations to handle and interpret these
- // extensions.
+ // The xDS and LRS clients do not interpret the contents of this field.
+ // It is the responsibility of the user's custom [TransportBuilder] and/or
+ // ResourceType implementations to handle and interpret these extensions.
//
// For example, a custom [TransportBuilder] might use this field to
// configure a specific security credentials.
@@ -84,17 +95,18 @@ func (sc *ServerConfig) Equal(other *ServerConfig) bool {
return false
case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
return false
- case !cmp.Equal(sc.Extensions, other.Extensions):
- return false
}
- return true
+ if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
+ return true
+ }
+ return false
}
// String returns a string representation of the [ServerConfig].
//
-// NOTICE: This interface is intended mainly for logging/testing purposes and
-// that the user must not assume anything about the stability of the output
-// returned from this method.
+// WARNING: This method is primarily intended for logging and testing
+// purposes. The output returned by this method is not guaranteed to be stable
+// and may change at any time. Do not rely on it for production use.
func (sc *ServerConfig) String() string {
return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
}
@@ -109,40 +121,34 @@ type Authority struct {
// provide additional configuration or context specific to the user's
// needs.
//
- // The xDS and LRS clients itself do not interpret the contents of this
- // field. It is the responsibility of the user's implementations to handle
- // and interpret these extensions.
- //
- // For example, a custom name resolver might use this field for the name of
- // listener resource to subscribe to.
+ // The xDS and LRS clients do not interpret the contents of this field. It
+ // is the responsibility of the user's implementations to handle and
+ // interpret these extensions.
Extensions any
}
-// Node represents the node of the xDS client for management servers to
-// identify the application making the request.
+// Node represents the identity of the xDS client, allowing
+// management servers to identify the source of xDS requests.
type Node struct {
- // ID is a string identifier of the node.
+ // ID is a string identifier of the application.
ID string
- // Cluster is the name of the cluster the node belongs to.
+ // Cluster is the name of the cluster the application belongs to.
Cluster string
- // Locality is the location of the node including region, zone, sub-zone.
+ // Locality is the location of the application including region, zone,
+ // sub-zone.
Locality Locality
- // Metadata is any arbitrary values associated with the node to provide
- // additional context.
+ // Metadata provides additional context about the application by associating
+ // arbitrary key-value pairs with it.
Metadata any
- // UserAgentName is the user agent name. It is typically set to a hardcoded
- // constant such as grpc-go.
+ // UserAgentName is the user agent name of application.
UserAgentName string
- // UserAgentVersion is the user agent version. It is typically set to the
- // version of the library.
+ // UserAgentVersion is the user agent version of application.
UserAgentVersion string
- // ClientFeatures is a list of features supported by this client. These are
- // typically hardcoded within the xDS client, but may be overridden for
- // testing purposes.
+ // ClientFeatures is a list of xDS features supported by this client.
ClientFeatures []string
}
-// ToProto converts the [Node] object to its protobuf representation.
+// ToProto converts an instance of [Node] to its protobuf representation.
func (n Node) ToProto() *v3corepb.Node {
return &v3corepb.Node{
Id: n.ID,
@@ -164,13 +170,13 @@ func (n Node) ToProto() *v3corepb.Node {
}
}
-// Locality represents the location of the xDS client node.
+// Locality represents the location of the xDS client application.
type Locality struct {
- // Region is the region of the xDS client node.
+ // Region is the region of the xDS client application.
Region string
// Zone is the area within a region.
Zone string
- // SubZone is the further subdivision within a sub-zone.
+ // SubZone is the further subdivision within a zone.
SubZone string
}
diff --git a/xds/clients/transport_builder.go b/xds/clients/transport_builder.go
index 3b4b4675bffb..5b0eb10cbd3c 100644
--- a/xds/clients/transport_builder.go
+++ b/xds/clients/transport_builder.go
@@ -22,29 +22,28 @@ import (
"context"
)
-// TransportBuilder is an interface to create communication channel to an xDS
-// management server.
+// TransportBuilder provides the functionality to create a communication
+// channel to an xDS management server.
type TransportBuilder interface {
// Build creates a new [Transport] instance to the xDS server based on the
- // provided Server Config.
+ // provided ServerConfig.
Build(ServerConfig ServerConfig) (Transport, error)
}
// Transport provides the functionality to communicate with an xDS server using
// streaming calls.
type Transport interface {
- // NewStream creates a new streaming call to the xDS server for
- // specified RPC method name. The returned Streaming interface can be used
+ // NewStream creates a new streaming call to the xDS server for the
+ // specified RPC method name. The returned Stream interface can be used
// to send and receive messages on the stream.
NewStream(context.Context, string) (Stream, error)
- // Close closes the underlying connection and cleans up any resources used
- // by the Transport.
+ // Close closes the Transport.
Close() error
}
-// Stream is an interface that provides a way to send and receive
-// messages on a stream. Messages are represented as a byte slice ([]byte).
+// Stream provides methods to send and receive messages on a stream. Messages
+// are represented as a byte slice ([]byte).
type Stream interface {
// Send sends the provided message on the stream.
Send([]byte) error
From 4cd4ef1e31e3591935da1ac6be9daa377af87137 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 28 Jan 2025 11:14:30 +0530
Subject: [PATCH 06/26] easwar comments round 4
---
xds/clients/README | 6 +++---
xds/clients/config.go | 11 +++++++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/xds/clients/README b/xds/clients/README
index d7dbc3f1cfa4..f5b65eef55b5 100644
--- a/xds/clients/README
+++ b/xds/clients/README
@@ -1,6 +1,6 @@
-This directory holds internal code for the xDS and LRS clients and is not meant
-for direct use. It's under active development and may change significantly.
-There are no API stability guarantees at this time.
+This directory contains the implementation of the xDS and LRS clients and is
+not meant for direct use. It's under active development and may change
+significantly. There are no API stability guarantees at this time.
If you are looking for a way to interact with xDS, please refer to the
[example](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
diff --git a/xds/clients/config.go b/xds/clients/config.go
index 6e3cfcb4302b..87d498388a62 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -81,6 +81,9 @@ type ServerConfig struct {
//
// For example, a custom [TransportBuilder] might use this field to
// configure a specific security credentials.
+ //
+ // Note: For custom types used in Extensions, ensure an Equal(any) bool
+ // method is implemented for equality checks on ServerConfig.
Extensions any
}
@@ -96,6 +99,12 @@ func (sc *ServerConfig) Equal(other *ServerConfig) bool {
case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
return false
}
+ if sc.Extensions == nil && other.Extensions == nil {
+ return true
+ }
+ if sc.Extensions == nil || other.Extensions == nil {
+ return false
+ }
if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
return true
}
@@ -145,6 +154,8 @@ type Node struct {
// UserAgentVersion is the user agent version of application.
UserAgentVersion string
// ClientFeatures is a list of xDS features supported by this client.
+ // These features are set within the xDS client, but may be overridden only
+ // for testing purposes.
ClientFeatures []string
}
From 49f1e3b387da7fbe955aac98158f4456ec8c63b2 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 28 Jan 2025 15:38:13 +0530
Subject: [PATCH 07/26] config tests
---
xds/clients/config.go | 13 +-
xds/clients/config_test.go | 327 +++++++++++++++++++++++++++++++++++++
2 files changed, 336 insertions(+), 4 deletions(-)
create mode 100644 xds/clients/config_test.go
diff --git a/xds/clients/config.go b/xds/clients/config.go
index 87d498388a62..77f18ebcdd2d 100644
--- a/xds/clients/config.go
+++ b/xds/clients/config.go
@@ -102,9 +102,6 @@ func (sc *ServerConfig) Equal(other *ServerConfig) bool {
if sc.Extensions == nil && other.Extensions == nil {
return true
}
- if sc.Extensions == nil || other.Extensions == nil {
- return false
- }
if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
return true
}
@@ -174,7 +171,15 @@ func (n Node) ToProto() *v3corepb.Node {
SubZone: n.Locality.SubZone,
}
}(),
- Metadata: proto.Clone(n.Metadata.(*structpb.Struct)).(*structpb.Struct),
+ Metadata: func() *structpb.Struct {
+ if n.Metadata == nil {
+ return nil
+ }
+ if md, ok := n.Metadata.(*structpb.Struct); ok {
+ return proto.Clone(md).(*structpb.Struct)
+ }
+ return nil
+ }(),
UserAgentName: n.UserAgentName,
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
ClientFeatures: slices.Clone(n.ClientFeatures),
diff --git a/xds/clients/config_test.go b/xds/clients/config_test.go
new file mode 100644
index 000000000000..b0e9d99e97c7
--- /dev/null
+++ b/xds/clients/config_test.go
@@ -0,0 +1,327 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package clients
+
+import (
+ "testing"
+
+ v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+ "github.com/google/go-cmp/cmp"
+ "google.golang.org/grpc/internal/grpctest"
+ "google.golang.org/protobuf/testing/protocmp"
+ "google.golang.org/protobuf/types/known/structpb"
+)
+
+type s struct {
+ grpctest.Tester
+}
+
+func Test(t *testing.T) {
+ grpctest.RunSubTests(t, s{})
+}
+
+type testServerConfigExtension struct{ x int }
+
+func (ts testServerConfigExtension) Equal(other any) bool {
+ ots, ok := other.(testServerConfigExtension)
+ if !ok {
+ return false
+ }
+ return ts.x == ots.x
+}
+
+func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct {
+ t.Helper()
+
+ ret, err := structpb.NewStruct(input)
+ if err != nil {
+ t.Fatalf("Failed to create new struct proto from map %v: %v", input, err)
+ }
+ return ret
+}
+
+func (s) TestServerConfig_Equal(t *testing.T) {
+ tests := []struct {
+ name string
+ s1 *ServerConfig
+ s2 *ServerConfig
+ wantEq bool
+ }{
+ {
+ name: "both nil",
+ s1: nil,
+ s2: nil,
+ wantEq: true,
+ },
+ {
+ name: "s1 nil",
+ s1: nil,
+ s2: &ServerConfig{},
+ wantEq: false,
+ },
+ {
+ name: "s2 nil",
+ s1: &ServerConfig{},
+ s2: nil,
+ wantEq: false,
+ },
+ {
+ name: "empty, equal",
+ s1: &ServerConfig{},
+ s2: &ServerConfig{},
+ wantEq: true,
+ },
+ {
+ name: "ServerURI different",
+ s1: &ServerConfig{ServerURI: "foo"},
+ s2: &ServerConfig{ServerURI: "bar"},
+ wantEq: false,
+ },
+ {
+ name: "IgnoreResourceDeletion different",
+ s1: &ServerConfig{IgnoreResourceDeletion: true},
+ s2: &ServerConfig{},
+ wantEq: false,
+ },
+ {
+ name: "Extensions different, no Equal method",
+ s1: &ServerConfig{
+ Extensions: 1,
+ },
+ s2: &ServerConfig{
+ Extensions: 2,
+ },
+ wantEq: false, // By default, if there's no Equal method, they are unequal
+ },
+ {
+ name: "Extensions same, no Equal method",
+ s1: &ServerConfig{
+ Extensions: 1,
+ },
+ s2: &ServerConfig{
+ Extensions: 1,
+ },
+ wantEq: false, // By default, if there's no Equal method, they are unequal
+ },
+ {
+ name: "Extensions different, with Equal method",
+ s1: &ServerConfig{
+ Extensions: testServerConfigExtension{1},
+ },
+ s2: &ServerConfig{
+ Extensions: testServerConfigExtension{2},
+ },
+ wantEq: false,
+ },
+ {
+ name: "Extensions same, with Equal method",
+ s1: &ServerConfig{
+ Extensions: testServerConfigExtension{1},
+ },
+ s2: &ServerConfig{
+ Extensions: testServerConfigExtension{1},
+ },
+ wantEq: true,
+ },
+ {
+ name: "all same",
+ s1: &ServerConfig{
+ ServerURI: "foo",
+ IgnoreResourceDeletion: true,
+ Extensions: testServerConfigExtension{1},
+ },
+ s2: &ServerConfig{
+ ServerURI: "foo",
+ IgnoreResourceDeletion: true,
+ Extensions: testServerConfigExtension{1},
+ },
+ wantEq: true,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ if gotEq := test.s1.Equal(test.s2); gotEq != test.wantEq {
+ t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
+ }
+ })
+ }
+}
+
+func (s) TestLocality_IsEmpty(t *testing.T) {
+ tests := []struct {
+ name string
+ locality Locality
+ want bool
+ }{
+ {
+ name: "empty locality",
+ locality: Locality{},
+ want: true,
+ },
+ {
+ name: "non-empty region",
+ locality: Locality{Region: "region"},
+ want: false,
+ },
+ {
+ name: "non-empty zone",
+ locality: Locality{Zone: "zone"},
+ want: false,
+ },
+ {
+ name: "non-empty subzone",
+ locality: Locality{SubZone: "subzone"},
+ want: false,
+ },
+ {
+ name: "non-empty all",
+ locality: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ want: false,
+ },
+ }
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ if got := test.locality.IsEmpty(); got != test.want {
+ t.Errorf("IsEmpty() = %v, want %v", got, test.want)
+ }
+ })
+ }
+}
+
+func (s) TestLocality_Equal(t *testing.T) {
+ tests := []struct {
+ name string
+ l1 Locality
+ l2 Locality
+ wantEq bool
+ }{
+ {
+ name: "equal localities",
+ l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ wantEq: true,
+ },
+ {
+ name: "different regions",
+ l1: Locality{Region: "region1", Zone: "zone", SubZone: "subzone"},
+ l2: Locality{Region: "region2", Zone: "zone", SubZone: "subzone"},
+ wantEq: false,
+ },
+
+ {
+ name: "different zones",
+ l1: Locality{Region: "region", Zone: "zone1", SubZone: "subzone"},
+ l2: Locality{Region: "region", Zone: "zone2", SubZone: "subzone"},
+ wantEq: false,
+ },
+ {
+ name: "different subzones",
+ l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone1"},
+ l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone2"},
+ wantEq: false,
+ },
+ {
+ name: "empty vs non-empty",
+ l1: Locality{},
+ l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ wantEq: false,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ if gotEq := test.l1.Equal(test.l2); gotEq != test.wantEq {
+ t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
+ }
+ })
+ }
+}
+
+func (s) TestNode_ToProto(t *testing.T) {
+ tests := []struct {
+ desc string
+ inputNode Node
+ wantProto *v3corepb.Node
+ }{
+ {
+ desc: "all fields set",
+ inputNode: Node{
+ ID: "id",
+ Cluster: "cluster",
+ Locality: Locality{
+ Region: "region",
+ Zone: "zone",
+ SubZone: "sub_zone",
+ },
+ Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
+ UserAgentName: "user agent",
+ UserAgentVersion: "version",
+ ClientFeatures: []string{"feature1", "feature2"},
+ },
+ wantProto: &v3corepb.Node{
+ Id: "id",
+ Cluster: "cluster",
+ Locality: &v3corepb.Locality{
+ Region: "region",
+ Zone: "zone",
+ SubZone: "sub_zone",
+ },
+ Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
+ UserAgentName: "user agent",
+ UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: "version"},
+ ClientFeatures: []string{"feature1", "feature2"},
+ },
+ },
+ {
+ desc: "some fields unset",
+ inputNode: Node{
+ ID: "id",
+ },
+ wantProto: &v3corepb.Node{
+ Id: "id",
+ UserAgentName: "",
+ UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
+ ClientFeatures: nil,
+ },
+ },
+ {
+ desc: "empty locality",
+ inputNode: Node{
+ ID: "id",
+ Locality: Locality{},
+ },
+ wantProto: &v3corepb.Node{
+ Id: "id",
+ UserAgentName: "",
+ UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
+ ClientFeatures: nil,
+ },
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.desc, func(t *testing.T) {
+ gotProto := test.inputNode.ToProto()
+ if diff := cmp.Diff(test.wantProto, gotProto, protocmp.Transform()); diff != "" {
+ t.Fatalf("Unexpected diff in node proto: (-want, +got):\n%s", diff)
+ }
+ })
+ }
+}
From 835f9e4c9cc5c54765a61e244d05164d837f202b Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Fri, 31 Jan 2025 21:43:40 +0530
Subject: [PATCH 08/26] merge with previous pr
---
xds/clients/README | 6 -
xds/clients/config.go | 207 -------------------
xds/clients/config_test.go | 327 -------------------------------
xds/clients/transport_builder.go | 53 -----
4 files changed, 593 deletions(-)
delete mode 100644 xds/clients/README
delete mode 100644 xds/clients/config.go
delete mode 100644 xds/clients/config_test.go
delete mode 100644 xds/clients/transport_builder.go
diff --git a/xds/clients/README b/xds/clients/README
deleted file mode 100644
index f5b65eef55b5..000000000000
--- a/xds/clients/README
+++ /dev/null
@@ -1,6 +0,0 @@
-This directory contains the implementation of the xDS and LRS clients and is
-not meant for direct use. It's under active development and may change
-significantly. There are no API stability guarantees at this time.
-
-If you are looking for a way to interact with xDS, please refer to the
-[example](https://github.com/grpc/grpc-go/blob/master/examples/features/xds/README.md).
diff --git a/xds/clients/config.go b/xds/clients/config.go
deleted file mode 100644
index 77f18ebcdd2d..000000000000
--- a/xds/clients/config.go
+++ /dev/null
@@ -1,207 +0,0 @@
-/*
- *
- * Copyright 2024 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-// Package clients provides implementations of the xDS and LRS clients,
-// enabling applications to communicate with xDS management servers and report
-// load.
-//
-// xDS Client
-//
-// The xDS client allows applications to:
-// - Create client instances with in-memory configurations.
-// - Register watches for named resources.
-// - Receive resources via the ADS (Aggregated Discovery Service) stream.
-//
-// This enables applications to dynamically discover and configure resources
-// such as listeners, routes, clusters, and endpoints from an xDS management
-// server.
-//
-// # LRS Client
-//
-// The LRS (Load Reporting Service) client allows applications to report load
-// data to an LRS server via the LRS stream. This data can be used for
-// monitoring, traffic management, and other purposes.
-//
-// # Experimental
-//
-// NOTICE: This package is EXPERIMENTAL and may be changed or removed
-// in a later release.
-//
-// See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md).
-package clients
-
-import (
- "fmt"
- "slices"
- "strings"
-
- "google.golang.org/protobuf/proto"
- "google.golang.org/protobuf/types/known/structpb"
-
- v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-)
-
-// ServerConfig holds settings for connecting to an xDS management server.
-type ServerConfig struct {
- // ServerURI is the target URI of the xDS management server.
- ServerURI string
-
- // IgnoreResourceDeletion is a server feature which if set to true,
- // indicates that resource deletion errors can be ignored and cached
- // resource data can be used.
- //
- // This will be removed in the future once we implement gRFC A88
- // and two new fields FailOnDataErrors and
- // ResourceTimerIsTransientError will be introduced.
- IgnoreResourceDeletion bool
-
- // Extensions can be populated with arbitrary data to be passed to the
- // [TransportBuilder] and/or xDS Client's ResourceType implementations.
- // This field can be used to provide additional configuration or context
- // specific to the user's needs.
- //
- // The xDS and LRS clients do not interpret the contents of this field.
- // It is the responsibility of the user's custom [TransportBuilder] and/or
- // ResourceType implementations to handle and interpret these extensions.
- //
- // For example, a custom [TransportBuilder] might use this field to
- // configure a specific security credentials.
- //
- // Note: For custom types used in Extensions, ensure an Equal(any) bool
- // method is implemented for equality checks on ServerConfig.
- Extensions any
-}
-
-// Equal returns true if sc and other are considered equal.
-func (sc *ServerConfig) Equal(other *ServerConfig) bool {
- switch {
- case sc == nil && other == nil:
- return true
- case (sc != nil) != (other != nil):
- return false
- case sc.ServerURI != other.ServerURI:
- return false
- case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
- return false
- }
- if sc.Extensions == nil && other.Extensions == nil {
- return true
- }
- if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
- return true
- }
- return false
-}
-
-// String returns a string representation of the [ServerConfig].
-//
-// WARNING: This method is primarily intended for logging and testing
-// purposes. The output returned by this method is not guaranteed to be stable
-// and may change at any time. Do not rely on it for production use.
-func (sc *ServerConfig) String() string {
- return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
-}
-
-// Authority contains configuration for an xDS control plane authority.
-type Authority struct {
- // XDSServers contains the list of server configurations for this authority.
- XDSServers []ServerConfig
-
- // Extensions can be populated with arbitrary data to be passed to the xDS
- // Client's user specific implementations. This field can be used to
- // provide additional configuration or context specific to the user's
- // needs.
- //
- // The xDS and LRS clients do not interpret the contents of this field. It
- // is the responsibility of the user's implementations to handle and
- // interpret these extensions.
- Extensions any
-}
-
-// Node represents the identity of the xDS client, allowing
-// management servers to identify the source of xDS requests.
-type Node struct {
- // ID is a string identifier of the application.
- ID string
- // Cluster is the name of the cluster the application belongs to.
- Cluster string
- // Locality is the location of the application including region, zone,
- // sub-zone.
- Locality Locality
- // Metadata provides additional context about the application by associating
- // arbitrary key-value pairs with it.
- Metadata any
- // UserAgentName is the user agent name of application.
- UserAgentName string
- // UserAgentVersion is the user agent version of application.
- UserAgentVersion string
- // ClientFeatures is a list of xDS features supported by this client.
- // These features are set within the xDS client, but may be overridden only
- // for testing purposes.
- ClientFeatures []string
-}
-
-// ToProto converts an instance of [Node] to its protobuf representation.
-func (n Node) ToProto() *v3corepb.Node {
- return &v3corepb.Node{
- Id: n.ID,
- Cluster: n.Cluster,
- Locality: func() *v3corepb.Locality {
- if n.Locality.IsEmpty() {
- return nil
- }
- return &v3corepb.Locality{
- Region: n.Locality.Region,
- Zone: n.Locality.Zone,
- SubZone: n.Locality.SubZone,
- }
- }(),
- Metadata: func() *structpb.Struct {
- if n.Metadata == nil {
- return nil
- }
- if md, ok := n.Metadata.(*structpb.Struct); ok {
- return proto.Clone(md).(*structpb.Struct)
- }
- return nil
- }(),
- UserAgentName: n.UserAgentName,
- UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
- ClientFeatures: slices.Clone(n.ClientFeatures),
- }
-}
-
-// Locality represents the location of the xDS client application.
-type Locality struct {
- // Region is the region of the xDS client application.
- Region string
- // Zone is the area within a region.
- Zone string
- // SubZone is the further subdivision within a zone.
- SubZone string
-}
-
-// IsEmpty reports whether l is considered empty.
-func (l Locality) IsEmpty() bool {
- return l.Equal(Locality{})
-}
-
-// Equal returns true if l and other are considered equal.
-func (l Locality) Equal(other Locality) bool {
- return l.Region == other.Region && l.Zone == other.Zone && l.SubZone == other.SubZone
-}
diff --git a/xds/clients/config_test.go b/xds/clients/config_test.go
deleted file mode 100644
index b0e9d99e97c7..000000000000
--- a/xds/clients/config_test.go
+++ /dev/null
@@ -1,327 +0,0 @@
-/*
- *
- * Copyright 2024 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-package clients
-
-import (
- "testing"
-
- v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
- "github.com/google/go-cmp/cmp"
- "google.golang.org/grpc/internal/grpctest"
- "google.golang.org/protobuf/testing/protocmp"
- "google.golang.org/protobuf/types/known/structpb"
-)
-
-type s struct {
- grpctest.Tester
-}
-
-func Test(t *testing.T) {
- grpctest.RunSubTests(t, s{})
-}
-
-type testServerConfigExtension struct{ x int }
-
-func (ts testServerConfigExtension) Equal(other any) bool {
- ots, ok := other.(testServerConfigExtension)
- if !ok {
- return false
- }
- return ts.x == ots.x
-}
-
-func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct {
- t.Helper()
-
- ret, err := structpb.NewStruct(input)
- if err != nil {
- t.Fatalf("Failed to create new struct proto from map %v: %v", input, err)
- }
- return ret
-}
-
-func (s) TestServerConfig_Equal(t *testing.T) {
- tests := []struct {
- name string
- s1 *ServerConfig
- s2 *ServerConfig
- wantEq bool
- }{
- {
- name: "both nil",
- s1: nil,
- s2: nil,
- wantEq: true,
- },
- {
- name: "s1 nil",
- s1: nil,
- s2: &ServerConfig{},
- wantEq: false,
- },
- {
- name: "s2 nil",
- s1: &ServerConfig{},
- s2: nil,
- wantEq: false,
- },
- {
- name: "empty, equal",
- s1: &ServerConfig{},
- s2: &ServerConfig{},
- wantEq: true,
- },
- {
- name: "ServerURI different",
- s1: &ServerConfig{ServerURI: "foo"},
- s2: &ServerConfig{ServerURI: "bar"},
- wantEq: false,
- },
- {
- name: "IgnoreResourceDeletion different",
- s1: &ServerConfig{IgnoreResourceDeletion: true},
- s2: &ServerConfig{},
- wantEq: false,
- },
- {
- name: "Extensions different, no Equal method",
- s1: &ServerConfig{
- Extensions: 1,
- },
- s2: &ServerConfig{
- Extensions: 2,
- },
- wantEq: false, // By default, if there's no Equal method, they are unequal
- },
- {
- name: "Extensions same, no Equal method",
- s1: &ServerConfig{
- Extensions: 1,
- },
- s2: &ServerConfig{
- Extensions: 1,
- },
- wantEq: false, // By default, if there's no Equal method, they are unequal
- },
- {
- name: "Extensions different, with Equal method",
- s1: &ServerConfig{
- Extensions: testServerConfigExtension{1},
- },
- s2: &ServerConfig{
- Extensions: testServerConfigExtension{2},
- },
- wantEq: false,
- },
- {
- name: "Extensions same, with Equal method",
- s1: &ServerConfig{
- Extensions: testServerConfigExtension{1},
- },
- s2: &ServerConfig{
- Extensions: testServerConfigExtension{1},
- },
- wantEq: true,
- },
- {
- name: "all same",
- s1: &ServerConfig{
- ServerURI: "foo",
- IgnoreResourceDeletion: true,
- Extensions: testServerConfigExtension{1},
- },
- s2: &ServerConfig{
- ServerURI: "foo",
- IgnoreResourceDeletion: true,
- Extensions: testServerConfigExtension{1},
- },
- wantEq: true,
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- if gotEq := test.s1.Equal(test.s2); gotEq != test.wantEq {
- t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
- }
- })
- }
-}
-
-func (s) TestLocality_IsEmpty(t *testing.T) {
- tests := []struct {
- name string
- locality Locality
- want bool
- }{
- {
- name: "empty locality",
- locality: Locality{},
- want: true,
- },
- {
- name: "non-empty region",
- locality: Locality{Region: "region"},
- want: false,
- },
- {
- name: "non-empty zone",
- locality: Locality{Zone: "zone"},
- want: false,
- },
- {
- name: "non-empty subzone",
- locality: Locality{SubZone: "subzone"},
- want: false,
- },
- {
- name: "non-empty all",
- locality: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
- want: false,
- },
- }
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- if got := test.locality.IsEmpty(); got != test.want {
- t.Errorf("IsEmpty() = %v, want %v", got, test.want)
- }
- })
- }
-}
-
-func (s) TestLocality_Equal(t *testing.T) {
- tests := []struct {
- name string
- l1 Locality
- l2 Locality
- wantEq bool
- }{
- {
- name: "equal localities",
- l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
- wantEq: true,
- },
- {
- name: "different regions",
- l1: Locality{Region: "region1", Zone: "zone", SubZone: "subzone"},
- l2: Locality{Region: "region2", Zone: "zone", SubZone: "subzone"},
- wantEq: false,
- },
-
- {
- name: "different zones",
- l1: Locality{Region: "region", Zone: "zone1", SubZone: "subzone"},
- l2: Locality{Region: "region", Zone: "zone2", SubZone: "subzone"},
- wantEq: false,
- },
- {
- name: "different subzones",
- l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone1"},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone2"},
- wantEq: false,
- },
- {
- name: "empty vs non-empty",
- l1: Locality{},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
- wantEq: false,
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- if gotEq := test.l1.Equal(test.l2); gotEq != test.wantEq {
- t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
- }
- })
- }
-}
-
-func (s) TestNode_ToProto(t *testing.T) {
- tests := []struct {
- desc string
- inputNode Node
- wantProto *v3corepb.Node
- }{
- {
- desc: "all fields set",
- inputNode: Node{
- ID: "id",
- Cluster: "cluster",
- Locality: Locality{
- Region: "region",
- Zone: "zone",
- SubZone: "sub_zone",
- },
- Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
- UserAgentName: "user agent",
- UserAgentVersion: "version",
- ClientFeatures: []string{"feature1", "feature2"},
- },
- wantProto: &v3corepb.Node{
- Id: "id",
- Cluster: "cluster",
- Locality: &v3corepb.Locality{
- Region: "region",
- Zone: "zone",
- SubZone: "sub_zone",
- },
- Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
- UserAgentName: "user agent",
- UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: "version"},
- ClientFeatures: []string{"feature1", "feature2"},
- },
- },
- {
- desc: "some fields unset",
- inputNode: Node{
- ID: "id",
- },
- wantProto: &v3corepb.Node{
- Id: "id",
- UserAgentName: "",
- UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
- ClientFeatures: nil,
- },
- },
- {
- desc: "empty locality",
- inputNode: Node{
- ID: "id",
- Locality: Locality{},
- },
- wantProto: &v3corepb.Node{
- Id: "id",
- UserAgentName: "",
- UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
- ClientFeatures: nil,
- },
- },
- }
-
- for _, test := range tests {
- t.Run(test.desc, func(t *testing.T) {
- gotProto := test.inputNode.ToProto()
- if diff := cmp.Diff(test.wantProto, gotProto, protocmp.Transform()); diff != "" {
- t.Fatalf("Unexpected diff in node proto: (-want, +got):\n%s", diff)
- }
- })
- }
-}
diff --git a/xds/clients/transport_builder.go b/xds/clients/transport_builder.go
deleted file mode 100644
index 5b0eb10cbd3c..000000000000
--- a/xds/clients/transport_builder.go
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- *
- * Copyright 2024 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-package clients
-
-import (
- "context"
-)
-
-// TransportBuilder provides the functionality to create a communication
-// channel to an xDS management server.
-type TransportBuilder interface {
- // Build creates a new [Transport] instance to the xDS server based on the
- // provided ServerConfig.
- Build(ServerConfig ServerConfig) (Transport, error)
-}
-
-// Transport provides the functionality to communicate with an xDS server using
-// streaming calls.
-type Transport interface {
- // NewStream creates a new streaming call to the xDS server for the
- // specified RPC method name. The returned Stream interface can be used
- // to send and receive messages on the stream.
- NewStream(context.Context, string) (Stream, error)
-
- // Close closes the Transport.
- Close() error
-}
-
-// Stream provides methods to send and receive messages on a stream. Messages
-// are represented as a byte slice ([]byte).
-type Stream interface {
- // Send sends the provided message on the stream.
- Send([]byte) error
-
- // Recv blocks until the next message is received on the stream.
- Recv() ([]byte, error)
-}
From f93c43816d2c1bddd7b250d4bda16d949671b3e5 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 28 Jan 2025 00:07:56 +0530
Subject: [PATCH 09/26] xds: add lrs client and xDS client interfaces
---
xds/internal/clients/lrsclient/client.go | 45 +++++++++
xds/internal/clients/lrsclient/config.go | 34 +++++++
xds/internal/clients/lrsclient/load_store.go | 30 ++++++
xds/internal/clients/xdsclient/client.go | 72 ++++++++++++++
xds/internal/clients/xdsclient/config.go | 93 +++++++++++++++++++
.../clients/xdsclient/resource_type.go | 90 ++++++++++++++++++
.../clients/xdsclient/resource_watcher.go | 79 ++++++++++++++++
7 files changed, 443 insertions(+)
create mode 100644 xds/internal/clients/lrsclient/client.go
create mode 100644 xds/internal/clients/lrsclient/config.go
create mode 100644 xds/internal/clients/lrsclient/load_store.go
create mode 100644 xds/internal/clients/xdsclient/client.go
create mode 100644 xds/internal/clients/xdsclient/config.go
create mode 100644 xds/internal/clients/xdsclient/resource_type.go
create mode 100644 xds/internal/clients/xdsclient/resource_watcher.go
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
new file mode 100644
index 000000000000..d16c8c3b5225
--- /dev/null
+++ b/xds/internal/clients/lrsclient/client.go
@@ -0,0 +1,45 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// Package lrsclient provides implementation of the LRS client for enabling
+// applications to report load to the xDS management servers.
+//
+// It allows applications to report load data to an LRS server via the LRS
+// stream. This data can be used for monitoring, traffic management, and other
+// purposes.
+package lrsclient
+
+import (
+ "google.golang.org/grpc/xds/internal/clients"
+)
+
+// LRSClient is a full fledged LRS client.
+type LRSClient struct {
+}
+
+// ReportLoad starts a load reporting stream to the given server. All load
+// reports to the same server share the LRS stream.
+//
+// It returns a [LoadStore] for the user to report loads, a function to
+// cancel the load reporting stream.
+//
+// The stats from [LoadStore] are reported periodically until cleanup
+// function is called.
+func (c *LRSClient) ReportLoad(_ *clients.ServerConfig) (*LoadStore, func()) {
+ return NewLoadStore(), func() {}
+}
diff --git a/xds/internal/clients/lrsclient/config.go b/xds/internal/clients/lrsclient/config.go
new file mode 100644
index 000000000000..80a8326b6490
--- /dev/null
+++ b/xds/internal/clients/lrsclient/config.go
@@ -0,0 +1,34 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package lrsclient
+
+import (
+ "google.golang.org/grpc/xds/internal/clients"
+)
+
+// Config provides parameters for configuring the LRS client.
+type Config struct {
+ // Node is the identity of the client application, reporting load to the
+ // xDS management server.
+ Node clients.Node
+
+ // TransportBuilder is the implementation to create a communication channel
+ // to an xDS management server.
+ TransportBuilder clients.TransportBuilder
+}
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
new file mode 100644
index 000000000000..2ff7f981d965
--- /dev/null
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -0,0 +1,30 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package lrsclient
+
+// LoadStore keeps the loads for multiple clusters and services to be reported via
+// LRS. It contains loads to report to one LRS server. Create multiple stores
+// for multiple servers.
+type LoadStore struct {
+}
+
+// NewLoadStore creates a new load store.
+func NewLoadStore() *LoadStore {
+ return &LoadStore{}
+}
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
new file mode 100644
index 000000000000..0502f13987b2
--- /dev/null
+++ b/xds/internal/clients/xdsclient/client.go
@@ -0,0 +1,72 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// Package xdsclient provides implementation of the xDS client for enabling
+// applications to communicate with xDS management servers.
+//
+// It allows applications to:
+// - Create xDS client instance with in-memory configurations.
+// - Register watches for named resources.
+// - Receive resources via the ADS (Aggregated Discovery Service) stream.
+//
+// This enables applications to dynamically discover and configure resources
+// such as listeners, routes, clusters, and endpoints from an xDS management
+// server.
+package xdsclient
+
+import (
+ "errors"
+
+ v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3"
+)
+
+// XDSClient is a full fledged client which queries a set of discovery APIs
+// (collectively termed as xDS) on a remote management server, to discover
+// various dynamic resources.
+type XDSClient struct {
+}
+
+// New returns a new xDS Client configured with provided config.
+func New(_ Config) (*XDSClient, error) {
+ return nil, errors.New("xds: xDS client is not yet implemented")
+}
+
+// WatchResource uses xDS to discover the resource associated with the provided
+// resource name. The resource type url look up the resource type
+// implementation which determines how xDS responses are received, are
+// deserialized and validated. Upon receipt of a response from the management
+// server, an appropriate callback on the watcher is invoked.
+//
+// During a race (e.g. an xDS response is received while the user is calling
+// cancel()), there's a small window where the callback can be called after
+// the watcher is canceled. Callers need to handle this case.
+func (c *XDSClient) WatchResource(_ string, _ string, _ ResourceWatcher) (cancel func()) {
+ return nil
+}
+
+// Close closes the xDS client and releases all resources. The caller is
+// expected to invoke it once they are done using the client.
+func (c *XDSClient) Close() error {
+ return nil
+}
+
+// DumpResources returns the status and contents of all xDS resources from the
+// xDS client.
+func (c *XDSClient) DumpResources() *v3statuspb.ClientStatusResponse {
+ return nil
+}
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
new file mode 100644
index 000000000000..0159485c5a64
--- /dev/null
+++ b/xds/internal/clients/xdsclient/config.go
@@ -0,0 +1,93 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package xdsclient
+
+import (
+ "time"
+
+ "google.golang.org/grpc/internal/backoff"
+ "google.golang.org/grpc/xds/internal/clients"
+)
+
+const (
+ defaultWatchExpiryTimeout = 15 * time.Second
+)
+
+var (
+ defaultStreamBackoffFunc = backoff.DefaultExponential.Backoff
+)
+
+// Config provides parameters for configuring the xDS client.
+type Config struct {
+ // Servers specifies a list of xDS servers to connect to. This field should
+ // be used only for old-style names without an authority.
+ Servers []clients.ServerConfig
+
+ // Authorities is a map of authority names to authority configurations.
+ // Each authority configuration contains list of xDS servers to connect to,
+ // including fallbacks.
+ Authorities map[string]clients.Authority
+
+ // Node is the identity of the xDS client, connecting to the xDS
+ // management server.
+ Node clients.Node
+
+ // TransportBuilder is the implementation to create a communication channel
+ // to an xDS management server.
+ TransportBuilder clients.TransportBuilder
+
+ // ResourceTypes is a map from resource type URLs to resource type
+ // implementations. Each resource type URL uniquely identifies a specific
+ // kind of xDS resource, and the corresponding resource type implementation
+ // provides logic for parsing, validating, and processing resources of that
+ // type.
+ ResourceTypes map[string]ResourceType
+
+ // Below values will have default values but can be overridden for testing
+
+ // watchExpiryTimeOut is the duration after which a watch for a resource
+ // will expire if no updates are received.
+ watchExpiryTimeOut time.Duration
+
+ // streamBackOffTimeout is a function that returns the backoff duration for
+ // retrying failed xDS streams.
+ streamBackOffTimeout func(int) time.Duration
+}
+
+// NewConfig returns a new xDS client config with provided parameters.
+func NewConfig(servers []clients.ServerConfig, authorities map[string]clients.Authority, node clients.Node, transport clients.TransportBuilder, resourceTypes map[string]ResourceType) Config {
+ c := Config{Servers: servers, Authorities: authorities, Node: node, TransportBuilder: transport, ResourceTypes: resourceTypes}
+ c.watchExpiryTimeOut = defaultWatchExpiryTimeout
+ c.streamBackOffTimeout = defaultStreamBackoffFunc
+ return c
+}
+
+// SetWatchExpiryTimeoutForTesting sets the watch expiry timeout.
+//
+// For testing purpose only.
+func (c *Config) SetWatchExpiryTimeoutForTesting(d time.Duration) {
+ c.watchExpiryTimeOut = d
+}
+
+// SetStreamBackOffTimeoutForTesting sets the stream backoff timeout function
+//
+// For testing purpose only.
+func (c *Config) SetStreamBackOffTimeoutForTesting(d func(int) time.Duration) {
+ c.streamBackOffTimeout = d
+}
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
new file mode 100644
index 000000000000..c349f3c39bee
--- /dev/null
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -0,0 +1,90 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package xdsclient
+
+import (
+ "google.golang.org/grpc/xds/internal/clients"
+ "google.golang.org/protobuf/types/known/anypb"
+)
+
+// ResourceType wraps all resource-type specific functionality. Each supported
+// resource type will provide an implementation of this interface.
+type ResourceType interface {
+ // TypeURL is the xDS type URL of this resource type for the v3 xDS
+ // protocol. This URL is used as the key to look up the corresponding
+ // ResourceType implementation in the ResourceTypes map provided in the
+ // [Config].
+ TypeURL() string
+
+ // TypeName identifies resources in a transport protocol agnostic way. This
+ // can be used for logging/debugging purposes, as well in cases where the
+ // resource type name is to be uniquely identified but the actual
+ // functionality provided by the resource type is not required.
+ TypeName() string
+
+ // AllResourcesRequiredInSotW indicates whether this resource type requires
+ // that all resources be present in every SotW response from the server. If
+ // true, a response that does not include a previously seen resource will
+ // be interpreted as a deletion of that resource.
+ AllResourcesRequiredInSotW() bool
+
+ // Decode deserializes and validates an xDS resource serialized inside the
+ // provided `Any` proto, as received from the xDS management server.
+ //
+ // If protobuf deserialization fails or resource validation fails,
+ // returns a non-nil error. Otherwise, returns a fully populated
+ // DecodeResult.
+ Decode(DecodeOptions, any) (*DecodeResult, error)
+}
+
+// DecodeOptions wraps the options required by ResourceType implementation for
+// decoding configuration received from the xDS management server.
+type DecodeOptions struct {
+ // Config contains the complete configuration passed to the xDS client.
+ // This contains useful data for resource validation.
+ Config *Config
+
+ // ServerConfig contains the server config (from the above bootstrap
+ // configuration) of the xDS server from which the current resource, for
+ // which Decode() is being invoked, was received.
+ ServerConfig *clients.ServerConfig
+}
+
+// DecodeResult is the result of a decode operation.
+type DecodeResult struct {
+ // Name is the name of the resource being watched.
+ Name string
+
+ // Resource contains the configuration associated with the resource being
+ // watched.
+ Resource ResourceData
+}
+
+// ResourceData contains the configuration data sent by the xDS management
+// server, associated with the resource being watched. Every resource type must
+// provide an implementation of this interface to represent the configuration
+// received from the xDS management server.
+type ResourceData interface {
+ // RawEqual returns true if the passed in resource data is equal to that of
+ // the receiver, based on the underlying raw protobuf message.
+ RawEqual(ResourceData) bool
+
+ // Raw returns the underlying raw protobuf form of the resource.
+ Raw() *anypb.Any
+}
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
new file mode 100644
index 000000000000..54f226b7edd4
--- /dev/null
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -0,0 +1,79 @@
+/*
+ *
+ * Copyright 2024 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package xdsclient
+
+// OnUpdateProcessed is a function to be invoked by watcher implementations
+// upon completing the processing of a callback from the xDS client. Failure to
+// invoke this callback prevents the xDS client from reading further messages
+// from the xDS server.
+type OnUpdateProcessed func()
+
+// ResourceDataOrError is a struct that contains either ResourceData or error.
+// It is used to represent the result of an xDS resource update. Exactly one of
+// Data or Err will be non-nil.
+type ResourceDataOrError struct {
+ Data ResourceData
+ Err error
+}
+
+// ResourceWatcher wraps the callbacks to be invoked for different events
+// corresponding to the resource being watched.
+type ResourceWatcher interface {
+ // OnResourceChanged is invoked to notify the watcher of a new version of
+ // the resource received from the xDS server or an error indicating the
+ // reason why the resource could not be obtained.
+ //
+ // In the former case, this callback will be invoked with a non-nil
+ // ResourceData in ResourceDataOrError. The ResourceData of the
+ // ResourceDataOrError needs to be type asserted to the appropriate type
+ // for the resource being watched.
+ //
+ // In the latter case, this callback will be invoked with a non-nil error
+ // value in ResourceDataOrError.
+ //
+ // Watcher is expected to use the most recent value passed to
+ // OnResourceChanged(), regardless of whether that's a ResourceData or an
+ // error i.e., if the watcher is given an error via OnResourceChanged(),
+ // that means it should stop using any previously delivered resource.
+ //
+ // It is invoked under different error conditions including but not
+ // limited to the following:
+ // - authority mentioned in the resource is not found
+ // - resource name parsing error
+ // - resource validation error (if resource is not cached)
+ // - ADS stream failure (if resource is not cached)
+ // - connection failure (if resource is not cached)
+ OnResourceChanged(ResourceDataOrError, OnUpdateProcessed)
+
+ // OnAmbientError is invoked to notify the watcher of an error that occurs
+ // after a resource has been received (i.e. we already have a cached
+ // resource) that should not modify the watcher’s use of that resource but
+ // that may be useful information about the ambient state of the XdsClient.
+ // In particular, the watcher should NOT stop using the previously seen
+ // resource, and the XdsClient will NOT remove the resource from its cache.
+ // However, the error message may be useful as additional context to
+ // include in errors that are being generated for other reasons.
+ //
+ // If resource is already cached, it is invoked under different error
+ // conditions including but not limited to the following:
+ // - resource validation error
+ // - ADS stream failure
+ // - connection failure
+ OnAmbientError(error, OnUpdateProcessed)
+}
From 72b99b4b43b79acdde00ceca7e65f1b151e9c27f Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Mon, 3 Feb 2025 13:10:28 +0530
Subject: [PATCH 10/26] second pass to documentation language
---
xds/internal/clients/lrsclient/client.go | 21 ++++---
xds/internal/clients/lrsclient/config.go | 4 +-
xds/internal/clients/lrsclient/load_store.go | 10 +--
xds/internal/clients/xdsclient/client.go | 47 +++++++-------
xds/internal/clients/xdsclient/config.go | 4 +-
.../clients/xdsclient/resource_type.go | 4 +-
.../clients/xdsclient/resource_watcher.go | 61 ++++++-------------
7 files changed, 65 insertions(+), 86 deletions(-)
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
index d16c8c3b5225..88ab888f9aed 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/client.go
@@ -1,6 +1,8 @@
+//revive:disable:unused-parameter
+
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,8 +18,8 @@
*
*/
-// Package lrsclient provides implementation of the LRS client for enabling
-// applications to report load to the xDS management servers.
+// Package lrsclient provides an implementation of the LRS client to report
+// load to the xDS management servers.
//
// It allows applications to report load data to an LRS server via the LRS
// stream. This data can be used for monitoring, traffic management, and other
@@ -28,18 +30,19 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// LRSClient is a full fledged LRS client.
+// LRSClient is an LRS client to report load data to the LRS servers.
type LRSClient struct {
}
-// ReportLoad starts a load reporting stream to the given server. All load
-// reports to the same server share the LRS stream.
+// ReportLoad starts a load reporting stream to the server in given
+// [clients.ServerConfig]. All load reports to the same server share the LRS
+// stream.
//
-// It returns a [LoadStore] for the user to report loads, a function to
+// It returns a [LoadStore] for the user to report loads and a function to
// cancel the load reporting stream.
//
// The stats from [LoadStore] are reported periodically until cleanup
// function is called.
-func (c *LRSClient) ReportLoad(_ *clients.ServerConfig) (*LoadStore, func()) {
- return NewLoadStore(), func() {}
+func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) (*LoadStore, func()) {
+ panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/config.go b/xds/internal/clients/lrsclient/config.go
index 80a8326b6490..359e533b9151 100644
--- a/xds/internal/clients/lrsclient/config.go
+++ b/xds/internal/clients/lrsclient/config.go
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -24,7 +24,7 @@ import (
// Config provides parameters for configuring the LRS client.
type Config struct {
- // Node is the identity of the client application, reporting load to the
+ // Node is the identity of the client application reporting load to the
// xDS management server.
Node clients.Node
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index 2ff7f981d965..b235b6aa084a 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,13 +18,13 @@
package lrsclient
-// LoadStore keeps the loads for multiple clusters and services to be reported via
-// LRS. It contains loads to report to one LRS server. Create multiple stores
-// for multiple servers.
+// LoadStore keeps the loads for multiple clusters and services to be reported
+// via LRS. It contains loads to report to one LRS server. It creates
+// multiple stores for multiple servers.
type LoadStore struct {
}
// NewLoadStore creates a new load store.
func NewLoadStore() *LoadStore {
- return &LoadStore{}
+ panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
index 0502f13987b2..2461d4446ff2 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/client.go
@@ -1,6 +1,8 @@
+//revive:disable:unused-parameter
+
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,13 +18,13 @@
*
*/
-// Package xdsclient provides implementation of the xDS client for enabling
-// applications to communicate with xDS management servers.
+// Package xdsclient provides an implementation of the xDS client to
+// communicate with xDS management servers.
//
// It allows applications to:
-// - Create xDS client instance with in-memory configurations.
+// - Create xDS client instances with in-memory configurations.
// - Register watches for named resources.
-// - Receive resources via the ADS (Aggregated Discovery Service) stream.
+// - Receive resources via an ADS (Aggregated Discovery Service) stream.
//
// This enables applications to dynamically discover and configure resources
// such as listeners, routes, clusters, and endpoints from an xDS management
@@ -30,43 +32,42 @@
package xdsclient
import (
- "errors"
-
v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3"
)
-// XDSClient is a full fledged client which queries a set of discovery APIs
-// (collectively termed as xDS) on a remote management server, to discover
+// XDSClient is a client which queries a set of discovery APIs (collectively
+// termed as xDS) on a remote management server, to discover
// various dynamic resources.
type XDSClient struct {
}
// New returns a new xDS Client configured with provided config.
-func New(_ Config) (*XDSClient, error) {
- return nil, errors.New("xds: xDS client is not yet implemented")
+func New(config Config) (*XDSClient, error) {
+ panic("unimplemented")
}
// WatchResource uses xDS to discover the resource associated with the provided
-// resource name. The resource type url look up the resource type
-// implementation which determines how xDS responses are received, are
-// deserialized and validated. Upon receipt of a response from the management
-// server, an appropriate callback on the watcher is invoked.
-//
-// During a race (e.g. an xDS response is received while the user is calling
-// cancel()), there's a small window where the callback can be called after
-// the watcher is canceled. Callers need to handle this case.
-func (c *XDSClient) WatchResource(_ string, _ string, _ ResourceWatcher) (cancel func()) {
- return nil
+// resource name.
+// - resourceTypeURL is used to look up the resource type implementation
+// provided in the [ResourceTypes] of the config which determines how
+// xDS responses are deserialized and validated after receiving from the xDS
+// management server.
+// - resourceName is the name of the resource to watch.
+// - resourceWatcher is used to notify the caller about updates to the resource
+// being watched. Upon receipt of a response from the management server, an
+// appropriate callback on the resourceWatcher is invoked.
+func (c *XDSClient) WatchResource(resourceTypeURL string, resourceName string, resourceWatcher ResourceWatcher) (cancel func()) {
+ panic("unimplemented")
}
// Close closes the xDS client and releases all resources. The caller is
// expected to invoke it once they are done using the client.
func (c *XDSClient) Close() error {
- return nil
+ panic("unimplemented")
}
// DumpResources returns the status and contents of all xDS resources from the
// xDS client.
func (c *XDSClient) DumpResources() *v3statuspb.ClientStatusResponse {
- return nil
+ panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index 0159485c5a64..e08bbeca3f55 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -44,7 +44,7 @@ type Config struct {
// including fallbacks.
Authorities map[string]clients.Authority
- // Node is the identity of the xDS client, connecting to the xDS
+ // Node is the identity of the xDS client connecting to the xDS
// management server.
Node clients.Node
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index c349f3c39bee..fc883360c078 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -24,7 +24,7 @@ import (
)
// ResourceType wraps all resource-type specific functionality. Each supported
-// resource type will provide an implementation of this interface.
+// resource type needs to provide an implementation of this interface.
type ResourceType interface {
// TypeURL is the xDS type URL of this resource type for the v3 xDS
// protocol. This URL is used as the key to look up the corresponding
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 54f226b7edd4..0e60e61c4bdc 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -1,6 +1,6 @@
/*
*
- * Copyright 2024 gRPC authors.
+ * Copyright 2025 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,62 +18,37 @@
package xdsclient
-// OnUpdateProcessed is a function to be invoked by watcher implementations
-// upon completing the processing of a callback from the xDS client. Failure to
-// invoke this callback prevents the xDS client from reading further messages
-// from the xDS server.
+// OnUpdateProcessed is a function to be invoked by resource watcher
+// implementations upon completing the processing of a callback from the xDS
+// client. Failure to invoke this callback prevents the xDS client from reading
+// further messages from the xDS server.
type OnUpdateProcessed func()
-// ResourceDataOrError is a struct that contains either ResourceData or error.
-// It is used to represent the result of an xDS resource update. Exactly one of
-// Data or Err will be non-nil.
+// ResourceDataOrError is a struct that contains either [ResourceData] or
+// error. It is used to represent the result of an xDS resource update. Exactly
+// one of Data or Err will be non-nil.
type ResourceDataOrError struct {
Data ResourceData
Err error
}
// ResourceWatcher wraps the callbacks to be invoked for different events
-// corresponding to the resource being watched.
+// corresponding to the resource being watched. gRFC A88 contains an exhaustive
+// list of what method is invoked under what conditions.
type ResourceWatcher interface {
// OnResourceChanged is invoked to notify the watcher of a new version of
// the resource received from the xDS server or an error indicating the
- // reason why the resource could not be obtained.
+ // reason why the resource cannot be obtained.
//
- // In the former case, this callback will be invoked with a non-nil
- // ResourceData in ResourceDataOrError. The ResourceData of the
- // ResourceDataOrError needs to be type asserted to the appropriate type
- // for the resource being watched.
- //
- // In the latter case, this callback will be invoked with a non-nil error
- // value in ResourceDataOrError.
- //
- // Watcher is expected to use the most recent value passed to
- // OnResourceChanged(), regardless of whether that's a ResourceData or an
- // error i.e., if the watcher is given an error via OnResourceChanged(),
- // that means it should stop using any previously delivered resource.
- //
- // It is invoked under different error conditions including but not
- // limited to the following:
- // - authority mentioned in the resource is not found
- // - resource name parsing error
- // - resource validation error (if resource is not cached)
- // - ADS stream failure (if resource is not cached)
- // - connection failure (if resource is not cached)
+ // Upon receiving this, in case of an error, the watcher should
+ // stop using any previously seen resource. The xDS client will remove the
+ // resource from its cache.
OnResourceChanged(ResourceDataOrError, OnUpdateProcessed)
- // OnAmbientError is invoked to notify the watcher of an error that occurs
- // after a resource has been received (i.e. we already have a cached
- // resource) that should not modify the watcher’s use of that resource but
- // that may be useful information about the ambient state of the XdsClient.
- // In particular, the watcher should NOT stop using the previously seen
- // resource, and the XdsClient will NOT remove the resource from its cache.
- // However, the error message may be useful as additional context to
- // include in errors that are being generated for other reasons.
+ // OnAmbientError is invoked if resource is already cached under different
+ // error conditions.
//
- // If resource is already cached, it is invoked under different error
- // conditions including but not limited to the following:
- // - resource validation error
- // - ADS stream failure
- // - connection failure
+ // Upon receiving this, the watcher may continue using the previously seen
+ // resource. The xDS client will not remove the resource from its cache.
OnAmbientError(error, OnUpdateProcessed)
}
From 77689c265cd0b5ace0de692d28a576c5335584f7 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 4 Feb 2025 16:14:33 +0530
Subject: [PATCH 11/26] change from godoc review
---
xds/internal/clients/config.go | 25 +++++--------------
xds/internal/clients/lrsclient/client.go | 6 ++---
xds/internal/clients/transport_builder.go | 8 +++---
xds/internal/clients/xdsclient/client.go | 2 +-
xds/internal/clients/xdsclient/config.go | 14 -----------
.../clients/xdsclient/resource_type.go | 12 ++++-----
.../clients/xdsclient/resource_watcher.go | 10 ++++----
7 files changed, 25 insertions(+), 52 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index 5640a1e0afed..9bba600034b4 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -20,7 +20,7 @@
// enabling applications to communicate with xDS management servers and report
// load.
//
-// xDS Client
+// # xDS Client
//
// The xDS client allows applications to:
// - Create client instances with in-memory configurations.
@@ -41,14 +41,10 @@
//
// NOTICE: This package is EXPERIMENTAL and may be changed or removed
// in a later release.
-//
-// See [README](https://github.com/grpc/grpc-go/tree/master/xds/clients/README.md).
package clients
import (
- "fmt"
"slices"
- "strings"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"
@@ -71,15 +67,15 @@ type ServerConfig struct {
IgnoreResourceDeletion bool
// Extensions can be populated with arbitrary data to be passed to the
- // [TransportBuilder] and/or xDS Client's ResourceType implementations.
+ // TransportBuilder and/or xDS Client's ResourceType implementations.
// This field can be used to provide additional configuration or context
// specific to the user's needs.
//
// The xDS and LRS clients do not interpret the contents of this field.
- // It is the responsibility of the user's custom [TransportBuilder] and/or
+ // It is the responsibility of the user's custom TransportBuilder and/or
// ResourceType implementations to handle and interpret these extensions.
//
- // For example, a custom [TransportBuilder] might use this field to
+ // For example, a custom TransportBuilder might use this field to
// configure a specific security credentials.
//
// Note: For custom types used in Extensions, ensure an Equal(any) bool
@@ -108,15 +104,6 @@ func (sc *ServerConfig) equal(other *ServerConfig) bool {
return false
}
-// String returns a string representation of the [ServerConfig].
-//
-// WARNING: This method is primarily intended for logging and testing
-// purposes. The output returned by this method is not guaranteed to be stable
-// and may change at any time. Do not rely on it for production use.
-func (sc *ServerConfig) String() string {
- return strings.Join([]string{sc.ServerURI, fmt.Sprintf("%v", sc.IgnoreResourceDeletion)}, "-")
-}
-
// Authority contains configuration for an xDS control plane authority.
type Authority struct {
// XDSServers contains the list of server configurations for this authority.
@@ -150,13 +137,13 @@ type Node struct {
UserAgentName string
// UserAgentVersion is the user agent version of application.
UserAgentVersion string
- // ClientFeatures is a list of xDS features supported by this client.
+ // clientFeatures is a list of xDS features supported by this client.
// These features are set within the xDS client, but may be overridden only
// for testing purposes.
clientFeatures []string
}
-// toProto converts an instance of [Node] to its protobuf representation.
+// toProto converts an instance of Node to its protobuf representation.
func (n Node) toProto() *v3corepb.Node {
return &v3corepb.Node{
Id: n.ID,
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
index 88ab888f9aed..a7e8b4f2488c 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/client.go
@@ -35,13 +35,13 @@ type LRSClient struct {
}
// ReportLoad starts a load reporting stream to the server in given
-// [clients.ServerConfig]. All load reports to the same server share the LRS
+// clients.ServerConfig. All load reports to the same server share the LRS
// stream.
//
-// It returns a [LoadStore] for the user to report loads and a function to
+// It returns a LoadStore for the user to report loads and a function to
// cancel the load reporting stream.
//
-// The stats from [LoadStore] are reported periodically until cleanup
+// The stats from LoadStore are reported periodically until cleanup
// function is called.
func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) (*LoadStore, func()) {
panic("unimplemented")
diff --git a/xds/internal/clients/transport_builder.go b/xds/internal/clients/transport_builder.go
index 5b0eb10cbd3c..e07606e8fc39 100644
--- a/xds/internal/clients/transport_builder.go
+++ b/xds/internal/clients/transport_builder.go
@@ -25,13 +25,13 @@ import (
// TransportBuilder provides the functionality to create a communication
// channel to an xDS management server.
type TransportBuilder interface {
- // Build creates a new [Transport] instance to the xDS server based on the
+ // Build creates a new Transport instance to the xDS server based on the
// provided ServerConfig.
Build(ServerConfig ServerConfig) (Transport, error)
}
-// Transport provides the functionality to communicate with an xDS server using
-// streaming calls.
+// Transport provides the functionality to communicate with an xDS management
+// server using streaming calls.
type Transport interface {
// NewStream creates a new streaming call to the xDS server for the
// specified RPC method name. The returned Stream interface can be used
@@ -43,7 +43,7 @@ type Transport interface {
}
// Stream provides methods to send and receive messages on a stream. Messages
-// are represented as a byte slice ([]byte).
+// are represented as a byte slice.
type Stream interface {
// Send sends the provided message on the stream.
Send([]byte) error
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
index 2461d4446ff2..2069e2fa8e49 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/client.go
@@ -49,7 +49,7 @@ func New(config Config) (*XDSClient, error) {
// WatchResource uses xDS to discover the resource associated with the provided
// resource name.
// - resourceTypeURL is used to look up the resource type implementation
-// provided in the [ResourceTypes] of the config which determines how
+// provided in the ResourceTypes map of the config which determines how
// xDS responses are deserialized and validated after receiving from the xDS
// management server.
// - resourceName is the name of the resource to watch.
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index e08bbeca3f55..248e084edc06 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -77,17 +77,3 @@ func NewConfig(servers []clients.ServerConfig, authorities map[string]clients.Au
c.streamBackOffTimeout = defaultStreamBackoffFunc
return c
}
-
-// SetWatchExpiryTimeoutForTesting sets the watch expiry timeout.
-//
-// For testing purpose only.
-func (c *Config) SetWatchExpiryTimeoutForTesting(d time.Duration) {
- c.watchExpiryTimeOut = d
-}
-
-// SetStreamBackOffTimeoutForTesting sets the stream backoff timeout function
-//
-// For testing purpose only.
-func (c *Config) SetStreamBackOffTimeoutForTesting(d func(int) time.Duration) {
- c.streamBackOffTimeout = d
-}
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index fc883360c078..d87289cb630b 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -29,7 +29,7 @@ type ResourceType interface {
// TypeURL is the xDS type URL of this resource type for the v3 xDS
// protocol. This URL is used as the key to look up the corresponding
// ResourceType implementation in the ResourceTypes map provided in the
- // [Config].
+ // Config.
TypeURL() string
// TypeName identifies resources in a transport protocol agnostic way. This
@@ -50,12 +50,12 @@ type ResourceType interface {
// If protobuf deserialization fails or resource validation fails,
// returns a non-nil error. Otherwise, returns a fully populated
// DecodeResult.
- Decode(DecodeOptions, any) (*DecodeResult, error)
+ Decode(ResourceDecodeOptions, any) (*ResourceDecodeResult, error)
}
-// DecodeOptions wraps the options required by ResourceType implementation for
+// ResourceDecodeOptions wraps the options required by ResourceType implementation for
// decoding configuration received from the xDS management server.
-type DecodeOptions struct {
+type ResourceDecodeOptions struct {
// Config contains the complete configuration passed to the xDS client.
// This contains useful data for resource validation.
Config *Config
@@ -66,8 +66,8 @@ type DecodeOptions struct {
ServerConfig *clients.ServerConfig
}
-// DecodeResult is the result of a decode operation.
-type DecodeResult struct {
+// ResourceDecodeResult is the result of a decode operation.
+type ResourceDecodeResult struct {
// Name is the name of the resource being watched.
Name string
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 0e60e61c4bdc..78779329e9ef 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -18,13 +18,13 @@
package xdsclient
-// OnUpdateProcessed is a function to be invoked by resource watcher
+// OnCallbackProcessed is a function to be invoked by resource watcher
// implementations upon completing the processing of a callback from the xDS
// client. Failure to invoke this callback prevents the xDS client from reading
// further messages from the xDS server.
-type OnUpdateProcessed func()
+type OnCallbackProcessed func()
-// ResourceDataOrError is a struct that contains either [ResourceData] or
+// ResourceDataOrError is a struct that contains either ResourceData or
// error. It is used to represent the result of an xDS resource update. Exactly
// one of Data or Err will be non-nil.
type ResourceDataOrError struct {
@@ -43,12 +43,12 @@ type ResourceWatcher interface {
// Upon receiving this, in case of an error, the watcher should
// stop using any previously seen resource. The xDS client will remove the
// resource from its cache.
- OnResourceChanged(ResourceDataOrError, OnUpdateProcessed)
+ OnResourceChanged(ResourceDataOrError, OnCallbackProcessed)
// OnAmbientError is invoked if resource is already cached under different
// error conditions.
//
// Upon receiving this, the watcher may continue using the previously seen
// resource. The xDS client will not remove the resource from its cache.
- OnAmbientError(error, OnUpdateProcessed)
+ OnAmbientError(error, OnCallbackProcessed)
}
From 0659a44eae13156410b1b10fcf094f5fe9153e31 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Fri, 7 Feb 2025 15:42:06 +0530
Subject: [PATCH 12/26] dfawley review 2
---
xds/internal/clients/config.go | 6 +-
xds/internal/clients/lrsclient/client.go | 26 +++---
xds/internal/clients/lrsclient/config.go | 9 ++-
xds/internal/clients/lrsclient/load_store.go | 79 ++++++++++++++++++-
xds/internal/clients/xdsclient/client.go | 24 +++---
xds/internal/clients/xdsclient/config.go | 46 +++--------
.../clients/xdsclient/resource_type.go | 14 ++--
.../clients/xdsclient/resource_watcher.go | 23 +++---
8 files changed, 132 insertions(+), 95 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index 9bba600034b4..c5717f89bf51 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -104,9 +104,13 @@ func (sc *ServerConfig) equal(other *ServerConfig) bool {
return false
}
-// Authority contains configuration for an xDS control plane authority.
+// Authority contains configuration for an xDS control plane [authority].
+//
+// [authority]: https://www.envoyproxy.io/docs/envoy/latest/xds/core/v3/authority.proto
type Authority struct {
// XDSServers contains the list of server configurations for this authority.
+ // xDS client use the first available server from the list. To ensure high
+ // availability, list the most reliable server first.
XDSServers []ServerConfig
// Extensions can be populated with arbitrary data to be passed to the xDS
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
index a7e8b4f2488c..5297d44e7433 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/client.go
@@ -18,31 +18,25 @@
*
*/
-// Package lrsclient provides an implementation of the LRS client to report
-// load to the xDS management servers.
+// Package lrsclient provides an [LRS] (Load Reporting Service) client.
//
-// It allows applications to report load data to an LRS server via the LRS
-// stream. This data can be used for monitoring, traffic management, and other
-// purposes.
+// [LRS]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
package lrsclient
import (
"google.golang.org/grpc/xds/internal/clients"
)
-// LRSClient is an LRS client to report load data to the LRS servers.
+// LRSClient is an LRS client.
type LRSClient struct {
}
-// ReportLoad starts a load reporting stream to the server in given
-// clients.ServerConfig. All load reports to the same server share the LRS
-// stream.
-//
-// It returns a LoadStore for the user to report loads and a function to
-// cancel the load reporting stream.
-//
-// The stats from LoadStore are reported periodically until cleanup
-// function is called.
-func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) (*LoadStore, func()) {
+// ReportLoad creates a new load reporting stream for the client.
+func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {
+ panic("unimplemented")
+}
+
+// Close closes the LRS client and releases all resources.
+func (c *LRSClient) Close() error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/config.go b/xds/internal/clients/lrsclient/config.go
index 359e533b9151..72924da7fe2a 100644
--- a/xds/internal/clients/lrsclient/config.go
+++ b/xds/internal/clients/lrsclient/config.go
@@ -22,13 +22,14 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// Config provides parameters for configuring the LRS client.
+// A Config structure is used to configure an LRS client. After one has been
+// passed to an LRS function it must not be modified. A Config may be used;
+// the LRS package will also not modify it.
type Config struct {
// Node is the identity of the client application reporting load to the
- // xDS management server.
+ // LRS server.
Node clients.Node
- // TransportBuilder is the implementation to create a communication channel
- // to an xDS management server.
+ // TransportBuilder is used to connect to the LRS server.
TransportBuilder clients.TransportBuilder
}
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index b235b6aa084a..c1e9f49434dd 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -1,3 +1,5 @@
+//revive:disable:unused-parameter
+
/*
*
* Copyright 2025 gRPC authors.
@@ -18,13 +20,86 @@
package lrsclient
+import "time"
+
// LoadStore keeps the loads for multiple clusters and services to be reported
// via LRS. It contains loads to report to one LRS server. It creates
// multiple stores for multiple servers.
+//
+// A LoadStore is created via LRSClient.ReportLoad and returned for the caller
+// to report loads.
type LoadStore struct {
}
-// NewLoadStore creates a new load store.
-func NewLoadStore() *LoadStore {
+// Stats returns the load data for the given cluster names. Data is returned in
+// a slice with no specific order.
+//
+// If no clusterName is given (an empty slice), all data for all known clusters
+// is returned.
+//
+// If a cluster's Data is empty (no load to report), it's not appended to the
+// returned slice.
+func (s *LoadStore) Stats(clusterNames []string) []*Data {
+ panic("unimplemented")
+}
+
+// PerCluster returns the perClusterStore for the given clusterName +
+// serviceName.
+func (s *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter {
panic("unimplemented")
}
+
+// Data contains all load data reported to the LoadStore since the most recent
+// call to Stats().
+type Data struct {
+ // Cluster is the name of the cluster this data is for.
+ Cluster string
+ // Service is the name of the EDS service this data is for.
+ Service string
+ // TotalDrops is the total number of dropped requests.
+ TotalDrops uint64
+ // Drops is the number of dropped requests per category.
+ Drops map[string]uint64
+ // LocalityStats contains load reports per locality.
+ LocalityStats map[string]LocalityData
+ // ReportInternal is the duration since last time load was reported (Stats()
+ // was called).
+ ReportInterval time.Duration
+}
+
+// LocalityData contains load data for a single locality.
+type LocalityData struct {
+ // RequestStats contains counts of requests made to the locality.
+ RequestStats RequestData
+ // LoadStats contains server load data for requests made to the locality,
+ // indexed by the load type.
+ LoadStats map[string]ServerLoadData
+}
+
+// RequestData contains request counts.
+type RequestData struct {
+ // Succeeded is the number of succeeded requests.
+ Succeeded uint64
+ // Errored is the number of requests which ran into errors.
+ Errored uint64
+ // InProgress is the number of requests in flight.
+ InProgress uint64
+ // Issued is the total number requests that were sent.
+ Issued uint64
+}
+
+// ServerLoadData contains server load data.
+type ServerLoadData struct {
+ // Count is the number of load reports.
+ Count uint64
+ // Sum is the total value of all load reports.
+ Sum float64
+}
+
+// PerClusterReporter wraps the methods from the LoadStore that are used here.
+type PerClusterReporter interface {
+ CallStarted(locality string)
+ CallFinished(locality string, err error)
+ CallServerLoad(locality, name string, val float64)
+ CallDropped(category string)
+}
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
index 2069e2fa8e49..912d7d23aeec 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/client.go
@@ -41,27 +41,23 @@ import (
type XDSClient struct {
}
-// New returns a new xDS Client configured with provided config.
+// New returns a new xDS Client configured with the provided config.
func New(config Config) (*XDSClient, error) {
panic("unimplemented")
}
-// WatchResource uses xDS to discover the resource associated with the provided
-// resource name.
-// - resourceTypeURL is used to look up the resource type implementation
-// provided in the ResourceTypes map of the config which determines how
-// xDS responses are deserialized and validated after receiving from the xDS
-// management server.
-// - resourceName is the name of the resource to watch.
-// - resourceWatcher is used to notify the caller about updates to the resource
-// being watched. Upon receipt of a response from the management server, an
-// appropriate callback on the resourceWatcher is invoked.
-func (c *XDSClient) WatchResource(resourceTypeURL string, resourceName string, resourceWatcher ResourceWatcher) (cancel func()) {
+// WatchResource starts watching the specified resource.
+//
+// typeURL must be present in the XDSClient's configured ResourceTypes. The
+// ResourceType will be used to decode the matching resource when it is
+// received, and the watcher will be called with the result.
+//
+// Cancel cancels the watch and prevents future calls to the watcher.
+func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) {
panic("unimplemented")
}
-// Close closes the xDS client and releases all resources. The caller is
-// expected to invoke it once they are done using the client.
+// Close closes the xDS client and releases all resources.
func (c *XDSClient) Close() error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index 248e084edc06..fddf6ea24ce2 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -19,37 +19,27 @@
package xdsclient
import (
- "time"
-
- "google.golang.org/grpc/internal/backoff"
"google.golang.org/grpc/xds/internal/clients"
)
-const (
- defaultWatchExpiryTimeout = 15 * time.Second
-)
-
-var (
- defaultStreamBackoffFunc = backoff.DefaultExponential.Backoff
-)
-
-// Config provides parameters for configuring the xDS client.
+// A Config structure is used to configure an xDS client. After one has been
+// passed to an xDS function it must not be modified. A Config may be used;
+// the xDS package will also not modify it.
type Config struct {
- // Servers specifies a list of xDS servers to connect to. This field should
- // be used only for old-style names without an authority.
+ // Servers specifies a list of xDS management servers to connect to,
+ // including fallbacks. xDS client use the first available server from the
+ // list. To ensure high availability, list the most reliable server first.
Servers []clients.ServerConfig
- // Authorities is a map of authority names to authority configurations.
- // Each authority configuration contains list of xDS servers to connect to,
- // including fallbacks.
+ // Authorities map is used to define different authorities, in a federated
+ // setup, each with its own set of xDS management servers.
Authorities map[string]clients.Authority
// Node is the identity of the xDS client connecting to the xDS
// management server.
Node clients.Node
- // TransportBuilder is the implementation to create a communication channel
- // to an xDS management server.
+ // TransportBuilder is used to connect to the management server.
TransportBuilder clients.TransportBuilder
// ResourceTypes is a map from resource type URLs to resource type
@@ -58,22 +48,4 @@ type Config struct {
// provides logic for parsing, validating, and processing resources of that
// type.
ResourceTypes map[string]ResourceType
-
- // Below values will have default values but can be overridden for testing
-
- // watchExpiryTimeOut is the duration after which a watch for a resource
- // will expire if no updates are received.
- watchExpiryTimeOut time.Duration
-
- // streamBackOffTimeout is a function that returns the backoff duration for
- // retrying failed xDS streams.
- streamBackOffTimeout func(int) time.Duration
-}
-
-// NewConfig returns a new xDS client config with provided parameters.
-func NewConfig(servers []clients.ServerConfig, authorities map[string]clients.Authority, node clients.Node, transport clients.TransportBuilder, resourceTypes map[string]ResourceType) Config {
- c := Config{Servers: servers, Authorities: authorities, Node: node, TransportBuilder: transport, ResourceTypes: resourceTypes}
- c.watchExpiryTimeOut = defaultWatchExpiryTimeout
- c.streamBackOffTimeout = defaultStreamBackoffFunc
- return c
}
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index d87289cb630b..58b80805beb8 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -33,8 +33,8 @@ type ResourceType interface {
TypeURL() string
// TypeName identifies resources in a transport protocol agnostic way. This
- // can be used for logging/debugging purposes, as well in cases where the
- // resource type name is to be uniquely identified but the actual
+ // can be used for logging/debugging purposes, as well as in cases where
+ // the resource type name is to be uniquely identified but the actual
// functionality provided by the resource type is not required.
TypeName() string
@@ -50,12 +50,12 @@ type ResourceType interface {
// If protobuf deserialization fails or resource validation fails,
// returns a non-nil error. Otherwise, returns a fully populated
// DecodeResult.
- Decode(ResourceDecodeOptions, any) (*ResourceDecodeResult, error)
+ Decode(DecodeOptions, any) (*DecodeResult, error)
}
-// ResourceDecodeOptions wraps the options required by ResourceType implementation for
+// DecodeOptions wraps the options required by ResourceType implementation for
// decoding configuration received from the xDS management server.
-type ResourceDecodeOptions struct {
+type DecodeOptions struct {
// Config contains the complete configuration passed to the xDS client.
// This contains useful data for resource validation.
Config *Config
@@ -66,8 +66,8 @@ type ResourceDecodeOptions struct {
ServerConfig *clients.ServerConfig
}
-// ResourceDecodeResult is the result of a decode operation.
-type ResourceDecodeResult struct {
+// DecodeResult is the result of a decode operation.
+type DecodeResult struct {
// Name is the name of the resource being watched.
Name string
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 78779329e9ef..91d0c3ea5b30 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -36,19 +36,14 @@ type ResourceDataOrError struct {
// corresponding to the resource being watched. gRFC A88 contains an exhaustive
// list of what method is invoked under what conditions.
type ResourceWatcher interface {
- // OnResourceChanged is invoked to notify the watcher of a new version of
- // the resource received from the xDS server or an error indicating the
- // reason why the resource cannot be obtained.
- //
- // Upon receiving this, in case of an error, the watcher should
- // stop using any previously seen resource. The xDS client will remove the
- // resource from its cache.
- OnResourceChanged(ResourceDataOrError, OnCallbackProcessed)
+ // ResourceChanged either indicates a new version of the resource is
+ // available or an an error occurred while trying to fetch or decode the
+ // associated resource. In case of an error, the previous version of the
+ // resource should be considered invalid.
+ ResourceChanged(ResourceDataOrError, OnCallbackProcessed)
- // OnAmbientError is invoked if resource is already cached under different
- // error conditions.
- //
- // Upon receiving this, the watcher may continue using the previously seen
- // resource. The xDS client will not remove the resource from its cache.
- OnAmbientError(error, OnCallbackProcessed)
+ // AmbientError indicates an error occurred while trying to fetch or decode
+ // the associated resource. The previous version of the resource should still
+ // be considered valid.
+ AmbientError(err error, done func())
}
From 5454ab104debc72a8fe2505ad955dc3b69d5f9ea Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Mon, 10 Feb 2025 22:19:00 +0530
Subject: [PATCH 13/26] easwar review 1
---
xds/internal/clients/config.go | 19 +++++----
xds/internal/clients/lrsclient/client.go | 5 ++-
xds/internal/clients/lrsclient/config.go | 6 +--
xds/internal/clients/lrsclient/load_store.go | 16 +++-----
xds/internal/clients/xdsclient/client.go | 11 ++---
xds/internal/clients/xdsclient/config.go | 18 ++++++---
.../clients/xdsclient/resource_watcher.go | 40 ++++++++-----------
7 files changed, 59 insertions(+), 56 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index c5717f89bf51..dc1d30e5fdc2 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -16,9 +16,8 @@
*
*/
-// Package clients provides implementations of the xDS and LRS clients,
-// enabling applications to communicate with xDS management servers and report
-// load.
+// Package clients provides implementations of the clients to interact with
+// envoy.
//
// # xDS Client
//
@@ -104,13 +103,19 @@ func (sc *ServerConfig) equal(other *ServerConfig) bool {
return false
}
-// Authority contains configuration for an xDS control plane [authority].
+// Authority contains configuration for an xDS control plane authority.
//
-// [authority]: https://www.envoyproxy.io/docs/envoy/latest/xds/core/v3/authority.proto
+// See https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
type Authority struct {
// XDSServers contains the list of server configurations for this authority.
- // xDS client use the first available server from the list. To ensure high
- // availability, list the most reliable server first.
+ // The order of the servers in this list reflects the order of preference
+ // of the data returned by those servers. xDS client use the first
+ // available server from the list.
+ //
+ // See [gRFC A71] for more details on fallback behavior when the primary
+ // xDS server is unavailable.
+ //
+ // [gRFC A71]: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
XDSServers []ServerConfig
// Extensions can be populated with arbitrary data to be passed to the xDS
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
index 5297d44e7433..37f7042f8a09 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/client.go
@@ -18,7 +18,7 @@
*
*/
-// Package lrsclient provides an [LRS] (Load Reporting Service) client.
+// Package lrsclient provides an LRS (Load Reporting Service) client.
//
// [LRS]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
package lrsclient
@@ -31,7 +31,8 @@ import (
type LRSClient struct {
}
-// ReportLoad creates a new load reporting stream for the client.
+// ReportLoad creates a new load reporting stream for the client. It creates a
+// LoadStore and return it for the caller to report loads.
func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/config.go b/xds/internal/clients/lrsclient/config.go
index 72924da7fe2a..4a476096cdde 100644
--- a/xds/internal/clients/lrsclient/config.go
+++ b/xds/internal/clients/lrsclient/config.go
@@ -22,9 +22,9 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// A Config structure is used to configure an LRS client. After one has been
-// passed to an LRS function it must not be modified. A Config may be used;
-// the LRS package will also not modify it.
+// Config is used to configure an LRS client. After one has been passed to an
+// LRS function, it must not be modified. A Config may be used; the LRS package
+// will also not modify it.
type Config struct {
// Node is the identity of the client application reporting load to the
// LRS server.
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index c1e9f49434dd..d4160875d758 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -23,11 +23,8 @@ package lrsclient
import "time"
// LoadStore keeps the loads for multiple clusters and services to be reported
-// via LRS. It contains loads to report to one LRS server. It creates
-// multiple stores for multiple servers.
-//
-// A LoadStore is created via LRSClient.ReportLoad and returned for the caller
-// to report loads.
+// via LRS. It contains loads reported to one LRS server. Create multiple
+// stores for multiple servers.
type LoadStore struct {
}
@@ -43,8 +40,7 @@ func (s *LoadStore) Stats(clusterNames []string) []*Data {
panic("unimplemented")
}
-// PerCluster returns the perClusterStore for the given clusterName +
-// serviceName.
+// PerCluster returns the PerClusterReporter for the given cluster and service.
func (s *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter {
panic("unimplemented")
}
@@ -62,8 +58,7 @@ type Data struct {
Drops map[string]uint64
// LocalityStats contains load reports per locality.
LocalityStats map[string]LocalityData
- // ReportInternal is the duration since last time load was reported (Stats()
- // was called).
+ // ReportInterval is the duration over which load was reported.
ReportInterval time.Duration
}
@@ -96,7 +91,8 @@ type ServerLoadData struct {
Sum float64
}
-// PerClusterReporter wraps the methods from the LoadStore that are used here.
+// PerClusterReporter defines the methods that the LoadStore uses to track
+// per-cluster load reporting data.
type PerClusterReporter interface {
CallStarted(locality string)
CallFinished(locality string, err error)
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
index 912d7d23aeec..aeb39501dace 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/client.go
@@ -18,8 +18,7 @@
*
*/
-// Package xdsclient provides an implementation of the xDS client to
-// communicate with xDS management servers.
+// Package xdsclient provides an xDS (Extensible Discovery Service) client.
//
// It allows applications to:
// - Create xDS client instances with in-memory configurations.
@@ -48,9 +47,11 @@ func New(config Config) (*XDSClient, error) {
// WatchResource starts watching the specified resource.
//
-// typeURL must be present in the XDSClient's configured ResourceTypes. The
-// ResourceType will be used to decode the matching resource when it is
-// received, and the watcher will be called with the result.
+// typeURL must be present in the XDSClient's configured ResourceTypes. If
+// typeURL is not present, watch will not be started. The ResourceType
+// obtained from the ResourceTypes against the typeURL will be used to decode
+// the matching resource when it is received, and the watcher will be called
+// with the result.
//
// Cancel cancels the watch and prevents future calls to the watcher.
func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) {
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index fddf6ea24ce2..973af0afcf9b 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -22,13 +22,19 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// A Config structure is used to configure an xDS client. After one has been
-// passed to an xDS function it must not be modified. A Config may be used;
-// the xDS package will also not modify it.
+// Config is used to configure an xDS client. After one has been passed to an
+// xDS function, it must not be modified. A Config may be used; the xDS package
+// will also not modify it.
type Config struct {
- // Servers specifies a list of xDS management servers to connect to,
- // including fallbacks. xDS client use the first available server from the
- // list. To ensure high availability, list the most reliable server first.
+ // Servers specifies a list of xDS management servers to connect to. The
+ // order of the servers in this list reflects the order of preference of
+ // the data returned by those servers. xDS client use the first
+ // available server from the list.
+ //
+ // See [gRFC A71] for more details on fallback behavior when the primary
+ // xDS server is unavailable.
+ //
+ // [gRFC A71]: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
Servers []clients.ServerConfig
// Authorities map is used to define different authorities, in a federated
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 91d0c3ea5b30..8d3a7d663385 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -18,32 +18,26 @@
package xdsclient
-// OnCallbackProcessed is a function to be invoked by resource watcher
-// implementations upon completing the processing of a callback from the xDS
-// client. Failure to invoke this callback prevents the xDS client from reading
-// further messages from the xDS server.
-type OnCallbackProcessed func()
-
-// ResourceDataOrError is a struct that contains either ResourceData or
-// error. It is used to represent the result of an xDS resource update. Exactly
-// one of Data or Err will be non-nil.
-type ResourceDataOrError struct {
- Data ResourceData
- Err error
-}
-
// ResourceWatcher wraps the callbacks to be invoked for different events
// corresponding to the resource being watched. gRFC A88 contains an exhaustive
// list of what method is invoked under what conditions.
+//
+// onCallbackProcessed in the callbacks is a function to be invoked by
+// resource watcher implementations upon completing the processing of a
+// callback from the xDS client. Failure to invoke this callback prevents the
+// xDS client from reading further messages from the xDS server.
type ResourceWatcher interface {
- // ResourceChanged either indicates a new version of the resource is
- // available or an an error occurred while trying to fetch or decode the
- // associated resource. In case of an error, the previous version of the
- // resource should be considered invalid.
- ResourceChanged(ResourceDataOrError, OnCallbackProcessed)
+ // ResourceChanged indicates a new version of the resource is available.
+ ResourceChanged(resourceData ResourceData, onCallbackProcessed func())
+
+ // ResourceError indicates an error occurred while trying to fetch or
+ // decode the associated resource. The previous version of the resource
+ // should be considered invalid.
+ ResourceError(err error, onCallbackProcessed func())
- // AmbientError indicates an error occurred while trying to fetch or decode
- // the associated resource. The previous version of the resource should still
- // be considered valid.
- AmbientError(err error, done func())
+ // AmbientError indicates an error occurred after a resource has been
+ // received that should not modify the use of that resource but may be
+ // useful information about the ambient state of the XdsClient. The
+ // previous version of the resource should still be considered valid.
+ AmbientError(err error, onCallbackProcessed func())
}
From c1a18cd14d7a2e0dfcb553970d9c356286499082 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 11 Feb 2025 18:11:25 +0700
Subject: [PATCH 14/26] changed to decoder struct
---
xds/internal/clients/config.go | 20 +++++++++----------
xds/internal/clients/lrsclient/client.go | 6 +++---
xds/internal/clients/lrsclient/load_store.go | 7 ++++---
xds/internal/clients/transport_builder.go | 12 +++++------
xds/internal/clients/xdsclient/client.go | 6 +++---
xds/internal/clients/xdsclient/config.go | 6 +++---
.../clients/xdsclient/resource_type.go | 18 ++++++++++++-----
.../clients/xdsclient/resource_watcher.go | 5 +++--
8 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index dc1d30e5fdc2..dcd054e15df5 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -17,7 +17,7 @@
*/
// Package clients provides implementations of the clients to interact with
-// envoy.
+// xDS and LRS servers.
//
// # xDS Client
//
@@ -51,14 +51,14 @@ import (
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)
-// ServerConfig holds settings for connecting to an xDS management server.
+// ServerConfig holds settings for connecting to an xDS management or LRS server.
type ServerConfig struct {
- // ServerURI is the target URI of the xDS management server.
+ // ServerURI is the target URI of the server.
ServerURI string
// IgnoreResourceDeletion is a server feature which if set to true,
- // indicates that resource deletion errors can be ignored and cached
- // resource data can be used.
+ // indicates that resource deletion errors from xDS management servers can
+ // be ignored and cached resource data can be used.
//
// This will be removed in the future once we implement gRFC A88
// and two new fields FailOnDataErrors and
@@ -105,17 +105,17 @@ func (sc *ServerConfig) equal(other *ServerConfig) bool {
// Authority contains configuration for an xDS control plane authority.
//
-// See https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
+// See: https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
type Authority struct {
// XDSServers contains the list of server configurations for this authority.
// The order of the servers in this list reflects the order of preference
// of the data returned by those servers. xDS client use the first
// available server from the list.
//
- // See [gRFC A71] for more details on fallback behavior when the primary
+ // See gRFC A71 for more details on fallback behavior when the primary
// xDS server is unavailable.
//
- // [gRFC A71]: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
+ // gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
XDSServers []ServerConfig
// Extensions can be populated with arbitrary data to be passed to the xDS
@@ -129,8 +129,8 @@ type Authority struct {
Extensions any
}
-// Node represents the identity of the xDS client, allowing
-// management servers to identify the source of xDS requests.
+// Node represents the identity of the xDS client, allowing xDS and LRS servers
+// to identify the source of xDS requests.
type Node struct {
// ID is a string identifier of the application.
ID string
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/client.go
index 37f7042f8a09..34a6452ab952 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/client.go
@@ -20,14 +20,14 @@
// Package lrsclient provides an LRS (Load Reporting Service) client.
//
-// [LRS]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
+// See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
package lrsclient
import (
"google.golang.org/grpc/xds/internal/clients"
)
-// LRSClient is an LRS client.
+// LRSClient is an LRS (Load Reporting Service) client.
type LRSClient struct {
}
@@ -37,7 +37,7 @@ func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {
panic("unimplemented")
}
-// Close closes the LRS client and releases all resources.
+// Close closes the LRS client.
func (c *LRSClient) Close() error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index d4160875d758..d59dd65bc431 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -22,9 +22,10 @@ package lrsclient
import "time"
-// LoadStore keeps the loads for multiple clusters and services to be reported
-// via LRS. It contains loads reported to one LRS server. Create multiple
-// stores for multiple servers.
+// LoadStore keep track of the loads for multiple clusters and services that
+// are intended to be reported via LRS. One store contains loads reported to
+// one LRS server. To track loads for multiple servers, multiple stores can be
+// created.
type LoadStore struct {
}
diff --git a/xds/internal/clients/transport_builder.go b/xds/internal/clients/transport_builder.go
index e07606e8fc39..a80eebdae970 100644
--- a/xds/internal/clients/transport_builder.go
+++ b/xds/internal/clients/transport_builder.go
@@ -23,19 +23,19 @@ import (
)
// TransportBuilder provides the functionality to create a communication
-// channel to an xDS management server.
+// channel to an xDS or LRS server.
type TransportBuilder interface {
- // Build creates a new Transport instance to the xDS server based on the
+ // Build creates a new Transport instance to the server based on the
// provided ServerConfig.
Build(ServerConfig ServerConfig) (Transport, error)
}
-// Transport provides the functionality to communicate with an xDS management
+// Transport provides the functionality to communicate with an xDS or LRS
// server using streaming calls.
type Transport interface {
- // NewStream creates a new streaming call to the xDS server for the
- // specified RPC method name. The returned Stream interface can be used
- // to send and receive messages on the stream.
+ // NewStream creates a new streaming call to the server for the specific
+ // RPC method name. The returned Stream interface can be used to send and
+ // receive messages on the stream.
NewStream(context.Context, string) (Stream, error)
// Close closes the Transport.
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/client.go
index aeb39501dace..57c70369c5d1 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/client.go
@@ -58,13 +58,13 @@ func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceW
panic("unimplemented")
}
-// Close closes the xDS client and releases all resources.
+// Close closes the xDS client.
func (c *XDSClient) Close() error {
panic("unimplemented")
}
-// DumpResources returns the status and contents of all xDS resources from the
-// xDS client.
+// DumpResources returns the status and contents of all xDS resources being
+// watched by the xDS client.
func (c *XDSClient) DumpResources() *v3statuspb.ClientStatusResponse {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index 973af0afcf9b..dcbc597c6448 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -31,10 +31,10 @@ type Config struct {
// the data returned by those servers. xDS client use the first
// available server from the list.
//
- // See [gRFC A71] for more details on fallback behavior when the primary
+ // See gRFC A71 for more details on fallback behavior when the primary
// xDS server is unavailable.
//
- // [gRFC A71]: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
+ // gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
Servers []clients.ServerConfig
// Authorities map is used to define different authorities, in a federated
@@ -45,7 +45,7 @@ type Config struct {
// management server.
Node clients.Node
- // TransportBuilder is used to connect to the management server.
+ // TransportBuilder is used to connect to the xDS management server.
TransportBuilder clients.TransportBuilder
// ResourceTypes is a map from resource type URLs to resource type
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index 58b80805beb8..aa49ae80734e 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -25,32 +25,40 @@ import (
// ResourceType wraps all resource-type specific functionality. Each supported
// resource type needs to provide an implementation of this interface.
-type ResourceType interface {
+type ResourceType struct {
// TypeURL is the xDS type URL of this resource type for the v3 xDS
// protocol. This URL is used as the key to look up the corresponding
// ResourceType implementation in the ResourceTypes map provided in the
// Config.
- TypeURL() string
+ TypeURL string
// TypeName identifies resources in a transport protocol agnostic way. This
// can be used for logging/debugging purposes, as well as in cases where
// the resource type name is to be uniquely identified but the actual
// functionality provided by the resource type is not required.
- TypeName() string
+ TypeName string
// AllResourcesRequiredInSotW indicates whether this resource type requires
// that all resources be present in every SotW response from the server. If
// true, a response that does not include a previously seen resource will
// be interpreted as a deletion of that resource.
- AllResourcesRequiredInSotW() bool
+ AllResourcesRequiredInSotW bool
+ // Decoder is used to deserialize and validate an xDS resource received
+ // from the xDS management server.
+ Decoder Decoder
+}
+
+// Decoder wraps the resource-type specific functionality for validation
+// and deserialization.
+type Decoder interface {
// Decode deserializes and validates an xDS resource serialized inside the
// provided `Any` proto, as received from the xDS management server.
//
// If protobuf deserialization fails or resource validation fails,
// returns a non-nil error. Otherwise, returns a fully populated
// DecodeResult.
- Decode(DecodeOptions, any) (*DecodeResult, error)
+ Decode(resource any, options DecodeOptions) (*DecodeResult, error)
}
// DecodeOptions wraps the options required by ResourceType implementation for
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 8d3a7d663385..0eead5c892d9 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -37,7 +37,8 @@ type ResourceWatcher interface {
// AmbientError indicates an error occurred after a resource has been
// received that should not modify the use of that resource but may be
- // useful information about the ambient state of the XdsClient. The
- // previous version of the resource should still be considered valid.
+ // provide useful information about the ambient state of the XdsClient for
+ // debugging purposes. The previous version of the resource should still be
+ // considered valid.
AmbientError(err error, onCallbackProcessed func())
}
From 077bdd005c343cbfac50c8b29edc1bc28eecb7a3 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 18 Feb 2025 23:24:46 +0530
Subject: [PATCH 15/26] move authorities under xds client
---
xds/internal/clients/config.go | 26 ----------------------
xds/internal/clients/xdsclient/config.go | 28 +++++++++++++++++++++++-
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index dcd054e15df5..22980ad921f1 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -103,32 +103,6 @@ func (sc *ServerConfig) equal(other *ServerConfig) bool {
return false
}
-// Authority contains configuration for an xDS control plane authority.
-//
-// See: https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
-type Authority struct {
- // XDSServers contains the list of server configurations for this authority.
- // The order of the servers in this list reflects the order of preference
- // of the data returned by those servers. xDS client use the first
- // available server from the list.
- //
- // See gRFC A71 for more details on fallback behavior when the primary
- // xDS server is unavailable.
- //
- // gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
- XDSServers []ServerConfig
-
- // Extensions can be populated with arbitrary data to be passed to the xDS
- // Client's user specific implementations. This field can be used to
- // provide additional configuration or context specific to the user's
- // needs.
- //
- // The xDS and LRS clients do not interpret the contents of this field. It
- // is the responsibility of the user's implementations to handle and
- // interpret these extensions.
- Extensions any
-}
-
// Node represents the identity of the xDS client, allowing xDS and LRS servers
// to identify the source of xDS requests.
type Node struct {
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/config.go
index dcbc597c6448..03798b9efd0f 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/config.go
@@ -39,7 +39,7 @@ type Config struct {
// Authorities map is used to define different authorities, in a federated
// setup, each with its own set of xDS management servers.
- Authorities map[string]clients.Authority
+ Authorities map[string]Authority
// Node is the identity of the xDS client connecting to the xDS
// management server.
@@ -55,3 +55,29 @@ type Config struct {
// type.
ResourceTypes map[string]ResourceType
}
+
+// Authority contains configuration for an xDS control plane authority.
+//
+// See: https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
+type Authority struct {
+ // XDSServers contains the list of server configurations for this authority.
+ // The order of the servers in this list reflects the order of preference
+ // of the data returned by those servers. xDS client use the first
+ // available server from the list.
+ //
+ // See gRFC A71 for more details on fallback behavior when the primary
+ // xDS server is unavailable.
+ //
+ // gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
+ XDSServers []clients.ServerConfig
+
+ // Extensions can be populated with arbitrary data to be passed to the xDS
+ // Client's user specific implementations. This field can be used to
+ // provide additional configuration or context specific to the user's
+ // needs.
+ //
+ // The xDS and LRS clients do not interpret the contents of this field. It
+ // is the responsibility of the user's implementations to handle and
+ // interpret these extensions.
+ Extensions any
+}
From 0dd84b6fc9799056817a65a67a5ff5a55951ecaa Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Wed, 19 Feb 2025 23:57:58 +0530
Subject: [PATCH 16/26] easwar review 2
---
xds/internal/clients/lrsclient/load_store.go | 15 +++++++++++---
.../lrsclient/{client.go => lrsclient.go} | 5 +++++
.../lrsclient/{config.go => lrsconfig.go} | 4 ++--
.../clients/xdsclient/resource_type.go | 8 +++-----
.../clients/xdsclient/resource_watcher.go | 2 +-
.../xdsclient/{client.go => xdsclient.go} | 13 ++++++------
.../xdsclient/{config.go => xdsconfig.go} | 20 ++++++++++---------
7 files changed, 40 insertions(+), 27 deletions(-)
rename xds/internal/clients/lrsclient/{client.go => lrsclient.go} (90%)
rename xds/internal/clients/lrsclient/{config.go => lrsconfig.go} (90%)
rename xds/internal/clients/xdsclient/{client.go => xdsclient.go} (80%)
rename xds/internal/clients/xdsclient/{config.go => xdsconfig.go} (82%)
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index d59dd65bc431..4fc6cec62735 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -23,9 +23,12 @@ package lrsclient
import "time"
// LoadStore keep track of the loads for multiple clusters and services that
-// are intended to be reported via LRS. One store contains loads reported to
-// one LRS server. To track loads for multiple servers, multiple stores can be
-// created.
+// are intended to be reported via LRS.
+//
+// LoadStore stores loads reported to a single LRS server. Use multiple stores
+// for multiple servers.
+//
+// It is safe for concurrent use.
type LoadStore struct {
}
@@ -37,6 +40,8 @@ type LoadStore struct {
//
// If a cluster's Data is empty (no load to report), it's not appended to the
// returned slice.
+//
+// Calling Stats clears the previous load data from the LoadStore.
func (s *LoadStore) Stats(clusterNames []string) []*Data {
panic("unimplemented")
}
@@ -95,8 +100,12 @@ type ServerLoadData struct {
// PerClusterReporter defines the methods that the LoadStore uses to track
// per-cluster load reporting data.
type PerClusterReporter interface {
+ // CallStarted records a call started in the LoadStore.
CallStarted(locality string)
+ // CallFinished records a call finished in the LoadStore.
CallFinished(locality string, err error)
+ // CallServerLoad records the server load in the LoadStore.
CallServerLoad(locality, name string, val float64)
+ // CallDropped records a call dropped in the LoadStore.
CallDropped(category string)
}
diff --git a/xds/internal/clients/lrsclient/client.go b/xds/internal/clients/lrsclient/lrsclient.go
similarity index 90%
rename from xds/internal/clients/lrsclient/client.go
rename to xds/internal/clients/lrsclient/lrsclient.go
index 34a6452ab952..eb4220813df2 100644
--- a/xds/internal/clients/lrsclient/client.go
+++ b/xds/internal/clients/lrsclient/lrsclient.go
@@ -31,6 +31,11 @@ import (
type LRSClient struct {
}
+// New returns a new LRS Client configured with the provided config.
+func New(config Config) (*LRSClient, error) {
+ panic("unimplemented")
+}
+
// ReportLoad creates a new load reporting stream for the client. It creates a
// LoadStore and return it for the caller to report loads.
func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {
diff --git a/xds/internal/clients/lrsclient/config.go b/xds/internal/clients/lrsclient/lrsconfig.go
similarity index 90%
rename from xds/internal/clients/lrsclient/config.go
rename to xds/internal/clients/lrsclient/lrsconfig.go
index 4a476096cdde..f0a8c0869019 100644
--- a/xds/internal/clients/lrsclient/config.go
+++ b/xds/internal/clients/lrsclient/lrsconfig.go
@@ -23,8 +23,8 @@ import (
)
// Config is used to configure an LRS client. After one has been passed to an
-// LRS function, it must not be modified. A Config may be used; the LRS package
-// will also not modify it.
+// LRS function, it must not be modified. A Config may be reused; the LRS
+// package will also not modify it.
type Config struct {
// Node is the identity of the client application reporting load to the
// LRS server.
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index aa49ae80734e..30e7fb49784f 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -24,7 +24,7 @@ import (
)
// ResourceType wraps all resource-type specific functionality. Each supported
-// resource type needs to provide an implementation of this interface.
+// resource type needs to provide an implementation of the Decoder.
type ResourceType struct {
// TypeURL is the xDS type URL of this resource type for the v3 xDS
// protocol. This URL is used as the key to look up the corresponding
@@ -32,10 +32,8 @@ type ResourceType struct {
// Config.
TypeURL string
- // TypeName identifies resources in a transport protocol agnostic way. This
- // can be used for logging/debugging purposes, as well as in cases where
- // the resource type name is to be uniquely identified but the actual
- // functionality provided by the resource type is not required.
+ // TypeName is the shorter representation of the TypeURL to identify the
+ // resource type. It can be used for logging/debugging purposes.
TypeName string
// AllResourcesRequiredInSotW indicates whether this resource type requires
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 0eead5c892d9..9fd78a7e2afc 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -37,7 +37,7 @@ type ResourceWatcher interface {
// AmbientError indicates an error occurred after a resource has been
// received that should not modify the use of that resource but may be
- // provide useful information about the ambient state of the XdsClient for
+ // provide useful information about the ambient state of the XDSClient for
// debugging purposes. The previous version of the resource should still be
// considered valid.
AmbientError(err error, onCallbackProcessed func())
diff --git a/xds/internal/clients/xdsclient/client.go b/xds/internal/clients/xdsclient/xdsclient.go
similarity index 80%
rename from xds/internal/clients/xdsclient/client.go
rename to xds/internal/clients/xdsclient/xdsclient.go
index 57c70369c5d1..601eddc45f01 100644
--- a/xds/internal/clients/xdsclient/client.go
+++ b/xds/internal/clients/xdsclient/xdsclient.go
@@ -18,7 +18,7 @@
*
*/
-// Package xdsclient provides an xDS (Extensible Discovery Service) client.
+// Package xdsclient provides an xDS (Discovery Service) client.
//
// It allows applications to:
// - Create xDS client instances with in-memory configurations.
@@ -47,13 +47,12 @@ func New(config Config) (*XDSClient, error) {
// WatchResource starts watching the specified resource.
//
-// typeURL must be present in the XDSClient's configured ResourceTypes. If
-// typeURL is not present, watch will not be started. The ResourceType
-// obtained from the ResourceTypes against the typeURL will be used to decode
-// the matching resource when it is received, and the watcher will be called
-// with the result.
+// typeURL specifies the resource type implementation to use. The watch fails
+// if there is no resource type implementation for the given typeURL. See the
+// ResourceTypes field in the Config struct used to create the XDSClient.
//
-// Cancel cancels the watch and prevents future calls to the watcher.
+// The returned function cancels the watch and prevents future calls to the
+// watcher.
func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/config.go b/xds/internal/clients/xdsclient/xdsconfig.go
similarity index 82%
rename from xds/internal/clients/xdsclient/config.go
rename to xds/internal/clients/xdsclient/xdsconfig.go
index 03798b9efd0f..2fd6c64741d5 100644
--- a/xds/internal/clients/xdsclient/config.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -23,8 +23,8 @@ import (
)
// Config is used to configure an xDS client. After one has been passed to an
-// xDS function, it must not be modified. A Config may be used; the xDS package
-// will also not modify it.
+// xDS function, it must not be modified. A Config may be reused; the xDS
+// package will also not modify it.
type Config struct {
// Servers specifies a list of xDS management servers to connect to. The
// order of the servers in this list reflects the order of preference of
@@ -71,13 +71,15 @@ type Authority struct {
// gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
XDSServers []clients.ServerConfig
- // Extensions can be populated with arbitrary data to be passed to the xDS
- // Client's user specific implementations. This field can be used to
- // provide additional configuration or context specific to the user's
- // needs.
+ // Extensions can be populated with arbitrary authority-specific data to be
+ // passed from the xDS client configuration down to the user defined
+ // resource decoder implementations. This allows the user to provide
+ // authority-specific context or configuration to their resource
+ // processing logic.
+
//
- // The xDS and LRS clients do not interpret the contents of this field. It
- // is the responsibility of the user's implementations to handle and
- // interpret these extensions.
+ // The xDS client do not interpret the contents of this field. It is the
+ // responsibility of the user's implementations to handle and interpret
+ // these extensions.
Extensions any
}
From 03b3e6d92a3061e313b6992312da2ef67121d265 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Thu, 20 Feb 2025 18:19:53 +0530
Subject: [PATCH 17/26] easwars review 4
---
xds/internal/clients/lrsclient/load_store.go | 67 ++-----------------
xds/internal/clients/lrsclient/lrsconfig.go | 6 +-
.../clients/xdsclient/resource_type.go | 2 +-
.../clients/xdsclient/resource_watcher.go | 15 +++--
xds/internal/clients/xdsclient/xdsclient.go | 2 +-
xds/internal/clients/xdsclient/xdsconfig.go | 15 ++---
6 files changed, 24 insertions(+), 83 deletions(-)
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index 4fc6cec62735..e4e6c0ccda0f 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -20,8 +20,6 @@
package lrsclient
-import "time"
-
// LoadStore keep track of the loads for multiple clusters and services that
// are intended to be reported via LRS.
//
@@ -32,73 +30,16 @@ import "time"
type LoadStore struct {
}
-// Stats returns the load data for the given cluster names. Data is returned in
-// a slice with no specific order.
-//
-// If no clusterName is given (an empty slice), all data for all known clusters
-// is returned.
-//
-// If a cluster's Data is empty (no load to report), it's not appended to the
-// returned slice.
-//
-// Calling Stats clears the previous load data from the LoadStore.
-func (s *LoadStore) Stats(clusterNames []string) []*Data {
- panic("unimplemented")
-}
-
// PerCluster returns the PerClusterReporter for the given cluster and service.
-func (s *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter {
+func (ls *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter {
panic("unimplemented")
}
-// Data contains all load data reported to the LoadStore since the most recent
-// call to Stats().
-type Data struct {
- // Cluster is the name of the cluster this data is for.
- Cluster string
- // Service is the name of the EDS service this data is for.
- Service string
- // TotalDrops is the total number of dropped requests.
- TotalDrops uint64
- // Drops is the number of dropped requests per category.
- Drops map[string]uint64
- // LocalityStats contains load reports per locality.
- LocalityStats map[string]LocalityData
- // ReportInterval is the duration over which load was reported.
- ReportInterval time.Duration
-}
-
-// LocalityData contains load data for a single locality.
-type LocalityData struct {
- // RequestStats contains counts of requests made to the locality.
- RequestStats RequestData
- // LoadStats contains server load data for requests made to the locality,
- // indexed by the load type.
- LoadStats map[string]ServerLoadData
-}
-
-// RequestData contains request counts.
-type RequestData struct {
- // Succeeded is the number of succeeded requests.
- Succeeded uint64
- // Errored is the number of requests which ran into errors.
- Errored uint64
- // InProgress is the number of requests in flight.
- InProgress uint64
- // Issued is the total number requests that were sent.
- Issued uint64
-}
-
-// ServerLoadData contains server load data.
-type ServerLoadData struct {
- // Count is the number of load reports.
- Count uint64
- // Sum is the total value of all load reports.
- Sum float64
-}
-
// PerClusterReporter defines the methods that the LoadStore uses to track
// per-cluster load reporting data.
+//
+// The lrsclient package provides an implementation of this which can be used
+// to push loads to the received LoadStore from the LRS client.
type PerClusterReporter interface {
// CallStarted records a call started in the LoadStore.
CallStarted(locality string)
diff --git a/xds/internal/clients/lrsclient/lrsconfig.go b/xds/internal/clients/lrsclient/lrsconfig.go
index f0a8c0869019..c05516dffef7 100644
--- a/xds/internal/clients/lrsclient/lrsconfig.go
+++ b/xds/internal/clients/lrsclient/lrsconfig.go
@@ -22,9 +22,9 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// Config is used to configure an LRS client. After one has been passed to an
-// LRS function, it must not be modified. A Config may be reused; the LRS
-// package will also not modify it.
+// Config is used to configure an LRS client. After one has been passed to the
+// LRS client's New function, it must not be modified. A Config may be reused;
+// the lrsclient package will also not modify it.
type Config struct {
// Node is the identity of the client application reporting load to the
// LRS server.
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index 30e7fb49784f..a9ce42f6b937 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -59,7 +59,7 @@ type Decoder interface {
Decode(resource any, options DecodeOptions) (*DecodeResult, error)
}
-// DecodeOptions wraps the options required by ResourceType implementation for
+// DecodeOptions wraps the options required by ResourceType implementations for
// decoding configuration received from the xDS management server.
type DecodeOptions struct {
// Config contains the complete configuration passed to the xDS client.
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 9fd78a7e2afc..89441a156e4c 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -22,23 +22,24 @@ package xdsclient
// corresponding to the resource being watched. gRFC A88 contains an exhaustive
// list of what method is invoked under what conditions.
//
-// onCallbackProcessed in the callbacks is a function to be invoked by
-// resource watcher implementations upon completing the processing of a
-// callback from the xDS client. Failure to invoke this callback prevents the
-// xDS client from reading further messages from the xDS server.
+// changeProcessed and errorProcessed in the callbacks are functions to be
+// invoked by resource watcher implementations upon completing the processing
+// of a callback from the xDS client for change and error respectively. Failure
+// to invoke this callback prevents the xDS client from reading further
+// messages from the xDS server.
type ResourceWatcher interface {
// ResourceChanged indicates a new version of the resource is available.
- ResourceChanged(resourceData ResourceData, onCallbackProcessed func())
+ ResourceChanged(resourceData ResourceData, changeProcessed func())
// ResourceError indicates an error occurred while trying to fetch or
// decode the associated resource. The previous version of the resource
// should be considered invalid.
- ResourceError(err error, onCallbackProcessed func())
+ ResourceError(err error, errorProcessed func())
// AmbientError indicates an error occurred after a resource has been
// received that should not modify the use of that resource but may be
// provide useful information about the ambient state of the XDSClient for
// debugging purposes. The previous version of the resource should still be
// considered valid.
- AmbientError(err error, onCallbackProcessed func())
+ AmbientError(err error, errorProcessed func())
}
diff --git a/xds/internal/clients/xdsclient/xdsclient.go b/xds/internal/clients/xdsclient/xdsclient.go
index 601eddc45f01..267475158e73 100644
--- a/xds/internal/clients/xdsclient/xdsclient.go
+++ b/xds/internal/clients/xdsclient/xdsclient.go
@@ -53,7 +53,7 @@ func New(config Config) (*XDSClient, error) {
//
// The returned function cancels the watch and prevents future calls to the
// watcher.
-func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) {
+func (c *XDSClient) WatchResource(typeURL, name string, watcher ResourceWatcher) (cancel func()) {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go
index 2fd6c64741d5..c13ddab2a57c 100644
--- a/xds/internal/clients/xdsclient/xdsconfig.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -22,9 +22,9 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// Config is used to configure an xDS client. After one has been passed to an
-// xDS function, it must not be modified. A Config may be reused; the xDS
-// package will also not modify it.
+// Config is used to configure an xDS client. After one has been passed to the
+// xDS client's New function, it must not be modified. A Config may be reused;
+// the xdsclient package will also not modify it.
type Config struct {
// Servers specifies a list of xDS management servers to connect to. The
// order of the servers in this list reflects the order of preference of
@@ -73,13 +73,12 @@ type Authority struct {
// Extensions can be populated with arbitrary authority-specific data to be
// passed from the xDS client configuration down to the user defined
- // resource decoder implementations. This allows the user to provide
+ // resource type implementations. This allows the user to provide
// authority-specific context or configuration to their resource
// processing logic.
-
//
- // The xDS client do not interpret the contents of this field. It is the
- // responsibility of the user's implementations to handle and interpret
- // these extensions.
+ // The xDS client does not interpret the contents of this field. It is the
+ // responsibility of the user's resource type implementations to handle and
+ // interpret these extensions.
Extensions any
}
From 8f7a4dfcc7372f986ad638c15a8f38a29c5ff9c5 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 25 Feb 2025 10:28:31 +0530
Subject: [PATCH 18/26] ResourceWatcher done and LoadStore stop
---
xds/internal/clients/lrsclient/load_store.go | 5 +++++
.../clients/xdsclient/resource_watcher.go | 15 +++++++--------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index e4e6c0ccda0f..6ad93ff29e33 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -50,3 +50,8 @@ type PerClusterReporter interface {
// CallDropped records a call dropped in the LoadStore.
CallDropped(category string)
}
+
+// Stop stops the LoadStore's load reporting stream.
+func (ls *LoadStore) Stop() error {
+ panic("unimplemented")
+}
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index 89441a156e4c..bc4b93328f70 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -22,24 +22,23 @@ package xdsclient
// corresponding to the resource being watched. gRFC A88 contains an exhaustive
// list of what method is invoked under what conditions.
//
-// changeProcessed and errorProcessed in the callbacks are functions to be
-// invoked by resource watcher implementations upon completing the processing
-// of a callback from the xDS client for change and error respectively. Failure
-// to invoke this callback prevents the xDS client from reading further
-// messages from the xDS server.
+// done() in the callbacks is a function to be invoked by resource watcher
+// implementations upon completing the processing of a callback from the xDS
+// client for change and error respectively. Failure to invoke this callback
+// prevents the xDS client from reading further messages from the xDS server.
type ResourceWatcher interface {
// ResourceChanged indicates a new version of the resource is available.
- ResourceChanged(resourceData ResourceData, changeProcessed func())
+ ResourceChanged(resourceData ResourceData, done func())
// ResourceError indicates an error occurred while trying to fetch or
// decode the associated resource. The previous version of the resource
// should be considered invalid.
- ResourceError(err error, errorProcessed func())
+ ResourceError(err error, done func())
// AmbientError indicates an error occurred after a resource has been
// received that should not modify the use of that resource but may be
// provide useful information about the ambient state of the XDSClient for
// debugging purposes. The previous version of the resource should still be
// considered valid.
- AmbientError(err error, errorProcessed func())
+ AmbientError(err error, done func())
}
From 48b05eb30c7d8dd0aadb3932b35a5da9df44d3b2 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Wed, 26 Feb 2025 14:21:00 +0530
Subject: [PATCH 19/26] doug
---
xds/internal/clients/lrsclient/load_store.go | 51 +++++++++++--------
xds/internal/clients/lrsclient/lrsclient.go | 20 ++++----
xds/internal/clients/lrsclient/lrsconfig.go | 4 +-
.../clients/xdsclient/resource_type.go | 31 +++++------
.../clients/xdsclient/resource_watcher.go | 21 ++++----
xds/internal/clients/xdsclient/xdsclient.go | 4 +-
xds/internal/clients/xdsclient/xdsconfig.go | 22 +++-----
7 files changed, 78 insertions(+), 75 deletions(-)
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index 6ad93ff29e33..9857eedc61ac 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -20,8 +20,8 @@
package lrsclient
-// LoadStore keep track of the loads for multiple clusters and services that
-// are intended to be reported via LRS.
+// A LoadStore aggregates loads for multiple clusters and services that are
+// intended to be reported via LRS.
//
// LoadStore stores loads reported to a single LRS server. Use multiple stores
// for multiple servers.
@@ -30,28 +30,39 @@ package lrsclient
type LoadStore struct {
}
-// PerCluster returns the PerClusterReporter for the given cluster and service.
-func (ls *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter {
+// Stop stops the LoadStore's load reporting stream.
+func (ls *LoadStore) Stop() error {
panic("unimplemented")
}
-// PerClusterReporter defines the methods that the LoadStore uses to track
-// per-cluster load reporting data.
-//
-// The lrsclient package provides an implementation of this which can be used
-// to push loads to the received LoadStore from the LRS client.
-type PerClusterReporter interface {
- // CallStarted records a call started in the LoadStore.
- CallStarted(locality string)
- // CallFinished records a call finished in the LoadStore.
- CallFinished(locality string, err error)
- // CallServerLoad records the server load in the LoadStore.
- CallServerLoad(locality, name string, val float64)
- // CallDropped records a call dropped in the LoadStore.
- CallDropped(category string)
+// ReporterForCluster returns the PerClusterReporter for the given cluster and
+// service.
+func (ls *LoadStore) ReporterForCluster(clusterName, serviceName string) PerClusterReporter {
+ panic("unimplemented")
}
-// Stop stops the LoadStore's load reporting stream.
-func (ls *LoadStore) Stop() error {
+// PerClusterReporter records load data pertaining to a single cluster. It
+// provides methods to record call starts, finishes, server-reported loads,
+// and dropped calls.
+type PerClusterReporter struct {
+}
+
+// CallStarted records a call started in the LoadStore.
+func (p *PerClusterReporter) CallStarted(locality string) {
+ panic("unimplemented")
+}
+
+// CallFinished records a call finished in the LoadStore.
+func (p *PerClusterReporter) CallFinished(locality string, err error) {
+ panic("unimplemented")
+}
+
+// CallServerLoad records the server load in the LoadStore.
+func (p *PerClusterReporter) CallServerLoad(locality, name string, val float64) {
+ panic("unimplemented")
+}
+
+// CallDropped records a call dropped in the LoadStore.
+func (p *PerClusterReporter) CallDropped(category string) {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/lrsclient.go b/xds/internal/clients/lrsclient/lrsclient.go
index eb4220813df2..44f50da7e474 100644
--- a/xds/internal/clients/lrsclient/lrsclient.go
+++ b/xds/internal/clients/lrsclient/lrsclient.go
@@ -1,5 +1,3 @@
-//revive:disable:unused-parameter
-
/*
*
* Copyright 2025 gRPC authors.
@@ -23,26 +21,26 @@
// See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
package lrsclient
-import (
- "google.golang.org/grpc/xds/internal/clients"
-)
-
// LRSClient is an LRS (Load Reporting Service) client.
type LRSClient struct {
}
// New returns a new LRS Client configured with the provided config.
-func New(config Config) (*LRSClient, error) {
+func New(_ Config) (*LRSClient, error) {
panic("unimplemented")
}
-// ReportLoad creates a new load reporting stream for the client. It creates a
-// LoadStore and return it for the caller to report loads.
-func (c *LRSClient) ReportLoad(serverConfig clients.ServerConfig) *LoadStore {
+// ReportLoad creates a new load reporting stream for the client. It creates
+// and returns a LoadStore for the caller to report loads.
+func (*LRSClient) ReportLoad() *LoadStore {
panic("unimplemented")
}
// Close closes the LRS client.
-func (c *LRSClient) Close() error {
+//
+// It blocks until all the in-flight load reporting streams of the LoadStore
+// are canceled and clean up the resources associated with the active
+// LoadStore.
+func (*LRSClient) Close() error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/lrsconfig.go b/xds/internal/clients/lrsclient/lrsconfig.go
index c05516dffef7..c4862ff76794 100644
--- a/xds/internal/clients/lrsclient/lrsconfig.go
+++ b/xds/internal/clients/lrsclient/lrsconfig.go
@@ -23,8 +23,8 @@ import (
)
// Config is used to configure an LRS client. After one has been passed to the
-// LRS client's New function, it must not be modified. A Config may be reused;
-// the lrsclient package will also not modify it.
+// LRS client's New function, no part of it may modified. A Config may be
+// reused; the lrsclient package will also not modify it.
type Config struct {
// Node is the identity of the client application reporting load to the
// LRS server.
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index a9ce42f6b937..211a717b438d 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -20,7 +20,6 @@ package xdsclient
import (
"google.golang.org/grpc/xds/internal/clients"
- "google.golang.org/protobuf/types/known/anypb"
)
// ResourceType wraps all resource-type specific functionality. Each supported
@@ -32,8 +31,8 @@ type ResourceType struct {
// Config.
TypeURL string
- // TypeName is the shorter representation of the TypeURL to identify the
- // resource type. It can be used for logging/debugging purposes.
+ // TypeName is a shorter representation of the TypeURL to identify the
+ // resource type. It is used for logging/debugging purposes.
TypeName string
// AllResourcesRequiredInSotW indicates whether this resource type requires
@@ -50,12 +49,11 @@ type ResourceType struct {
// Decoder wraps the resource-type specific functionality for validation
// and deserialization.
type Decoder interface {
- // Decode deserializes and validates an xDS resource serialized inside the
- // provided `Any` proto, as received from the xDS management server.
+ // Decode deserializes and validates an xDS resource as received from the
+ // xDS management server.
//
- // If protobuf deserialization fails or resource validation fails,
- // returns a non-nil error. Otherwise, returns a fully populated
- // DecodeResult.
+ // If deserialization fails or resource validation fails, it returns a
+ // non-nil error. Otherwise, returns a fully populated DecodeResult.
Decode(resource any, options DecodeOptions) (*DecodeResult, error)
}
@@ -66,19 +64,18 @@ type DecodeOptions struct {
// This contains useful data for resource validation.
Config *Config
- // ServerConfig contains the server config (from the above bootstrap
- // configuration) of the xDS server from which the current resource, for
- // which Decode() is being invoked, was received.
+ // ServerConfig contains the configuration of the xDS server that provided
+ // the current resource being decoded.
ServerConfig *clients.ServerConfig
}
// DecodeResult is the result of a decode operation.
type DecodeResult struct {
- // Name is the name of the resource being watched.
+ // Name is the name of the decoded resource.
Name string
- // Resource contains the configuration associated with the resource being
- // watched.
+ // Resource contains the configuration associated with the decoded
+ // resource.
Resource ResourceData
}
@@ -88,9 +85,9 @@ type DecodeResult struct {
// received from the xDS management server.
type ResourceData interface {
// RawEqual returns true if the passed in resource data is equal to that of
- // the receiver, based on the underlying raw protobuf message.
+ // the receiver, based on the underlying raw message.
RawEqual(ResourceData) bool
- // Raw returns the underlying raw protobuf form of the resource.
- Raw() *anypb.Any
+ // Raw returns the underlying raw form of the resource.
+ Raw() any
}
diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go
index bc4b93328f70..37d01bc71e76 100644
--- a/xds/internal/clients/xdsclient/resource_watcher.go
+++ b/xds/internal/clients/xdsclient/resource_watcher.go
@@ -18,14 +18,15 @@
package xdsclient
-// ResourceWatcher wraps the callbacks to be invoked for different events
-// corresponding to the resource being watched. gRFC A88 contains an exhaustive
-// list of what method is invoked under what conditions.
+// ResourceWatcher is notified of the resource updates and errors that are
+// received by the xDS client from the management server.
//
-// done() in the callbacks is a function to be invoked by resource watcher
-// implementations upon completing the processing of a callback from the xDS
-// client for change and error respectively. Failure to invoke this callback
-// prevents the xDS client from reading further messages from the xDS server.
+// All methods contain a done parameter which should be called when processing
+// of the update has completed. For example, if processing a resource requires
+// watching new resources, those watches should be completed before done is
+// called, which can happen after the ResourceWatcher method has returned.
+// Failure to call done will prevent the xDS client from providing future
+// ResourceWatcher notifications.
type ResourceWatcher interface {
// ResourceChanged indicates a new version of the resource is available.
ResourceChanged(resourceData ResourceData, done func())
@@ -36,9 +37,9 @@ type ResourceWatcher interface {
ResourceError(err error, done func())
// AmbientError indicates an error occurred after a resource has been
- // received that should not modify the use of that resource but may be
- // provide useful information about the ambient state of the XDSClient for
- // debugging purposes. The previous version of the resource should still be
+ // received that should not modify the use of that resource but may provide
+ // useful information about the state of the XDSClient for debugging
+ // purposes. The previous version of the resource should still be
// considered valid.
AmbientError(err error, done func())
}
diff --git a/xds/internal/clients/xdsclient/xdsclient.go b/xds/internal/clients/xdsclient/xdsclient.go
index 267475158e73..a363326b08b3 100644
--- a/xds/internal/clients/xdsclient/xdsclient.go
+++ b/xds/internal/clients/xdsclient/xdsclient.go
@@ -18,12 +18,14 @@
*
*/
-// Package xdsclient provides an xDS (Discovery Service) client.
+// Package xdsclient provides an xDS (*Discovery Service) client.
//
// It allows applications to:
// - Create xDS client instances with in-memory configurations.
// - Register watches for named resources.
// - Receive resources via an ADS (Aggregated Discovery Service) stream.
+// - Register watches for named resources (e.g. listeners, routes, or
+// clusters).
//
// This enables applications to dynamically discover and configure resources
// such as listeners, routes, clusters, and endpoints from an xDS management
diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go
index c13ddab2a57c..6389817cdbd5 100644
--- a/xds/internal/clients/xdsclient/xdsconfig.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -23,12 +23,12 @@ import (
)
// Config is used to configure an xDS client. After one has been passed to the
-// xDS client's New function, it must not be modified. A Config may be reused;
-// the xdsclient package will also not modify it.
+// xDS client's New function, no part of it may be modified. A Config may be
+// reused; the xdsclient package will also not modify it.
type Config struct {
// Servers specifies a list of xDS management servers to connect to. The
// order of the servers in this list reflects the order of preference of
- // the data returned by those servers. xDS client use the first
+ // the data returned by those servers. The xDS client uses the first
// available server from the list.
//
// See gRFC A71 for more details on fallback behavior when the primary
@@ -37,15 +37,15 @@ type Config struct {
// gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
Servers []clients.ServerConfig
- // Authorities map is used to define different authorities, in a federated
- // setup, each with its own set of xDS management servers.
+ // Authorities defines the configuration for each xDS authority. Federated resources
+ // will be fetched from the servers specified by the corresponding Authority.
Authorities map[string]Authority
// Node is the identity of the xDS client connecting to the xDS
// management server.
Node clients.Node
- // TransportBuilder is used to connect to the xDS management server.
+ // TransportBuilder is used to create connections to xDS management servers.
TransportBuilder clients.TransportBuilder
// ResourceTypes is a map from resource type URLs to resource type
@@ -58,17 +58,11 @@ type Config struct {
// Authority contains configuration for an xDS control plane authority.
//
-// See: https://github.com/grpc/grpc/blob/master/doc/grpc_xds_bootstrap_format.md
+// See: https://www.envoyproxy.io/docs/envoy/latest/xds/core/v3/resource_locator.proto#xds-core-v3-resourcelocator
type Authority struct {
// XDSServers contains the list of server configurations for this authority.
- // The order of the servers in this list reflects the order of preference
- // of the data returned by those servers. xDS client use the first
- // available server from the list.
//
- // See gRFC A71 for more details on fallback behavior when the primary
- // xDS server is unavailable.
- //
- // gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
+ // See Config.Servers for more details.
XDSServers []clients.ServerConfig
// Extensions can be populated with arbitrary authority-specific data to be
From 0418208001a23d5ed758b6bb60b17d2cbfb2db42 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Thu, 27 Feb 2025 15:10:42 +0530
Subject: [PATCH 20/26] address godoc review comments
---
xds/internal/clients/config.go | 28 +++----
xds/internal/clients/config_test.go | 80 +++++++++----------
xds/internal/clients/lrsclient/load_store.go | 12 ++-
xds/internal/clients/lrsclient/lrsclient.go | 17 ++--
xds/internal/clients/lrsclient/lrsconfig.go | 6 ++
xds/internal/clients/transport_builder.go | 4 +-
.../clients/xdsclient/resource_type.go | 8 +-
xds/internal/clients/xdsclient/xdsclient.go | 6 +-
xds/internal/clients/xdsclient/xdsconfig.go | 32 +++++---
9 files changed, 91 insertions(+), 102 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index 22980ad921f1..af92d8724369 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -51,20 +51,12 @@ import (
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)
-// ServerConfig holds settings for connecting to an xDS management or LRS server.
-type ServerConfig struct {
+// ServerIdentifier holds identifying information for connecting to an xDS
+// management or LRS server.
+type ServerIdentifier struct {
// ServerURI is the target URI of the server.
ServerURI string
- // IgnoreResourceDeletion is a server feature which if set to true,
- // indicates that resource deletion errors from xDS management servers can
- // be ignored and cached resource data can be used.
- //
- // This will be removed in the future once we implement gRFC A88
- // and two new fields FailOnDataErrors and
- // ResourceTimerIsTransientError will be introduced.
- IgnoreResourceDeletion bool
-
// Extensions can be populated with arbitrary data to be passed to the
// TransportBuilder and/or xDS Client's ResourceType implementations.
// This field can be used to provide additional configuration or context
@@ -83,21 +75,19 @@ type ServerConfig struct {
}
// equal returns true if sc and other are considered equal.
-func (sc *ServerConfig) equal(other *ServerConfig) bool {
+func (si *ServerIdentifier) equal(other *ServerIdentifier) bool {
switch {
- case sc == nil && other == nil:
+ case si == nil && other == nil:
return true
- case (sc != nil) != (other != nil):
- return false
- case sc.ServerURI != other.ServerURI:
+ case (si != nil) != (other != nil):
return false
- case sc.IgnoreResourceDeletion != other.IgnoreResourceDeletion:
+ case si.ServerURI != other.ServerURI:
return false
}
- if sc.Extensions == nil && other.Extensions == nil {
+ if si.Extensions == nil && other.Extensions == nil {
return true
}
- if ex, ok := sc.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
+ if ex, ok := si.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
return true
}
return false
diff --git a/xds/internal/clients/config_test.go b/xds/internal/clients/config_test.go
index 48417db034d4..5ca80375d99e 100644
--- a/xds/internal/clients/config_test.go
+++ b/xds/internal/clients/config_test.go
@@ -36,10 +36,10 @@ func Test(t *testing.T) {
grpctest.RunSubTests(t, s{})
}
-type testServerConfigExtension struct{ x int }
+type testServerIdentifierExtension struct{ x int }
-func (ts testServerConfigExtension) Equal(other any) bool {
- ots, ok := other.(testServerConfigExtension)
+func (ts testServerIdentifierExtension) Equal(other any) bool {
+ ots, ok := other.(testServerIdentifierExtension)
if !ok {
return false
}
@@ -56,11 +56,11 @@ func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct
return ret
}
-func (s) TestServerConfig_Equal(t *testing.T) {
+func (s) TestServerIdentifier_Equal(t *testing.T) {
tests := []struct {
name string
- s1 *ServerConfig
- s2 *ServerConfig
+ s1 *ServerIdentifier
+ s2 *ServerIdentifier
wantEq bool
}{
{
@@ -72,104 +72,96 @@ func (s) TestServerConfig_Equal(t *testing.T) {
{
name: "one_nil",
s1: nil,
- s2: &ServerConfig{},
+ s2: &ServerIdentifier{},
wantEq: false,
},
{
name: "other_nil",
- s1: &ServerConfig{},
+ s1: &ServerIdentifier{},
s2: nil,
wantEq: false,
},
{
name: "both_empty_and_equal",
- s1: &ServerConfig{},
- s2: &ServerConfig{},
+ s1: &ServerIdentifier{},
+ s2: &ServerIdentifier{},
wantEq: true,
},
{
name: "different_ServerURI",
- s1: &ServerConfig{ServerURI: "foo"},
- s2: &ServerConfig{ServerURI: "bar"},
- wantEq: false,
- },
- {
- name: "different_IgnoreResourceDeletion",
- s1: &ServerConfig{IgnoreResourceDeletion: true},
- s2: &ServerConfig{},
+ s1: &ServerIdentifier{ServerURI: "foo"},
+ s2: &ServerIdentifier{ServerURI: "bar"},
wantEq: false,
},
{
name: "different_Extensions_with_no_Equal_method",
- s1: &ServerConfig{
+ s1: &ServerIdentifier{
Extensions: 1,
},
- s2: &ServerConfig{
+ s2: &ServerIdentifier{
Extensions: 2,
},
wantEq: false, // By default, if there's no Equal method, they are unequal
},
{
name: "same_Extensions_with_no_Equal_method",
- s1: &ServerConfig{
+ s1: &ServerIdentifier{
Extensions: 1,
},
- s2: &ServerConfig{
+ s2: &ServerIdentifier{
Extensions: 1,
},
wantEq: false, // By default, if there's no Equal method, they are unequal
},
{
name: "different_Extensions_with_Equal_method",
- s1: &ServerConfig{
- Extensions: testServerConfigExtension{1},
+ s1: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerConfig{
- Extensions: testServerConfigExtension{2},
+ s2: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{2},
},
wantEq: false,
},
{
name: "same_Extensions_same_with_Equal_method",
- s1: &ServerConfig{
- Extensions: testServerConfigExtension{1},
+ s1: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerConfig{
- Extensions: testServerConfigExtension{1},
+ s2: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{1},
},
wantEq: true,
},
{
name: "first_config_Extensions_is_nil",
- s1: &ServerConfig{
- Extensions: testServerConfigExtension{1},
+ s1: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerConfig{
+ s2: &ServerIdentifier{
Extensions: nil,
},
wantEq: false,
},
{
name: "other_config_Extensions_is_nil",
- s1: &ServerConfig{
+ s1: &ServerIdentifier{
Extensions: nil,
},
- s2: &ServerConfig{
- Extensions: testServerConfigExtension{2},
+ s2: &ServerIdentifier{
+ Extensions: testServerIdentifierExtension{2},
},
wantEq: false,
},
{
name: "all_fields_same",
- s1: &ServerConfig{
- ServerURI: "foo",
- IgnoreResourceDeletion: true,
- Extensions: testServerConfigExtension{1},
+ s1: &ServerIdentifier{
+ ServerURI: "foo",
+ Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerConfig{
- ServerURI: "foo",
- IgnoreResourceDeletion: true,
- Extensions: testServerConfigExtension{1},
+ s2: &ServerIdentifier{
+ ServerURI: "foo",
+ Extensions: testServerIdentifierExtension{1},
},
wantEq: true,
},
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index 9857eedc61ac..f973880dc606 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -30,7 +30,17 @@ package lrsclient
type LoadStore struct {
}
-// Stop stops the LoadStore's load reporting stream.
+// Stop stops the LRS stream associated with this LoadStore.
+//
+// If this LoadStore is the only one using the underlying LRS stream, the
+// stream will be closed. If other LoadStores are also using the same stream,
+// the reference count to the stream is decremented, and the stream remains
+// open until all LoadStores have called Stop().
+//
+// If the stream is being closed, this method attempts to flush any remaining
+// load data to the LRS server within the specified ReportLoadTimeout in the
+// client's Config. If the data cannot be written within the timeout, the
+// stream is canceled without flushing the data.
func (ls *LoadStore) Stop() error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/lrsclient.go b/xds/internal/clients/lrsclient/lrsclient.go
index 44f50da7e474..5bd8aa60dcc2 100644
--- a/xds/internal/clients/lrsclient/lrsclient.go
+++ b/xds/internal/clients/lrsclient/lrsclient.go
@@ -21,6 +21,8 @@
// See: https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/load_stats/v3/lrs.proto
package lrsclient
+import "google.golang.org/grpc/xds/internal/clients"
+
// LRSClient is an LRS (Load Reporting Service) client.
type LRSClient struct {
}
@@ -30,17 +32,8 @@ func New(_ Config) (*LRSClient, error) {
panic("unimplemented")
}
-// ReportLoad creates a new load reporting stream for the client. It creates
-// and returns a LoadStore for the caller to report loads.
-func (*LRSClient) ReportLoad() *LoadStore {
- panic("unimplemented")
-}
-
-// Close closes the LRS client.
-//
-// It blocks until all the in-flight load reporting streams of the LoadStore
-// are canceled and clean up the resources associated with the active
-// LoadStore.
-func (*LRSClient) Close() error {
+// ReportLoad creates a new load reporting stream for the provided server. It
+// creates and returns a LoadStore for the caller to report loads.
+func (*LRSClient) ReportLoad(_ clients.ServerIdentifier) *LoadStore {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/lrsconfig.go b/xds/internal/clients/lrsclient/lrsconfig.go
index c4862ff76794..75f014f13f66 100644
--- a/xds/internal/clients/lrsclient/lrsconfig.go
+++ b/xds/internal/clients/lrsclient/lrsconfig.go
@@ -19,6 +19,8 @@
package lrsclient
import (
+ "time"
+
"google.golang.org/grpc/xds/internal/clients"
)
@@ -32,4 +34,8 @@ type Config struct {
// TransportBuilder is used to connect to the LRS server.
TransportBuilder clients.TransportBuilder
+
+ // ReportLoadTimeout is the timeout used when reporting load to the LRS
+ // server.
+ ReportLoadTimeout time.Duration
}
diff --git a/xds/internal/clients/transport_builder.go b/xds/internal/clients/transport_builder.go
index a80eebdae970..2a2ab850db1b 100644
--- a/xds/internal/clients/transport_builder.go
+++ b/xds/internal/clients/transport_builder.go
@@ -26,8 +26,8 @@ import (
// channel to an xDS or LRS server.
type TransportBuilder interface {
// Build creates a new Transport instance to the server based on the
- // provided ServerConfig.
- Build(ServerConfig ServerConfig) (Transport, error)
+ // provided ServerIdentifier.
+ Build(ServerIdentifier ServerIdentifier) (Transport, error)
}
// Transport provides the functionality to communicate with an xDS or LRS
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index 211a717b438d..b2d157fda899 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -18,10 +18,6 @@
package xdsclient
-import (
- "google.golang.org/grpc/xds/internal/clients"
-)
-
// ResourceType wraps all resource-type specific functionality. Each supported
// resource type needs to provide an implementation of the Decoder.
type ResourceType struct {
@@ -66,7 +62,7 @@ type DecodeOptions struct {
// ServerConfig contains the configuration of the xDS server that provided
// the current resource being decoded.
- ServerConfig *clients.ServerConfig
+ ServerConfig *ServerConfig
}
// DecodeResult is the result of a decode operation.
@@ -89,5 +85,5 @@ type ResourceData interface {
RawEqual(ResourceData) bool
// Raw returns the underlying raw form of the resource.
- Raw() any
+ Raw() []byte
}
diff --git a/xds/internal/clients/xdsclient/xdsclient.go b/xds/internal/clients/xdsclient/xdsclient.go
index a363326b08b3..667941cd6fa4 100644
--- a/xds/internal/clients/xdsclient/xdsclient.go
+++ b/xds/internal/clients/xdsclient/xdsclient.go
@@ -32,10 +32,6 @@
// server.
package xdsclient
-import (
- v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3"
-)
-
// XDSClient is a client which queries a set of discovery APIs (collectively
// termed as xDS) on a remote management server, to discover
// various dynamic resources.
@@ -66,6 +62,6 @@ func (c *XDSClient) Close() error {
// DumpResources returns the status and contents of all xDS resources being
// watched by the xDS client.
-func (c *XDSClient) DumpResources() *v3statuspb.ClientStatusResponse {
+func (c *XDSClient) DumpResources() []byte {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go
index 6389817cdbd5..e01ff2b16ce6 100644
--- a/xds/internal/clients/xdsclient/xdsconfig.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -35,7 +35,7 @@ type Config struct {
// xDS server is unavailable.
//
// gRFC A71: https://github.com/grpc/proposal/blob/master/A71-xds-fallback.md
- Servers []clients.ServerConfig
+ Servers []ServerConfig
// Authorities defines the configuration for each xDS authority. Federated resources
// will be fetched from the servers specified by the corresponding Authority.
@@ -53,9 +53,26 @@ type Config struct {
// kind of xDS resource, and the corresponding resource type implementation
// provides logic for parsing, validating, and processing resources of that
// type.
+ //
+ // "type.googleapis.com/envoy.service.discovery.v3.Resource" is an example
+ // URL for listener resource type.
ResourceTypes map[string]ResourceType
}
+// ServerConfig contains configuration for an xDS management server.
+type ServerConfig struct {
+ ServerIdentifier clients.ServerIdentifier
+
+ // IgnoreResourceDeletion is a server feature which if set to true,
+ // indicates that resource deletion errors from xDS management servers can
+ // be ignored and cached resource data can be used.
+ //
+ // This will be removed in the future once we implement gRFC A88
+ // and two new fields FailOnDataErrors and
+ // ResourceTimerIsTransientError will be introduced.
+ IgnoreResourceDeletion bool
+}
+
// Authority contains configuration for an xDS control plane authority.
//
// See: https://www.envoyproxy.io/docs/envoy/latest/xds/core/v3/resource_locator.proto#xds-core-v3-resourcelocator
@@ -63,16 +80,5 @@ type Authority struct {
// XDSServers contains the list of server configurations for this authority.
//
// See Config.Servers for more details.
- XDSServers []clients.ServerConfig
-
- // Extensions can be populated with arbitrary authority-specific data to be
- // passed from the xDS client configuration down to the user defined
- // resource type implementations. This allows the user to provide
- // authority-specific context or configuration to their resource
- // processing logic.
- //
- // The xDS client does not interpret the contents of this field. It is the
- // responsibility of the user's resource type implementations to handle and
- // interpret these extensions.
- Extensions any
+ XDSServers []ServerConfig
}
From 1569a116ddf90bf253cf4a7c3256d91883be1d5c Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Fri, 28 Feb 2025 14:57:04 +0530
Subject: [PATCH 21/26] dfawley nits
---
xds/internal/clients/transport_builder.go | 2 +-
xds/internal/clients/xdsclient/xdsclient.go | 2 +-
xds/internal/clients/xdsclient/xdsconfig.go | 5 +++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/xds/internal/clients/transport_builder.go b/xds/internal/clients/transport_builder.go
index 2a2ab850db1b..10a25fcab1dc 100644
--- a/xds/internal/clients/transport_builder.go
+++ b/xds/internal/clients/transport_builder.go
@@ -27,7 +27,7 @@ import (
type TransportBuilder interface {
// Build creates a new Transport instance to the server based on the
// provided ServerIdentifier.
- Build(ServerIdentifier ServerIdentifier) (Transport, error)
+ Build(serverIdentifier ServerIdentifier) (Transport, error)
}
// Transport provides the functionality to communicate with an xDS or LRS
diff --git a/xds/internal/clients/xdsclient/xdsclient.go b/xds/internal/clients/xdsclient/xdsclient.go
index 667941cd6fa4..f893e8c925fc 100644
--- a/xds/internal/clients/xdsclient/xdsclient.go
+++ b/xds/internal/clients/xdsclient/xdsclient.go
@@ -18,7 +18,7 @@
*
*/
-// Package xdsclient provides an xDS (*Discovery Service) client.
+// Package xdsclient provides an xDS (* Discovery Service) client.
//
// It allows applications to:
// - Create xDS client instances with in-memory configurations.
diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go
index e01ff2b16ce6..db2191904fb8 100644
--- a/xds/internal/clients/xdsclient/xdsconfig.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -54,8 +54,7 @@ type Config struct {
// provides logic for parsing, validating, and processing resources of that
// type.
//
- // "type.googleapis.com/envoy.service.discovery.v3.Resource" is an example
- // URL for listener resource type.
+ // For example: "type.googleapis.com/envoy.config.listener.v3.Listener"
ResourceTypes map[string]ResourceType
}
@@ -70,6 +69,8 @@ type ServerConfig struct {
// This will be removed in the future once we implement gRFC A88
// and two new fields FailOnDataErrors and
// ResourceTimerIsTransientError will be introduced.
+ //
+ // TODO: Link to gRFC A88
IgnoreResourceDeletion bool
}
From 0d234e0e121875bfbfb343e40ed4e4d3b624c1eb Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Fri, 28 Feb 2025 15:46:29 +0530
Subject: [PATCH 22/26] rebase over grpctransport changes
---
xds/internal/clients/config.go | 13 +++++-
.../clients/grpctransport/grpc_transport.go | 32 +++++++--------
.../grpctransport/grpc_transport_test.go | 40 ++++++++++---------
.../clients/xdsclient/resource_type.go | 5 ++-
xds/internal/clients/xdsclient/xdsconfig.go | 4 +-
5 files changed, 55 insertions(+), 39 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index af92d8724369..7321702311ea 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -43,7 +43,9 @@
package clients
import (
+ "fmt"
"slices"
+ "strings"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"
@@ -70,10 +72,19 @@ type ServerIdentifier struct {
// configure a specific security credentials.
//
// Note: For custom types used in Extensions, ensure an Equal(any) bool
- // method is implemented for equality checks on ServerConfig.
+ // method is implemented for equality checks on ServerIdentifier.
Extensions any
}
+// String returns a string representation of the ServerIdentifier.
+//
+// WARNING: This method is primarily intended for logging and testing
+// purposes. The output returned by this method is not guaranteed to be stable
+// and may change at any time. Do not rely on it for production use.
+func (si *ServerIdentifier) String() string {
+ return strings.Join([]string{si.ServerURI, fmt.Sprintf("%v", si.Extensions)}, "-")
+}
+
// equal returns true if sc and other are considered equal.
func (si *ServerIdentifier) equal(other *ServerIdentifier) bool {
switch {
diff --git a/xds/internal/clients/grpctransport/grpc_transport.go b/xds/internal/clients/grpctransport/grpc_transport.go
index c5c1f99694ba..9040be3cd673 100644
--- a/xds/internal/clients/grpctransport/grpc_transport.go
+++ b/xds/internal/clients/grpctransport/grpc_transport.go
@@ -31,38 +31,38 @@ import (
"google.golang.org/grpc/xds/internal/clients"
)
-// ServerConfigExtension holds settings for connecting to a gRPC server,
+// ServerIdentifierExtension holds settings for connecting to a gRPC server,
// such as an xDS management or an LRS server.
-type ServerConfigExtension struct {
+type ServerIdentifierExtension struct {
// Credentials will be used for all gRPC transports. If it is unset,
// transport creation will fail.
Credentials credentials.Bundle
}
-// Builder creates gRPC-based Transports. It must be paired with ServerConfigs
-// that contain an Extension field of type ServerConfigExtension.
+// Builder creates gRPC-based Transports. It must be paired with ServerIdentifiers
+// that contain an Extension field of type ServerIdentifierExtension.
type Builder struct{}
// Build returns a gRPC-based clients.Transport.
//
-// The Extension field of the ServerConfig must be a ServerConfigExtension.
-func (b *Builder) Build(sc clients.ServerConfig) (clients.Transport, error) {
- if sc.ServerURI == "" {
- return nil, fmt.Errorf("grpctransport: ServerURI is not set in ServerConfig")
+// The Extension field of the ServerIdentifier must be a ServerIdentifierExtension.
+func (b *Builder) Build(si clients.ServerIdentifier) (clients.Transport, error) {
+ if si.ServerURI == "" {
+ return nil, fmt.Errorf("grpctransport: ServerURI is not set in ServerIdentifier")
}
- if sc.Extensions == nil {
- return nil, fmt.Errorf("grpctransport: Extensions is not set in ServerConfig")
+ if si.Extensions == nil {
+ return nil, fmt.Errorf("grpctransport: Extensions is not set in ServerIdentifier")
}
- sce, ok := sc.Extensions.(ServerConfigExtension)
+ sce, ok := si.Extensions.(ServerIdentifierExtension)
if !ok {
- return nil, fmt.Errorf("grpctransport: Extensions field is %T, but must be %T in ServerConfig", sc.Extensions, ServerConfigExtension{})
+ return nil, fmt.Errorf("grpctransport: Extensions field is %T, but must be %T in ServerIdentifier", si.Extensions, ServerIdentifierExtension{})
}
if sce.Credentials == nil {
- return nil, fmt.Errorf("grptransport: Credentials field is not set in ServerConfigExtension")
+ return nil, fmt.Errorf("grptransport: Credentials field is not set in ServerIdentifierExtension")
}
// TODO: Incorporate reference count map for existing transports and
- // deduplicate transports based on the provided ServerConfig so that
+ // deduplicate transports based on the provided ServerIdentifier so that
// transport channel to same server can be shared between xDS and LRS
// client.
@@ -74,9 +74,9 @@ func (b *Builder) Build(sc clients.ServerConfig) (clients.Transport, error) {
Time: 5 * time.Minute,
Timeout: 20 * time.Second,
})
- cc, err := grpc.NewClient(sc.ServerURI, kpCfg, grpc.WithCredentialsBundle(sce.Credentials), grpc.WithDefaultCallOptions(grpc.ForceCodec(&byteCodec{})))
+ cc, err := grpc.NewClient(si.ServerURI, kpCfg, grpc.WithCredentialsBundle(sce.Credentials), grpc.WithDefaultCallOptions(grpc.ForceCodec(&byteCodec{})))
if err != nil {
- return nil, fmt.Errorf("grpctransport: failed to create transport to server %q: %v", sc.ServerURI, err)
+ return nil, fmt.Errorf("grpctransport: failed to create transport to server %q: %v", si.ServerURI, err)
}
return &grpcTransport{cc: cc}, nil
diff --git a/xds/internal/clients/grpctransport/grpc_transport_test.go b/xds/internal/clients/grpctransport/grpc_transport_test.go
index 0ab48707c05e..cb308d8e60ef 100644
--- a/xds/internal/clients/grpctransport/grpc_transport_test.go
+++ b/xds/internal/clients/grpctransport/grpc_transport_test.go
@@ -105,6 +105,8 @@ func (s *testServer) StreamAggregatedResources(stream v3discoverygrpc.Aggregated
return err // Handle other errors
}
+ // Push received request for client to verify the correct request was
+ // received.
select {
case s.requestChan <- req:
case <-ctx.Done():
@@ -130,9 +132,9 @@ func (tc *testCredentials) TransportCredentials() credentials.TransportCredentia
// TestBuild_Success verifies that the Builder successfully creates a new
// Transport with a non-nil grpc.ClientConn.
func (s) TestBuild_Success(t *testing.T) {
- serverCfg := clients.ServerConfig{
+ serverCfg := clients.ServerIdentifier{
ServerURI: "server-address",
- Extensions: ServerConfigExtension{Credentials: &testCredentials{transportCredentials: local.NewCredentials()}},
+ Extensions: ServerIdentifierExtension{Credentials: &testCredentials{transportCredentials: local.NewCredentials()}},
}
b := &Builder{}
@@ -151,41 +153,41 @@ func (s) TestBuild_Success(t *testing.T) {
}
// TestBuild_Failure verifies that the Builder returns error when incorrect
-// ServerConfig is provided.
+// ServerIdentifier is provided.
//
// It covers the following scenarios:
// - ServerURI is empty.
// - Extensions is nil.
-// - Extensions is not ServerConfigExtension.
+// - Extensions is not ServerIdentifierExtension.
// - Credentials are nil.
func (s) TestBuild_Failure(t *testing.T) {
tests := []struct {
name string
- serverCfg clients.ServerConfig
+ serverCfg clients.ServerIdentifier
}{
{
name: "ServerURI is empty",
- serverCfg: clients.ServerConfig{
+ serverCfg: clients.ServerIdentifier{
ServerURI: "",
- Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+ Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
},
},
{
name: "Extensions is nil",
- serverCfg: clients.ServerConfig{ServerURI: "server-address"},
+ serverCfg: clients.ServerIdentifier{ServerURI: "server-address"},
},
{
- name: "Extensions is not a ServerConfigExtension",
- serverCfg: clients.ServerConfig{
+ name: "Extensions is not a ServerIdentifierExtension",
+ serverCfg: clients.ServerIdentifier{
ServerURI: "server-address",
Extensions: 1,
},
},
{
- name: "ServerConfigExtension Credentials is nil",
- serverCfg: clients.ServerConfig{
+ name: "ServerIdentifierExtension Credentials is nil",
+ serverCfg: clients.ServerIdentifier{
ServerURI: "server-address",
- Extensions: ServerConfigExtension{},
+ Extensions: ServerIdentifierExtension{},
},
},
}
@@ -208,9 +210,9 @@ func (s) TestBuild_Failure(t *testing.T) {
func (s) TestNewStream_Success(t *testing.T) {
ts := setupTestServer(t, &v3discoverypb.DiscoveryResponse{VersionInfo: "1"})
- serverCfg := clients.ServerConfig{
+ serverCfg := clients.ServerIdentifier{
ServerURI: ts.address,
- Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+ Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
}
builder := Builder{}
transport, err := builder.Build(serverCfg)
@@ -229,9 +231,9 @@ func (s) TestNewStream_Success(t *testing.T) {
// TestNewStream_Error verifies that NewStream() returns an error
// when attempting to create a stream with an invalid server URI.
func (s) TestNewStream_Error(t *testing.T) {
- serverCfg := clients.ServerConfig{
+ serverCfg := clients.ServerIdentifier{
ServerURI: "invalid-server-uri",
- Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+ Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
}
builder := Builder{}
transport, err := builder.Build(serverCfg)
@@ -262,9 +264,9 @@ func (s) TestStream_SendAndRecv(t *testing.T) {
ts := setupTestServer(t, &v3discoverypb.DiscoveryResponse{VersionInfo: "1"})
// Build a grpc-based transport to the above server.
- serverCfg := clients.ServerConfig{
+ serverCfg := clients.ServerIdentifier{
ServerURI: ts.address,
- Extensions: ServerConfigExtension{Credentials: insecure.NewBundle()},
+ Extensions: ServerIdentifierExtension{Credentials: insecure.NewBundle()},
}
builder := Builder{}
transport, err := builder.Build(serverCfg)
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index b2d157fda899..fe28d85c5397 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -27,6 +27,9 @@ type ResourceType struct {
// Config.
TypeURL string
+ // TODO: Revisit if we need TypeURL to be part of the struct because it is
+ // already a key in config's ResouceTypes map.
+
// TypeName is a shorter representation of the TypeURL to identify the
// resource type. It is used for logging/debugging purposes.
TypeName string
@@ -50,7 +53,7 @@ type Decoder interface {
//
// If deserialization fails or resource validation fails, it returns a
// non-nil error. Otherwise, returns a fully populated DecodeResult.
- Decode(resource any, options DecodeOptions) (*DecodeResult, error)
+ Decode(resource []byte, options DecodeOptions) (*DecodeResult, error)
}
// DecodeOptions wraps the options required by ResourceType implementations for
diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go
index db2191904fb8..bfbe41679ed4 100644
--- a/xds/internal/clients/xdsclient/xdsconfig.go
+++ b/xds/internal/clients/xdsclient/xdsconfig.go
@@ -69,9 +69,9 @@ type ServerConfig struct {
// This will be removed in the future once we implement gRFC A88
// and two new fields FailOnDataErrors and
// ResourceTimerIsTransientError will be introduced.
- //
- // TODO: Link to gRFC A88
IgnoreResourceDeletion bool
+
+ // TODO: Link to gRFC A88
}
// Authority contains configuration for an xDS control plane authority.
From 06081871a51c7a8a8d94e218107be81ca6e62db8 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Mon, 3 Mar 2025 10:31:38 +0530
Subject: [PATCH 23/26] Move clients config helpers to clients/internal
---
xds/internal/clients/config.go | 83 ------------
.../internal/clientsutils/clientsutils.go | 107 +++++++++++++++
.../clientsutils/clientsutils_test.go} | 127 +++++++++---------
3 files changed, 173 insertions(+), 144 deletions(-)
create mode 100644 xds/internal/clients/internal/clientsutils/clientsutils.go
rename xds/internal/clients/{config_test.go => internal/clientsutils/clientsutils_test.go} (67%)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index 7321702311ea..568e160ce3f4 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -42,17 +42,6 @@
// in a later release.
package clients
-import (
- "fmt"
- "slices"
- "strings"
-
- "google.golang.org/protobuf/proto"
- "google.golang.org/protobuf/types/known/structpb"
-
- v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-)
-
// ServerIdentifier holds identifying information for connecting to an xDS
// management or LRS server.
type ServerIdentifier struct {
@@ -76,34 +65,6 @@ type ServerIdentifier struct {
Extensions any
}
-// String returns a string representation of the ServerIdentifier.
-//
-// WARNING: This method is primarily intended for logging and testing
-// purposes. The output returned by this method is not guaranteed to be stable
-// and may change at any time. Do not rely on it for production use.
-func (si *ServerIdentifier) String() string {
- return strings.Join([]string{si.ServerURI, fmt.Sprintf("%v", si.Extensions)}, "-")
-}
-
-// equal returns true if sc and other are considered equal.
-func (si *ServerIdentifier) equal(other *ServerIdentifier) bool {
- switch {
- case si == nil && other == nil:
- return true
- case (si != nil) != (other != nil):
- return false
- case si.ServerURI != other.ServerURI:
- return false
- }
- if si.Extensions == nil && other.Extensions == nil {
- return true
- }
- if ex, ok := si.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(other.Extensions) {
- return true
- }
- return false
-}
-
// Node represents the identity of the xDS client, allowing xDS and LRS servers
// to identify the source of xDS requests.
type Node struct {
@@ -121,40 +82,6 @@ type Node struct {
UserAgentName string
// UserAgentVersion is the user agent version of application.
UserAgentVersion string
- // clientFeatures is a list of xDS features supported by this client.
- // These features are set within the xDS client, but may be overridden only
- // for testing purposes.
- clientFeatures []string
-}
-
-// toProto converts an instance of Node to its protobuf representation.
-func (n Node) toProto() *v3corepb.Node {
- return &v3corepb.Node{
- Id: n.ID,
- Cluster: n.Cluster,
- Locality: func() *v3corepb.Locality {
- if n.Locality.isEmpty() {
- return nil
- }
- return &v3corepb.Locality{
- Region: n.Locality.Region,
- Zone: n.Locality.Zone,
- SubZone: n.Locality.SubZone,
- }
- }(),
- Metadata: func() *structpb.Struct {
- if n.Metadata == nil {
- return nil
- }
- if md, ok := n.Metadata.(*structpb.Struct); ok {
- return proto.Clone(md).(*structpb.Struct)
- }
- return nil
- }(),
- UserAgentName: n.UserAgentName,
- UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
- ClientFeatures: slices.Clone(n.clientFeatures),
- }
}
// Locality represents the location of the xDS client application.
@@ -166,13 +93,3 @@ type Locality struct {
// SubZone is the further subdivision within a zone.
SubZone string
}
-
-// isEmpty reports whether l is considered empty.
-func (l Locality) isEmpty() bool {
- return l.equal(Locality{})
-}
-
-// equal returns true if l and other are considered equal.
-func (l Locality) equal(other Locality) bool {
- return l.Region == other.Region && l.Zone == other.Zone && l.SubZone == other.SubZone
-}
diff --git a/xds/internal/clients/internal/clientsutils/clientsutils.go b/xds/internal/clients/internal/clientsutils/clientsutils.go
new file mode 100644
index 000000000000..075e8af9abf2
--- /dev/null
+++ b/xds/internal/clients/internal/clientsutils/clientsutils.go
@@ -0,0 +1,107 @@
+/*
+ *
+ * Copyright 2025 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// Package clientsutils contains helpers for xDS and LRS clients.
+package clientsutils
+
+import (
+ "fmt"
+ "slices"
+ "strings"
+
+ "google.golang.org/grpc/xds/internal/clients"
+ "google.golang.org/protobuf/proto"
+ "google.golang.org/protobuf/types/known/structpb"
+
+ v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+)
+
+// ServerIdentifierString returns a string representation of the
+// clients.ServerIdentifier si.
+//
+// WARNING: This method is primarily intended for logging and testing
+// purposes. The output returned by this method is not guaranteed to be stable
+// and may change at any time. Do not rely on it for production use.
+func ServerIdentifierString(si clients.ServerIdentifier) string {
+ return strings.Join([]string{si.ServerURI, fmt.Sprintf("%v", si.Extensions)}, "-")
+}
+
+// IsServerIdentifierEqual returns true if clients.ServerIdentifier si1 and si2
+// are considered equal.
+func IsServerIdentifierEqual(si1, si2 *clients.ServerIdentifier) bool {
+ switch {
+ case si1 == nil && si2 == nil:
+ return true
+ case (si1 != nil) != (si2 != nil):
+ return false
+ case si1.ServerURI != si2.ServerURI:
+ return false
+ }
+ if si1.Extensions == nil && si2.Extensions == nil {
+ return true
+ }
+ if ex, ok := si1.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(si2.Extensions) {
+ return true
+ }
+ return false
+}
+
+// NodeProto returns a protobuf representation of clients.Node n
+// and client features cf.
+//
+// This function is intended to be used by the client implementation to convert
+// the user-provided Node configuration to its protobuf representation.
+func NodeProto(n clients.Node, cf []string) *v3corepb.Node {
+ return &v3corepb.Node{
+ Id: n.ID,
+ Cluster: n.Cluster,
+ Locality: func() *v3corepb.Locality {
+ if IsLocalityEmpty(n.Locality) {
+ return nil
+ }
+ return &v3corepb.Locality{
+ Region: n.Locality.Region,
+ Zone: n.Locality.Zone,
+ SubZone: n.Locality.SubZone,
+ }
+ }(),
+ Metadata: func() *structpb.Struct {
+ if n.Metadata == nil {
+ return nil
+ }
+ if md, ok := n.Metadata.(*structpb.Struct); ok {
+ return proto.Clone(md).(*structpb.Struct)
+ }
+ return nil
+ }(),
+ UserAgentName: n.UserAgentName,
+ UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
+ ClientFeatures: slices.Clone(cf),
+ }
+}
+
+// IsLocalityEmpty reports whether clients.Locality l is considered empty.
+func IsLocalityEmpty(l clients.Locality) bool {
+ return IsLocalityEqual(l, clients.Locality{})
+}
+
+// IsLocalityEqual returns true if clients.Locality l1 and l2 are considered
+// equal.
+func IsLocalityEqual(l1, l2 clients.Locality) bool {
+ return l1.Region == l2.Region && l1.Zone == l2.Zone && l1.SubZone == l2.SubZone
+}
diff --git a/xds/internal/clients/config_test.go b/xds/internal/clients/internal/clientsutils/clientsutils_test.go
similarity index 67%
rename from xds/internal/clients/config_test.go
rename to xds/internal/clients/internal/clientsutils/clientsutils_test.go
index 5ca80375d99e..88aa8499c256 100644
--- a/xds/internal/clients/config_test.go
+++ b/xds/internal/clients/internal/clientsutils/clientsutils_test.go
@@ -16,16 +16,20 @@
*
*/
-package clients
+package clientsutils
import (
"testing"
- v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
"github.com/google/go-cmp/cmp"
+
"google.golang.org/grpc/internal/grpctest"
+ "google.golang.org/grpc/xds/internal/clients"
+
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/structpb"
+
+ v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
)
type s struct {
@@ -56,11 +60,11 @@ func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct
return ret
}
-func (s) TestServerIdentifier_Equal(t *testing.T) {
+func (s) TestIsServerIdentifierEqual(t *testing.T) {
tests := []struct {
name string
- s1 *ServerIdentifier
- s2 *ServerIdentifier
+ s1 *clients.ServerIdentifier
+ s2 *clients.ServerIdentifier
wantEq bool
}{
{
@@ -72,94 +76,94 @@ func (s) TestServerIdentifier_Equal(t *testing.T) {
{
name: "one_nil",
s1: nil,
- s2: &ServerIdentifier{},
+ s2: &clients.ServerIdentifier{},
wantEq: false,
},
{
name: "other_nil",
- s1: &ServerIdentifier{},
+ s1: &clients.ServerIdentifier{},
s2: nil,
wantEq: false,
},
{
name: "both_empty_and_equal",
- s1: &ServerIdentifier{},
- s2: &ServerIdentifier{},
+ s1: &clients.ServerIdentifier{},
+ s2: &clients.ServerIdentifier{},
wantEq: true,
},
{
name: "different_ServerURI",
- s1: &ServerIdentifier{ServerURI: "foo"},
- s2: &ServerIdentifier{ServerURI: "bar"},
+ s1: &clients.ServerIdentifier{ServerURI: "foo"},
+ s2: &clients.ServerIdentifier{ServerURI: "bar"},
wantEq: false,
},
{
name: "different_Extensions_with_no_Equal_method",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: 1,
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: 2,
},
wantEq: false, // By default, if there's no Equal method, they are unequal
},
{
name: "same_Extensions_with_no_Equal_method",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: 1,
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: 1,
},
wantEq: false, // By default, if there's no Equal method, they are unequal
},
{
name: "different_Extensions_with_Equal_method",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{2},
},
wantEq: false,
},
{
name: "same_Extensions_same_with_Equal_method",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{1},
},
wantEq: true,
},
{
name: "first_config_Extensions_is_nil",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: nil,
},
wantEq: false,
},
{
name: "other_config_Extensions_is_nil",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
Extensions: nil,
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
Extensions: testServerIdentifierExtension{2},
},
wantEq: false,
},
{
name: "all_fields_same",
- s1: &ServerIdentifier{
+ s1: &clients.ServerIdentifier{
ServerURI: "foo",
Extensions: testServerIdentifierExtension{1},
},
- s2: &ServerIdentifier{
+ s2: &clients.ServerIdentifier{
ServerURI: "foo",
Extensions: testServerIdentifierExtension{1},
},
@@ -169,121 +173,122 @@ func (s) TestServerIdentifier_Equal(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if gotEq := test.s1.equal(test.s2); gotEq != test.wantEq {
+ if gotEq := IsServerIdentifierEqual(test.s1, test.s2); gotEq != test.wantEq {
t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
}
})
}
}
-func (s) TestLocality_IsEmpty(t *testing.T) {
+func (s) TestIsLocalityEmpty(t *testing.T) {
tests := []struct {
name string
- locality Locality
+ locality clients.Locality
want bool
}{
{
name: "empty_locality",
- locality: Locality{},
+ locality: clients.Locality{},
want: true,
},
{
name: "non_empty_region",
- locality: Locality{Region: "region"},
+ locality: clients.Locality{Region: "region"},
want: false,
},
{
name: "non_empty_zone",
- locality: Locality{Zone: "zone"},
+ locality: clients.Locality{Zone: "zone"},
want: false,
},
{
name: "non_empty_subzone",
- locality: Locality{SubZone: "subzone"},
+ locality: clients.Locality{SubZone: "subzone"},
want: false,
},
{
name: "non_empty_all_fields",
- locality: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ locality: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
want: false,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if got := test.locality.isEmpty(); got != test.want {
+ if got := IsLocalityEmpty(test.locality); got != test.want {
t.Errorf("IsEmpty() = %v, want %v", got, test.want)
}
})
}
}
-func (s) TestLocality_Equal(t *testing.T) {
+func (s) TestIsLocalityEqual(t *testing.T) {
tests := []struct {
name string
- l1 Locality
- l2 Locality
+ l1 clients.Locality
+ l2 clients.Locality
wantEq bool
}{
{
name: "both_equal",
- l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ l1: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ l2: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
wantEq: true,
},
{
name: "different_regions",
- l1: Locality{Region: "region1", Zone: "zone", SubZone: "subzone"},
- l2: Locality{Region: "region2", Zone: "zone", SubZone: "subzone"},
+ l1: clients.Locality{Region: "region1", Zone: "zone", SubZone: "subzone"},
+ l2: clients.Locality{Region: "region2", Zone: "zone", SubZone: "subzone"},
wantEq: false,
},
{
name: "different_zones",
- l1: Locality{Region: "region", Zone: "zone1", SubZone: "subzone"},
- l2: Locality{Region: "region", Zone: "zone2", SubZone: "subzone"},
+ l1: clients.Locality{Region: "region", Zone: "zone1", SubZone: "subzone"},
+ l2: clients.Locality{Region: "region", Zone: "zone2", SubZone: "subzone"},
wantEq: false,
},
{
name: "different_subzones",
- l1: Locality{Region: "region", Zone: "zone", SubZone: "subzone1"},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone2"},
+ l1: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone1"},
+ l2: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone2"},
wantEq: false,
},
{
name: "one_empty",
- l1: Locality{},
- l2: Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
+ l1: clients.Locality{},
+ l2: clients.Locality{Region: "region", Zone: "zone", SubZone: "subzone"},
wantEq: false,
},
{
name: "both_empty",
- l1: Locality{},
- l2: Locality{},
+ l1: clients.Locality{},
+ l2: clients.Locality{},
wantEq: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if gotEq := test.l1.equal(test.l2); gotEq != test.wantEq {
+ if gotEq := IsLocalityEqual(test.l1, test.l2); gotEq != test.wantEq {
t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
}
})
}
}
-func (s) TestNode_ToProto(t *testing.T) {
+func (s) TestNodeProto(t *testing.T) {
tests := []struct {
- desc string
- inputNode Node
- wantProto *v3corepb.Node
+ desc string
+ inputNode clients.Node
+ inputClientFeatures []string
+ wantProto *v3corepb.Node
}{
{
desc: "all_fields_set",
- inputNode: Node{
+ inputNode: clients.Node{
ID: "id",
Cluster: "cluster",
- Locality: Locality{
+ Locality: clients.Locality{
Region: "region",
Zone: "zone",
SubZone: "sub_zone",
@@ -291,8 +296,8 @@ func (s) TestNode_ToProto(t *testing.T) {
Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
UserAgentName: "user agent",
UserAgentVersion: "version",
- clientFeatures: []string{"feature1", "feature2"},
},
+ inputClientFeatures: []string{"feature1", "feature2"},
wantProto: &v3corepb.Node{
Id: "id",
Cluster: "cluster",
@@ -309,7 +314,7 @@ func (s) TestNode_ToProto(t *testing.T) {
},
{
desc: "some_fields_unset",
- inputNode: Node{
+ inputNode: clients.Node{
ID: "id",
},
wantProto: &v3corepb.Node{
@@ -321,9 +326,9 @@ func (s) TestNode_ToProto(t *testing.T) {
},
{
desc: "empty_locality",
- inputNode: Node{
+ inputNode: clients.Node{
ID: "id",
- Locality: Locality{},
+ Locality: clients.Locality{},
},
wantProto: &v3corepb.Node{
Id: "id",
@@ -336,7 +341,7 @@ func (s) TestNode_ToProto(t *testing.T) {
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
- gotProto := test.inputNode.toProto()
+ gotProto := NodeProto(test.inputNode, test.inputClientFeatures)
if diff := cmp.Diff(test.wantProto, gotProto, protocmp.Transform()); diff != "" {
t.Fatalf("Unexpected diff in node proto: (-want, +got):\n%s", diff)
}
From 833a19b40f925f0687d3264614a742cbdef579a1 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Mon, 3 Mar 2025 11:57:06 +0530
Subject: [PATCH 24/26] remove ReportLoadTimeout from lrs config
---
xds/internal/clients/lrsclient/load_store.go | 12 +++++++-----
xds/internal/clients/lrsclient/lrsconfig.go | 6 ------
xds/internal/clients/xdsclient/resource_type.go | 6 +++---
3 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index f973880dc606..126973b10293 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -20,6 +20,8 @@
package lrsclient
+import "context"
+
// A LoadStore aggregates loads for multiple clusters and services that are
// intended to be reported via LRS.
//
@@ -37,11 +39,11 @@ type LoadStore struct {
// the reference count to the stream is decremented, and the stream remains
// open until all LoadStores have called Stop().
//
-// If the stream is being closed, this method attempts to flush any remaining
-// load data to the LRS server within the specified ReportLoadTimeout in the
-// client's Config. If the data cannot be written within the timeout, the
-// stream is canceled without flushing the data.
-func (ls *LoadStore) Stop() error {
+// If the stream is being closed, this method makes a last attempt to flush any
+// remaining load data to the LRS server. It will either wait for this attempt
+// to succeed, or for the provided context to be done before canceling the LRS
+// stream.
+func (ls *LoadStore) Stop(ctx context.Context) error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/lrsclient/lrsconfig.go b/xds/internal/clients/lrsclient/lrsconfig.go
index 75f014f13f66..c4862ff76794 100644
--- a/xds/internal/clients/lrsclient/lrsconfig.go
+++ b/xds/internal/clients/lrsclient/lrsconfig.go
@@ -19,8 +19,6 @@
package lrsclient
import (
- "time"
-
"google.golang.org/grpc/xds/internal/clients"
)
@@ -34,8 +32,4 @@ type Config struct {
// TransportBuilder is used to connect to the LRS server.
TransportBuilder clients.TransportBuilder
-
- // ReportLoadTimeout is the timeout used when reporting load to the LRS
- // server.
- ReportLoadTimeout time.Duration
}
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index fe28d85c5397..c73631ace31e 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -83,9 +83,9 @@ type DecodeResult struct {
// provide an implementation of this interface to represent the configuration
// received from the xDS management server.
type ResourceData interface {
- // RawEqual returns true if the passed in resource data is equal to that of
- // the receiver, based on the underlying raw message.
- RawEqual(ResourceData) bool
+ // Equal returns true if the passed in resource data is equal to that of
+ // the receiver.
+ Equal(ResourceData) bool
// Raw returns the underlying raw form of the resource.
Raw() []byte
From 60166b7ccb6e9d60cf297740d530e88efd81dc9f Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Tue, 4 Mar 2025 17:15:09 +0530
Subject: [PATCH 25/26] doug final nits
---
.../clientsutils.go => internal.go} | 23 ++++++++-----------
.../clientsutils_test.go => internal_test.go} | 19 ++++++---------
xds/internal/clients/lrsclient/load_store.go | 8 +++----
.../clients/xdsclient/resource_type.go | 6 ++---
4 files changed, 24 insertions(+), 32 deletions(-)
rename xds/internal/clients/internal/{clientsutils/clientsutils.go => internal.go} (83%)
rename xds/internal/clients/internal/{clientsutils/clientsutils_test.go => internal_test.go} (93%)
diff --git a/xds/internal/clients/internal/clientsutils/clientsutils.go b/xds/internal/clients/internal/internal.go
similarity index 83%
rename from xds/internal/clients/internal/clientsutils/clientsutils.go
rename to xds/internal/clients/internal/internal.go
index 075e8af9abf2..73c3700ca9f0 100644
--- a/xds/internal/clients/internal/clientsutils/clientsutils.go
+++ b/xds/internal/clients/internal/internal.go
@@ -16,12 +16,11 @@
*
*/
-// Package clientsutils contains helpers for xDS and LRS clients.
-package clientsutils
+// Package internal contains helpers for xDS and LRS clients.
+package internal
import (
"fmt"
- "slices"
"strings"
"google.golang.org/grpc/xds/internal/clients"
@@ -61,17 +60,16 @@ func IsServerIdentifierEqual(si1, si2 *clients.ServerIdentifier) bool {
return false
}
-// NodeProto returns a protobuf representation of clients.Node n
-// and client features cf.
+// NodeProto returns a protobuf representation of clients.Node n.
//
// This function is intended to be used by the client implementation to convert
// the user-provided Node configuration to its protobuf representation.
-func NodeProto(n clients.Node, cf []string) *v3corepb.Node {
+func NodeProto(n clients.Node) *v3corepb.Node {
return &v3corepb.Node{
Id: n.ID,
Cluster: n.Cluster,
Locality: func() *v3corepb.Locality {
- if IsLocalityEmpty(n.Locality) {
+ if isLocalityEmpty(n.Locality) {
return nil
}
return &v3corepb.Locality{
@@ -91,17 +89,16 @@ func NodeProto(n clients.Node, cf []string) *v3corepb.Node {
}(),
UserAgentName: n.UserAgentName,
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: n.UserAgentVersion},
- ClientFeatures: slices.Clone(cf),
}
}
-// IsLocalityEmpty reports whether clients.Locality l is considered empty.
-func IsLocalityEmpty(l clients.Locality) bool {
- return IsLocalityEqual(l, clients.Locality{})
+// isLocalityEqual reports whether clients.Locality l is considered empty.
+func isLocalityEmpty(l clients.Locality) bool {
+ return isLocalityEqual(l, clients.Locality{})
}
-// IsLocalityEqual returns true if clients.Locality l1 and l2 are considered
+// isLocalityEqual returns true if clients.Locality l1 and l2 are considered
// equal.
-func IsLocalityEqual(l1, l2 clients.Locality) bool {
+func isLocalityEqual(l1, l2 clients.Locality) bool {
return l1.Region == l2.Region && l1.Zone == l2.Zone && l1.SubZone == l2.SubZone
}
diff --git a/xds/internal/clients/internal/clientsutils/clientsutils_test.go b/xds/internal/clients/internal/internal_test.go
similarity index 93%
rename from xds/internal/clients/internal/clientsutils/clientsutils_test.go
rename to xds/internal/clients/internal/internal_test.go
index 88aa8499c256..6fd1cbd169b6 100644
--- a/xds/internal/clients/internal/clientsutils/clientsutils_test.go
+++ b/xds/internal/clients/internal/internal_test.go
@@ -16,7 +16,7 @@
*
*/
-package clientsutils
+package internal
import (
"testing"
@@ -214,7 +214,7 @@ func (s) TestIsLocalityEmpty(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if got := IsLocalityEmpty(test.locality); got != test.want {
+ if got := isLocalityEmpty(test.locality); got != test.want {
t.Errorf("IsEmpty() = %v, want %v", got, test.want)
}
})
@@ -269,7 +269,7 @@ func (s) TestIsLocalityEqual(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
- if gotEq := IsLocalityEqual(test.l1, test.l2); gotEq != test.wantEq {
+ if gotEq := isLocalityEqual(test.l1, test.l2); gotEq != test.wantEq {
t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
}
})
@@ -278,10 +278,9 @@ func (s) TestIsLocalityEqual(t *testing.T) {
func (s) TestNodeProto(t *testing.T) {
tests := []struct {
- desc string
- inputNode clients.Node
- inputClientFeatures []string
- wantProto *v3corepb.Node
+ desc string
+ inputNode clients.Node
+ wantProto *v3corepb.Node
}{
{
desc: "all_fields_set",
@@ -297,7 +296,6 @@ func (s) TestNodeProto(t *testing.T) {
UserAgentName: "user agent",
UserAgentVersion: "version",
},
- inputClientFeatures: []string{"feature1", "feature2"},
wantProto: &v3corepb.Node{
Id: "id",
Cluster: "cluster",
@@ -309,7 +307,6 @@ func (s) TestNodeProto(t *testing.T) {
Metadata: newStructProtoFromMap(t, map[string]any{"k1": "v1", "k2": 101, "k3": 280.0}),
UserAgentName: "user agent",
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: "version"},
- ClientFeatures: []string{"feature1", "feature2"},
},
},
{
@@ -321,7 +318,6 @@ func (s) TestNodeProto(t *testing.T) {
Id: "id",
UserAgentName: "",
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
- ClientFeatures: nil,
},
},
{
@@ -334,14 +330,13 @@ func (s) TestNodeProto(t *testing.T) {
Id: "id",
UserAgentName: "",
UserAgentVersionType: &v3corepb.Node_UserAgentVersion{UserAgentVersion: ""},
- ClientFeatures: nil,
},
},
}
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
- gotProto := NodeProto(test.inputNode, test.inputClientFeatures)
+ gotProto := NodeProto(test.inputNode)
if diff := cmp.Diff(test.wantProto, gotProto, protocmp.Transform()); diff != "" {
t.Fatalf("Unexpected diff in node proto: (-want, +got):\n%s", diff)
}
diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go
index 126973b10293..d52db0c78330 100644
--- a/xds/internal/clients/lrsclient/load_store.go
+++ b/xds/internal/clients/lrsclient/load_store.go
@@ -39,10 +39,10 @@ type LoadStore struct {
// the reference count to the stream is decremented, and the stream remains
// open until all LoadStores have called Stop().
//
-// If the stream is being closed, this method makes a last attempt to flush any
-// remaining load data to the LRS server. It will either wait for this attempt
-// to succeed, or for the provided context to be done before canceling the LRS
-// stream.
+// If this is the last LoadStore for the stream, this method makes a last
+// attempt to flush any unreported load data to the LRS server. It will either
+// wait for this attempt to complete, or for the provided context to be done
+// before canceling the LRS stream.
func (ls *LoadStore) Stop(ctx context.Context) error {
panic("unimplemented")
}
diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go
index c73631ace31e..8ca466ed716e 100644
--- a/xds/internal/clients/xdsclient/resource_type.go
+++ b/xds/internal/clients/xdsclient/resource_type.go
@@ -85,8 +85,8 @@ type DecodeResult struct {
type ResourceData interface {
// Equal returns true if the passed in resource data is equal to that of
// the receiver.
- Equal(ResourceData) bool
+ Equal(other ResourceData) bool
- // Raw returns the underlying raw form of the resource.
- Raw() []byte
+ // Bytes returns the underlying raw bytes of the resource proto.
+ Bytes() []byte
}
From b6ac539bba5171369208364bcb6bef61139de441 Mon Sep 17 00:00:00 2001
From: Purnesh Dixit <purneshdixit@google.com>
Date: Wed, 5 Mar 2025 23:20:27 +0530
Subject: [PATCH 26/26] remove ServerIdentifierEqual
---
xds/internal/clients/config.go | 3 -
xds/internal/clients/internal/internal.go | 20 ---
.../clients/internal/internal_test.go | 130 ------------------
3 files changed, 153 deletions(-)
diff --git a/xds/internal/clients/config.go b/xds/internal/clients/config.go
index 568e160ce3f4..a2220f995737 100644
--- a/xds/internal/clients/config.go
+++ b/xds/internal/clients/config.go
@@ -59,9 +59,6 @@ type ServerIdentifier struct {
//
// For example, a custom TransportBuilder might use this field to
// configure a specific security credentials.
- //
- // Note: For custom types used in Extensions, ensure an Equal(any) bool
- // method is implemented for equality checks on ServerIdentifier.
Extensions any
}
diff --git a/xds/internal/clients/internal/internal.go b/xds/internal/clients/internal/internal.go
index 73c3700ca9f0..d712129843ba 100644
--- a/xds/internal/clients/internal/internal.go
+++ b/xds/internal/clients/internal/internal.go
@@ -40,26 +40,6 @@ func ServerIdentifierString(si clients.ServerIdentifier) string {
return strings.Join([]string{si.ServerURI, fmt.Sprintf("%v", si.Extensions)}, "-")
}
-// IsServerIdentifierEqual returns true if clients.ServerIdentifier si1 and si2
-// are considered equal.
-func IsServerIdentifierEqual(si1, si2 *clients.ServerIdentifier) bool {
- switch {
- case si1 == nil && si2 == nil:
- return true
- case (si1 != nil) != (si2 != nil):
- return false
- case si1.ServerURI != si2.ServerURI:
- return false
- }
- if si1.Extensions == nil && si2.Extensions == nil {
- return true
- }
- if ex, ok := si1.Extensions.(interface{ Equal(any) bool }); ok && ex.Equal(si2.Extensions) {
- return true
- }
- return false
-}
-
// NodeProto returns a protobuf representation of clients.Node n.
//
// This function is intended to be used by the client implementation to convert
diff --git a/xds/internal/clients/internal/internal_test.go b/xds/internal/clients/internal/internal_test.go
index 6fd1cbd169b6..8815b925114b 100644
--- a/xds/internal/clients/internal/internal_test.go
+++ b/xds/internal/clients/internal/internal_test.go
@@ -40,16 +40,6 @@ func Test(t *testing.T) {
grpctest.RunSubTests(t, s{})
}
-type testServerIdentifierExtension struct{ x int }
-
-func (ts testServerIdentifierExtension) Equal(other any) bool {
- ots, ok := other.(testServerIdentifierExtension)
- if !ok {
- return false
- }
- return ts.x == ots.x
-}
-
func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct {
t.Helper()
@@ -60,126 +50,6 @@ func newStructProtoFromMap(t *testing.T, input map[string]any) *structpb.Struct
return ret
}
-func (s) TestIsServerIdentifierEqual(t *testing.T) {
- tests := []struct {
- name string
- s1 *clients.ServerIdentifier
- s2 *clients.ServerIdentifier
- wantEq bool
- }{
- {
- name: "both_nil",
- s1: nil,
- s2: nil,
- wantEq: true,
- },
- {
- name: "one_nil",
- s1: nil,
- s2: &clients.ServerIdentifier{},
- wantEq: false,
- },
- {
- name: "other_nil",
- s1: &clients.ServerIdentifier{},
- s2: nil,
- wantEq: false,
- },
- {
- name: "both_empty_and_equal",
- s1: &clients.ServerIdentifier{},
- s2: &clients.ServerIdentifier{},
- wantEq: true,
- },
- {
- name: "different_ServerURI",
- s1: &clients.ServerIdentifier{ServerURI: "foo"},
- s2: &clients.ServerIdentifier{ServerURI: "bar"},
- wantEq: false,
- },
- {
- name: "different_Extensions_with_no_Equal_method",
- s1: &clients.ServerIdentifier{
- Extensions: 1,
- },
- s2: &clients.ServerIdentifier{
- Extensions: 2,
- },
- wantEq: false, // By default, if there's no Equal method, they are unequal
- },
- {
- name: "same_Extensions_with_no_Equal_method",
- s1: &clients.ServerIdentifier{
- Extensions: 1,
- },
- s2: &clients.ServerIdentifier{
- Extensions: 1,
- },
- wantEq: false, // By default, if there's no Equal method, they are unequal
- },
- {
- name: "different_Extensions_with_Equal_method",
- s1: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{1},
- },
- s2: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{2},
- },
- wantEq: false,
- },
- {
- name: "same_Extensions_same_with_Equal_method",
- s1: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{1},
- },
- s2: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{1},
- },
- wantEq: true,
- },
- {
- name: "first_config_Extensions_is_nil",
- s1: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{1},
- },
- s2: &clients.ServerIdentifier{
- Extensions: nil,
- },
- wantEq: false,
- },
- {
- name: "other_config_Extensions_is_nil",
- s1: &clients.ServerIdentifier{
- Extensions: nil,
- },
- s2: &clients.ServerIdentifier{
- Extensions: testServerIdentifierExtension{2},
- },
- wantEq: false,
- },
- {
- name: "all_fields_same",
- s1: &clients.ServerIdentifier{
- ServerURI: "foo",
- Extensions: testServerIdentifierExtension{1},
- },
- s2: &clients.ServerIdentifier{
- ServerURI: "foo",
- Extensions: testServerIdentifierExtension{1},
- },
- wantEq: true,
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- if gotEq := IsServerIdentifierEqual(test.s1, test.s2); gotEq != test.wantEq {
- t.Errorf("Equal() = %v, want %v", gotEq, test.wantEq)
- }
- })
- }
-}
-
func (s) TestIsLocalityEmpty(t *testing.T) {
tests := []struct {
name string