From 40959865c08bcb181244fa2f4d6deed1588c7b24 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Mon, 8 Jul 2024 19:51:42 +0530 Subject: [PATCH 01/18] Add dualstack support to the framework behind flag --- framework/infrastructure/gcp/api.py | 2 + framework/infrastructure/gcp/compute.py | 7 ++ framework/infrastructure/traffic_director.py | 85 ++++++++++++++++--- .../test_app/runners/k8s/k8s_base_runner.py | 2 +- framework/xds_flags.py | 8 ++ kubernetes-manifests/client.deployment.yaml | 6 ++ kubernetes-manifests/server.deployment.yaml | 3 + kubernetes-manifests/server.service.yaml | 6 ++ 8 files changed, 104 insertions(+), 15 deletions(-) diff --git a/framework/infrastructure/gcp/api.py b/framework/infrastructure/gcp/api.py index 746950a6..df4ad8ba 100644 --- a/framework/infrastructure/gcp/api.py +++ b/framework/infrastructure/gcp/api.py @@ -157,6 +157,8 @@ def compute(self, version: str): return self._build_from_discovery_v1(api_name, version) elif version == "v1alpha": return self._build_from_discovery_v1(api_name, "alpha") + elif version == "v1beta" or version == "v1beta1": + return self._build_from_discovery_v1(api_name, "beta") raise NotImplementedError(f"Compute {version} not supported") diff --git a/framework/infrastructure/gcp/compute.py b/framework/infrastructure/gcp/compute.py index f90d104a..1e223f42 100644 --- a/framework/infrastructure/gcp/compute.py +++ b/framework/infrastructure/gcp/compute.py @@ -39,6 +39,7 @@ class ComputeV1( _WAIT_FOR_BACKEND_SLEEP_SEC = 4 _WAIT_FOR_OPERATION_SEC = 60 * 10 gfe_debug_header: Optional[str] + add_dualstack_support: Optional[bool] @dataclasses.dataclass(frozen=True) class GcpResource: @@ -61,10 +62,12 @@ def __init__( api_manager: gcp.api.GcpApiManager, project: str, gfe_debug_header: Optional[str] = None, + add_dualstack_support: Optional[bool] = False, version: str = "v1", ): super().__init__(api_manager.compute(version), project) self.gfe_debug_header = gfe_debug_header + self.add_dualstack_support = add_dualstack_support class HealthCheckProtocol(enum.Enum): TCP = enum.auto() @@ -160,6 +163,10 @@ def create_backend_service_traffic_director( "healthChecks": [health_check.url], "protocol": protocol.name, } + # If add dualstack support is specified True, config the backend service + # to support IPv6 + if self.add_dualstack_support: + body["ipAddressSelectionPolicy"] = "PREFER_IPV6" # If affinity header is specified, config the backend service to support # affinity, and set affinity header to the one given. if affinity_header: diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index decd4102..247ec630 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -114,6 +114,7 @@ def __init__( project, version=compute_api_version, gfe_debug_header=xds_flags.GFE_DEBUG_HEADER.value, + add_dualstack_support=xds_flags.ADD_DUALSTACK_SUPPORT.value ) # Settings @@ -129,10 +130,12 @@ def __init__( self.firewall_rule: Optional[GcpResource] = None self.firewall_rule_ipv6: Optional[GcpResource] = None self.target_proxy: Optional[GcpResource] = None + self.target_proxy_v6: Optional[GcpResource] = None # TODO(sergiitk): remove this flag once target proxy resource loaded self.target_proxy_is_http: bool = False self.alternative_target_proxy: Optional[GcpResource] = None self.forwarding_rule: Optional[GcpResource] = None + self.forwarding_rule_v6: Optional[GcpResource] = None self.alternative_forwarding_rule: Optional[GcpResource] = None # Backends. @@ -171,6 +174,10 @@ def setup_routing_rule_map_for_grpc(self, service_host, service_port): self.create_target_proxy() self.create_forwarding_rule(service_port) + if self.add_dualstack_support: + self.create_target_proxy(ipv6=True) + self.create_forwarding_rule(service_port, ipv6=True) + def cleanup(self, *, force=False): # Cleanup in the reverse order of creation self.delete_firewall_rules(force=force) @@ -178,6 +185,10 @@ def cleanup(self, *, force=False): self.delete_alternative_forwarding_rule(force=force) self.delete_target_http_proxy(force=force) self.delete_target_grpc_proxy(force=force) + self.delete_forwarding_rule(force=force) + if self.add_dualstack_support: + self.delete_target_http_proxy(force=force, ipv6=True) + self.delete_forwarding_rule(force=force, ipv6=True) self.delete_alternative_target_grpc_proxy(force=force) self.delete_url_map(force=force) self.delete_alternative_url_map(force=force) @@ -593,8 +604,12 @@ def delete_alternative_url_map(self, force=False): self.compute.delete_url_map(name) self.url_map = None - def create_target_proxy(self): - name = self.make_resource_name(self.TARGET_PROXY_NAME) + def create_target_proxy(self, ipv6=False): + if ipv6: + name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") + else: + name = self.make_resource_name(self.TARGET_PROXY_NAME) + if self.backend_service_protocol is BackendServiceProtocol.GRPC: target_proxy_type = "GRPC" create_proxy_fn = self.compute.create_target_grpc_proxy @@ -612,30 +627,54 @@ def create_target_proxy(self): target_proxy_type, self.url_map.name, ) - self.target_proxy = create_proxy_fn(name, self.url_map) + + if ipv6: + self.target_proxy_v6 = create_proxy_fn(name, self.url_map) + else: + self.target_proxy = create_proxy_fn(name, self.url_map) def delete_target_grpc_proxy(self, force=False): if force: - name = self.make_resource_name(self.TARGET_PROXY_NAME) + if ipv6: + name = self.make_resource_name(self.TARGET_PROXY_NAME) + else: + name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") elif self.target_proxy: name = self.target_proxy.name + elif self.target_proxy_v6: + name = self.target_proxy_v6.name else: return + logger.info('Deleting Target GRPC proxy "%s"', name) self.compute.delete_target_grpc_proxy(name) - self.target_proxy = None + if ipv6: + self.target_proxy_v6 = None + else: + self.target_proxy = None + self.target_proxy_is_http = False - def delete_target_http_proxy(self, force=False): + def delete_target_http_proxy(self, force=False, ipv6=False): if force: - name = self.make_resource_name(self.TARGET_PROXY_NAME) + if ipv6: + name = self.make_resource_name(self.TARGET_PROXY_NAME) + else: + name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") elif self.target_proxy and self.target_proxy_is_http: name = self.target_proxy.name + elif self.target_proxy_v6 and self.target_proxy_is_http: + name = self.target_proxy_v6.name else: return + logger.info('Deleting HTTP Target proxy "%s"', name) self.compute.delete_target_http_proxy(name) - self.target_proxy = None + if ipv6: + self.target_proxy_v6 = None + else: + self.target_proxy = None + self.target_proxy_is_http = False def create_alternative_target_proxy(self): @@ -679,8 +718,12 @@ def find_unused_forwarding_rule_port( # TODO(sergiitk): custom exception raise RuntimeError("Couldn't find unused forwarding rule port") - def create_forwarding_rule(self, src_port: int): - name = self.make_resource_name(self.FORWARDING_RULE_NAME) + def create_forwarding_rule(self, src_port: int, ipv6=False): + if ipPv6: + name = self.make_resource_name(self.FORWARDING_RULE_NAME + "-v6") + else: + name = self.make_resource_name(self.FORWARDING_RULE_NAME) + src_port = int(src_port) logging.info( 'Creating forwarding rule "%s" in network "%s": 0.0.0.0:%s -> %s', @@ -692,19 +735,33 @@ def create_forwarding_rule(self, src_port: int): resource = self.compute.create_forwarding_rule( name, src_port, self.target_proxy, self.network_url ) - self.forwarding_rule = resource + + if ipv6: + self.forwarding_rule_v6 = resource + else: + self.forwarding_rule = resource + return resource - def delete_forwarding_rule(self, force=False): + def delete_forwarding_rule(self, force=False, ipv6=False): if force: - name = self.make_resource_name(self.FORWARDING_RULE_NAME) + if ipv6: + name = self.make_resource_name(self.FORWARDING_RULE_NAME) + else: + name = self.make_resource_name(self.FORWARDING_RULE_NAME + "-v6") elif self.forwarding_rule: name = self.forwarding_rule.name + elif self.forwarding_rule_v6: + name = self.forwarding_rule_v6.name else: return + logger.info('Deleting Forwarding rule "%s"', name) self.compute.delete_forwarding_rule(name) - self.forwarding_rule = None + if ipv6: + self.forwarding_rule_v6 = None + else: + self.forwarding_rule = None def create_alternative_forwarding_rule( self, src_port: int, ip_address="0.0.0.0" diff --git a/framework/test_app/runners/k8s/k8s_base_runner.py b/framework/test_app/runners/k8s/k8s_base_runner.py index 867f50c7..65652b85 100644 --- a/framework/test_app/runners/k8s/k8s_base_runner.py +++ b/framework/test_app/runners/k8s/k8s_base_runner.py @@ -999,7 +999,7 @@ def _wait_pod_started(self, name, **kwargs) -> k8s.V1Pod: self.k8s_namespace.wait_for_pod_started(name, **kwargs) pod = self.k8s_namespace.get_pod(name) logger.info( - "Pod %s ready, IP: %s", pod.metadata.name, pod.status.pod_ip + "Pod %s ready, IP: %s", pod.metadata.name, pod.status.pod_ip_s ) return pod diff --git a/framework/xds_flags.py b/framework/xds_flags.py index 971f1b4c..9e496051 100644 --- a/framework/xds_flags.py +++ b/framework/xds_flags.py @@ -190,6 +190,14 @@ help="Whether to enable GFE debug headers and what value to use.", ) +ADD_DUALSTACK_SUPPORT = flags.DEFINE_bool( + "add_dualstack_support", + default=False, + help=( + f"Add support for Dual Stack resources to the framework." + ), +) + def set_socket_default_timeout_from_flag() -> None: """A helper to configure default socket timeout from a flag. diff --git a/kubernetes-manifests/client.deployment.yaml b/kubernetes-manifests/client.deployment.yaml index 897c2ed2..4e6ad5b7 100644 --- a/kubernetes-manifests/client.deployment.yaml +++ b/kubernetes-manifests/client.deployment.yaml @@ -66,6 +66,12 @@ spec: value: "true" - name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST value: "true" + % if add_dualstack_support: + - name: GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST + value: "true" + - name: GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS + value: "true" + % endif % if csm_workload_name: - name: CSM_WORKLOAD_NAME value: ${csm_workload_name} diff --git a/kubernetes-manifests/server.deployment.yaml b/kubernetes-manifests/server.deployment.yaml index 7dfeaa0e..c6948545 100644 --- a/kubernetes-manifests/server.deployment.yaml +++ b/kubernetes-manifests/server.deployment.yaml @@ -42,6 +42,9 @@ spec: ${prestop_hook.main_lifecycle() if pre_stop_hook else ''} args: - "--port=${test_port}" + % if add_dualstack_support: + - "--address_type=IPV4" + % endif % if enable_csm_observability: - "--enable_csm_observability=true" % endif diff --git a/kubernetes-manifests/server.service.yaml b/kubernetes-manifests/server.service.yaml index 376de175..ce2f6ae7 100644 --- a/kubernetes-manifests/server.service.yaml +++ b/kubernetes-manifests/server.service.yaml @@ -10,6 +10,12 @@ metadata: cloud.google.com/neg: '{"exposed_ports": {"${test_port}":{"name":"${neg_name}"}}}' spec: type: ClusterIP + % if add_dualstack_support: + ipFamilyPolicy: PreferDualStack + ipFamilies: + - IPv6 + - IPv4 + % endif selector: app: ${deployment_name} ports: From 4722d50a265d5edb36489404e59e57ff53fd34bc Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Wed, 10 Jul 2024 20:49:26 +0530 Subject: [PATCH 02/18] Resolving comments by sergii --- framework/infrastructure/gcp/api.py | 2 +- framework/infrastructure/gcp/compute.py | 4 +- framework/infrastructure/traffic_director.py | 43 +++++++++++--------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/framework/infrastructure/gcp/api.py b/framework/infrastructure/gcp/api.py index df4ad8ba..84b8446c 100644 --- a/framework/infrastructure/gcp/api.py +++ b/framework/infrastructure/gcp/api.py @@ -157,7 +157,7 @@ def compute(self, version: str): return self._build_from_discovery_v1(api_name, version) elif version == "v1alpha": return self._build_from_discovery_v1(api_name, "alpha") - elif version == "v1beta" or version == "v1beta1": + elif version in ["v1beta", "v1beta1"]: return self._build_from_discovery_v1(api_name, "beta") raise NotImplementedError(f"Compute {version} not supported") diff --git a/framework/infrastructure/gcp/compute.py b/framework/infrastructure/gcp/compute.py index 1e223f42..768fdb3c 100644 --- a/framework/infrastructure/gcp/compute.py +++ b/framework/infrastructure/gcp/compute.py @@ -39,7 +39,7 @@ class ComputeV1( _WAIT_FOR_BACKEND_SLEEP_SEC = 4 _WAIT_FOR_OPERATION_SEC = 60 * 10 gfe_debug_header: Optional[str] - add_dualstack_support: Optional[bool] + add_dualstack_support: bool = False @dataclasses.dataclass(frozen=True) class GcpResource: @@ -62,7 +62,7 @@ def __init__( api_manager: gcp.api.GcpApiManager, project: str, gfe_debug_header: Optional[str] = None, - add_dualstack_support: Optional[bool] = False, + add_dualstack_support: bool = False, version: str = "v1", ): super().__init__(api_manager.compute(version), project) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 247ec630..b7032746 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -65,9 +65,11 @@ class TrafficDirectorManager: # pylint: disable=too-many-public-methods URL_MAP_PATH_MATCHER_NAME: Final[str] = "path-matcher" TARGET_PROXY_NAME: Final[str] = "target-proxy" + TARGET_PROXY_NAME_IPV6: Final[str] = "target-proxy-ipv6" ALTERNATIVE_TARGET_PROXY_NAME: Final[str] = "target-proxy-alt" FORWARDING_RULE_NAME: Final[str] = "forwarding-rule" + FORWARDING_RULE_NAME_IPV6: Final[str] = "forwarding-rule-ipv6" ALTERNATIVE_FORWARDING_RULE_NAME: Final[str] = "forwarding-rule-alt" HEALTH_CHECK_NAME: Final[str] = "health-check" @@ -114,7 +116,7 @@ def __init__( project, version=compute_api_version, gfe_debug_header=xds_flags.GFE_DEBUG_HEADER.value, - add_dualstack_support=xds_flags.ADD_DUALSTACK_SUPPORT.value + add_dualstack_support=xds_flags.ADD_DUALSTACK_SUPPORT.value, ) # Settings @@ -137,6 +139,7 @@ def __init__( self.forwarding_rule: Optional[GcpResource] = None self.forwarding_rule_v6: Optional[GcpResource] = None self.alternative_forwarding_rule: Optional[GcpResource] = None + self.add_dualstack_support = xds_flags.ADD_DUALSTACK_SUPPORT.value # Backends. self.backends = set() @@ -604,12 +607,12 @@ def delete_alternative_url_map(self, force=False): self.compute.delete_url_map(name) self.url_map = None - def create_target_proxy(self, ipv6=False): + def create_target_proxy(self, ipv6=False): if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") + name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) else: name = self.make_resource_name(self.TARGET_PROXY_NAME) - + if self.backend_service_protocol is BackendServiceProtocol.GRPC: target_proxy_type = "GRPC" create_proxy_fn = self.compute.create_target_grpc_proxy @@ -627,32 +630,32 @@ def create_target_proxy(self, ipv6=False): target_proxy_type, self.url_map.name, ) - + if ipv6: self.target_proxy_v6 = create_proxy_fn(name, self.url_map) else: self.target_proxy = create_proxy_fn(name, self.url_map) - def delete_target_grpc_proxy(self, force=False): + def delete_target_grpc_proxy(self, force=False, ipv6=False): if force: if ipv6: name = self.make_resource_name(self.TARGET_PROXY_NAME) else: - name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") + name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) elif self.target_proxy: name = self.target_proxy.name elif self.target_proxy_v6: - name = self.target_proxy_v6.name + name = self.target_proxy_v6.name else: return - + logger.info('Deleting Target GRPC proxy "%s"', name) self.compute.delete_target_grpc_proxy(name) if ipv6: self.target_proxy_v6 = None else: self.target_proxy = None - + self.target_proxy_is_http = False def delete_target_http_proxy(self, force=False, ipv6=False): @@ -660,21 +663,21 @@ def delete_target_http_proxy(self, force=False, ipv6=False): if ipv6: name = self.make_resource_name(self.TARGET_PROXY_NAME) else: - name = self.make_resource_name(self.TARGET_PROXY_NAME + "-v6") + name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) elif self.target_proxy and self.target_proxy_is_http: name = self.target_proxy.name elif self.target_proxy_v6 and self.target_proxy_is_http: name = self.target_proxy_v6.name else: return - + logger.info('Deleting HTTP Target proxy "%s"', name) self.compute.delete_target_http_proxy(name) if ipv6: self.target_proxy_v6 = None else: self.target_proxy = None - + self.target_proxy_is_http = False def create_alternative_target_proxy(self): @@ -719,11 +722,11 @@ def find_unused_forwarding_rule_port( raise RuntimeError("Couldn't find unused forwarding rule port") def create_forwarding_rule(self, src_port: int, ipv6=False): - if ipPv6: - name = self.make_resource_name(self.FORWARDING_RULE_NAME + "-v6") + if ipv6: + name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) else: name = self.make_resource_name(self.FORWARDING_RULE_NAME) - + src_port = int(src_port) logging.info( 'Creating forwarding rule "%s" in network "%s": 0.0.0.0:%s -> %s', @@ -735,12 +738,12 @@ def create_forwarding_rule(self, src_port: int, ipv6=False): resource = self.compute.create_forwarding_rule( name, src_port, self.target_proxy, self.network_url ) - + if ipv6: self.forwarding_rule_v6 = resource else: self.forwarding_rule = resource - + return resource def delete_forwarding_rule(self, force=False, ipv6=False): @@ -748,14 +751,14 @@ def delete_forwarding_rule(self, force=False, ipv6=False): if ipv6: name = self.make_resource_name(self.FORWARDING_RULE_NAME) else: - name = self.make_resource_name(self.FORWARDING_RULE_NAME + "-v6") + name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) elif self.forwarding_rule: name = self.forwarding_rule.name elif self.forwarding_rule_v6: name = self.forwarding_rule_v6.name else: return - + logger.info('Deleting Forwarding rule "%s"', name) self.compute.delete_forwarding_rule(name) if ipv6: From 4793e802a5b2a030db01bc0923506633c48216b0 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Wed, 10 Jul 2024 22:33:08 +0530 Subject: [PATCH 03/18] Separate ip address for ip4 and ipv6 --- framework/infrastructure/traffic_director.py | 21 +++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index b7032746..7a23edd9 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -179,7 +179,9 @@ def setup_routing_rule_map_for_grpc(self, service_host, service_port): if self.add_dualstack_support: self.create_target_proxy(ipv6=True) - self.create_forwarding_rule(service_port, ipv6=True) + self.create_forwarding_rule( + service_port, ipv6=True, ip_address="::" + ) def cleanup(self, *, force=False): # Cleanup in the reverse order of creation @@ -721,22 +723,31 @@ def find_unused_forwarding_rule_port( # TODO(sergiitk): custom exception raise RuntimeError("Couldn't find unused forwarding rule port") - def create_forwarding_rule(self, src_port: int, ipv6=False): + def create_forwarding_rule( + self, src_port: int, ipv6=False, ip_address="0.0.0.0" + ): if ipv6: name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) + target_proxy = self.target_proxy_v6 else: name = self.make_resource_name(self.FORWARDING_RULE_NAME) + target_proxy = self.target_proxy src_port = int(src_port) logging.info( - 'Creating forwarding rule "%s" in network "%s": 0.0.0.0:%s -> %s', + 'Creating forwarding rule "%s" in network "%s": %s:%s -> %s', name, self.network, src_port, - self.target_proxy.url, + ip_address, + target_proxy.url, ) resource = self.compute.create_forwarding_rule( - name, src_port, self.target_proxy, self.network_url + name, + src_port, + target_proxy, + self.network_url, + ip_address=ip_address, ) if ipv6: From 36915c5d08b853cec2701ee055dc6aaea30cbf5d Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Fri, 12 Jul 2024 01:48:54 +0530 Subject: [PATCH 04/18] Fix backwards if IPv6 in traffic_director and xds_flags formatting --- framework/infrastructure/traffic_director.py | 12 ++++++------ framework/xds_flags.py | 4 +--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 7a23edd9..18f5bbf0 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -641,9 +641,9 @@ def create_target_proxy(self, ipv6=False): def delete_target_grpc_proxy(self, force=False, ipv6=False): if force: if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME) - else: name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) + else: + name = self.make_resource_name(self.TARGET_PROXY_NAME) elif self.target_proxy: name = self.target_proxy.name elif self.target_proxy_v6: @@ -663,9 +663,9 @@ def delete_target_grpc_proxy(self, force=False, ipv6=False): def delete_target_http_proxy(self, force=False, ipv6=False): if force: if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME) - else: name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) + else: + name = self.make_resource_name(self.TARGET_PROXY_NAME) elif self.target_proxy and self.target_proxy_is_http: name = self.target_proxy.name elif self.target_proxy_v6 and self.target_proxy_is_http: @@ -760,9 +760,9 @@ def create_forwarding_rule( def delete_forwarding_rule(self, force=False, ipv6=False): if force: if ipv6: - name = self.make_resource_name(self.FORWARDING_RULE_NAME) - else: name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) + else: + name = self.make_resource_name(self.FORWARDING_RULE_NAME) elif self.forwarding_rule: name = self.forwarding_rule.name elif self.forwarding_rule_v6: diff --git a/framework/xds_flags.py b/framework/xds_flags.py index 9e496051..4991fac0 100644 --- a/framework/xds_flags.py +++ b/framework/xds_flags.py @@ -193,9 +193,7 @@ ADD_DUALSTACK_SUPPORT = flags.DEFINE_bool( "add_dualstack_support", default=False, - help=( - f"Add support for Dual Stack resources to the framework." - ), + help=(f"Add support for Dual Stack resources to the framework."), ) From b12f8b3eaff8abad8bea8e36788e0148b4682929 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Fri, 12 Jul 2024 01:58:27 +0530 Subject: [PATCH 05/18] Fix delete proxy and forwarding rule if conditions --- framework/infrastructure/traffic_director.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 18f5bbf0..2bf1aff3 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -190,7 +190,6 @@ def cleanup(self, *, force=False): self.delete_alternative_forwarding_rule(force=force) self.delete_target_http_proxy(force=force) self.delete_target_grpc_proxy(force=force) - self.delete_forwarding_rule(force=force) if self.add_dualstack_support: self.delete_target_http_proxy(force=force, ipv6=True) self.delete_forwarding_rule(force=force, ipv6=True) @@ -644,10 +643,11 @@ def delete_target_grpc_proxy(self, force=False, ipv6=False): name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) else: name = self.make_resource_name(self.TARGET_PROXY_NAME) + elif ipv6: + if self.target_proxy_v6: + name = self.target_proxy_v6.name elif self.target_proxy: name = self.target_proxy.name - elif self.target_proxy_v6: - name = self.target_proxy_v6.name else: return @@ -666,10 +666,11 @@ def delete_target_http_proxy(self, force=False, ipv6=False): name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) else: name = self.make_resource_name(self.TARGET_PROXY_NAME) + elif ipv6: + if self.target_proxy_v6 and self.target_proxy_is_http: + name = self.target_proxy_v6.name elif self.target_proxy and self.target_proxy_is_http: name = self.target_proxy.name - elif self.target_proxy_v6 and self.target_proxy_is_http: - name = self.target_proxy_v6.name else: return @@ -763,10 +764,11 @@ def delete_forwarding_rule(self, force=False, ipv6=False): name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) else: name = self.make_resource_name(self.FORWARDING_RULE_NAME) + elif ipv6: + if self.forwarding_rule_v6: + name = self.forwarding_rule_v6.name elif self.forwarding_rule: name = self.forwarding_rule.name - elif self.forwarding_rule_v6: - name = self.forwarding_rule_v6.name else: return From e45fb6e5ef75f77f303cfdaf7630527dd0679505 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Fri, 12 Jul 2024 02:12:29 +0530 Subject: [PATCH 06/18] remove addressType if block in server.deployment.yaml --- kubernetes-manifests/server.deployment.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/kubernetes-manifests/server.deployment.yaml b/kubernetes-manifests/server.deployment.yaml index c6948545..7dfeaa0e 100644 --- a/kubernetes-manifests/server.deployment.yaml +++ b/kubernetes-manifests/server.deployment.yaml @@ -42,9 +42,6 @@ spec: ${prestop_hook.main_lifecycle() if pre_stop_hook else ''} args: - "--port=${test_port}" - % if add_dualstack_support: - - "--address_type=IPV4" - % endif % if enable_csm_observability: - "--enable_csm_observability=true" % endif From e8f0cef696c5a4875f91c19d281b95a44db26c58 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 16 Jul 2024 13:20:28 +0530 Subject: [PATCH 07/18] Handle pod ready case of with or without dualstack support --- framework/test_app/runners/k8s/k8s_base_runner.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/framework/test_app/runners/k8s/k8s_base_runner.py b/framework/test_app/runners/k8s/k8s_base_runner.py index 65652b85..aa37c39f 100644 --- a/framework/test_app/runners/k8s/k8s_base_runner.py +++ b/framework/test_app/runners/k8s/k8s_base_runner.py @@ -998,9 +998,12 @@ def _wait_pod_started(self, name, **kwargs) -> k8s.V1Pod: logger.info("Waiting for pod %s to start", name) self.k8s_namespace.wait_for_pod_started(name, **kwargs) pod = self.k8s_namespace.get_pod(name) - logger.info( - "Pod %s ready, IP: %s", pod.metadata.name, pod.status.pod_ip_s - ) + + if hasattr(pod.status, "pod_ip_s"): # if running with dualstack support + pod_ip_s = pod.status.pod_ip_s + else: + pod_ip_s = pod.status.pod_ip + logger.info("Pod %s ready, IP: %s", pod.metadata.name, pod_ip_s) return pod def _pod_started_logic(self, pod: k8s.V1Pod) -> bool: From 911f29dc3f12d87251e47491e4dfcd70c82cfa50 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Wed, 17 Jul 2024 23:47:51 +0530 Subject: [PATCH 08/18] change flag to enable_dualstack, refactor ipv6 setup to separate methods, address other code improvement suggestions --- framework/infrastructure/gcp/compute.py | 6 +- framework/infrastructure/traffic_director.py | 162 +++++++++--------- .../test_app/runners/k8s/k8s_base_runner.py | 6 +- framework/xds_flags.py | 6 +- kubernetes-manifests/client.deployment.yaml | 2 +- kubernetes-manifests/server.service.yaml | 2 +- 6 files changed, 89 insertions(+), 95 deletions(-) diff --git a/framework/infrastructure/gcp/compute.py b/framework/infrastructure/gcp/compute.py index 768fdb3c..d1f4f96c 100644 --- a/framework/infrastructure/gcp/compute.py +++ b/framework/infrastructure/gcp/compute.py @@ -39,7 +39,6 @@ class ComputeV1( _WAIT_FOR_BACKEND_SLEEP_SEC = 4 _WAIT_FOR_OPERATION_SEC = 60 * 10 gfe_debug_header: Optional[str] - add_dualstack_support: bool = False @dataclasses.dataclass(frozen=True) class GcpResource: @@ -62,12 +61,10 @@ def __init__( api_manager: gcp.api.GcpApiManager, project: str, gfe_debug_header: Optional[str] = None, - add_dualstack_support: bool = False, version: str = "v1", ): super().__init__(api_manager.compute(version), project) self.gfe_debug_header = gfe_debug_header - self.add_dualstack_support = add_dualstack_support class HealthCheckProtocol(enum.Enum): TCP = enum.auto() @@ -154,6 +151,7 @@ def create_backend_service_traffic_director( subset_size: Optional[int] = None, locality_lb_policies: Optional[List[dict]] = None, outlier_detection: Optional[dict] = None, + enable_dualstack: bool = False, ) -> "GcpResource": if not isinstance(protocol, self.BackendServiceProtocol): raise TypeError(f"Unexpected Backend Service protocol: {protocol}") @@ -165,7 +163,7 @@ def create_backend_service_traffic_director( } # If add dualstack support is specified True, config the backend service # to support IPv6 - if self.add_dualstack_support: + if enable_dualstack: body["ipAddressSelectionPolicy"] = "PREFER_IPV6" # If affinity header is specified, config the backend service to support # affinity, and set affinity header to the one given. diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 2bf1aff3..2e191eed 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -116,7 +116,6 @@ def __init__( project, version=compute_api_version, gfe_debug_header=xds_flags.GFE_DEBUG_HEADER.value, - add_dualstack_support=xds_flags.ADD_DUALSTACK_SUPPORT.value, ) # Settings @@ -132,14 +131,14 @@ def __init__( self.firewall_rule: Optional[GcpResource] = None self.firewall_rule_ipv6: Optional[GcpResource] = None self.target_proxy: Optional[GcpResource] = None - self.target_proxy_v6: Optional[GcpResource] = None + self.target_proxy_ipv6: Optional[GcpResource] = None # TODO(sergiitk): remove this flag once target proxy resource loaded self.target_proxy_is_http: bool = False self.alternative_target_proxy: Optional[GcpResource] = None self.forwarding_rule: Optional[GcpResource] = None - self.forwarding_rule_v6: Optional[GcpResource] = None + self.forwarding_rule_ipv6: Optional[GcpResource] = None self.alternative_forwarding_rule: Optional[GcpResource] = None - self.add_dualstack_support = xds_flags.ADD_DUALSTACK_SUPPORT.value + self.enable_dualstack = xds_flags.ENABLE_DUALSTACK.value # Backends. self.backends = set() @@ -177,11 +176,9 @@ def setup_routing_rule_map_for_grpc(self, service_host, service_port): self.create_target_proxy() self.create_forwarding_rule(service_port) - if self.add_dualstack_support: - self.create_target_proxy(ipv6=True) - self.create_forwarding_rule( - service_port, ipv6=True, ip_address="::" - ) + if self.enable_dualstack: + self.create_target_proxy_ipv6() + self.create_forwarding_rule_ipv6(service_port) def cleanup(self, *, force=False): # Cleanup in the reverse order of creation @@ -190,9 +187,8 @@ def cleanup(self, *, force=False): self.delete_alternative_forwarding_rule(force=force) self.delete_target_http_proxy(force=force) self.delete_target_grpc_proxy(force=force) - if self.add_dualstack_support: - self.delete_target_http_proxy(force=force, ipv6=True) - self.delete_forwarding_rule(force=force, ipv6=True) + if self.enable_dualstack: + self.delete_target_proxy_ipv6(force=force) self.delete_alternative_target_grpc_proxy(force=force) self.delete_url_map(force=force) self.delete_alternative_url_map(force=force) @@ -261,6 +257,7 @@ def create_backend_service( affinity_header=affinity_header, locality_lb_policies=locality_lb_policies, outlier_detection=outlier_detection, + enable_dualstack=self.enable_dualstack, ) self.backend_service = resource self.backend_service_protocol = protocol @@ -352,7 +349,10 @@ def create_alternative_backend_service( 'Creating %s Alternative Backend Service "%s"', protocol.name, name ) resource = self.compute.create_backend_service_traffic_director( - name, health_check=self.health_check, protocol=protocol + name, + health_check=self.health_check, + protocol=protocol, + enable_dualstack=self.enable_dualstack, ) self.alternative_backend_service = resource self.alternative_backend_service_protocol = protocol @@ -433,6 +433,7 @@ def create_affinity_backend_service( health_check=self.health_check, protocol=protocol, affinity_header=TEST_AFFINITY_METADATA_KEY, + enable_dualstack=self.enable_dualstack, ) self.affinity_backend_service = resource self.affinity_backend_service_protocol = protocol @@ -608,12 +609,8 @@ def delete_alternative_url_map(self, force=False): self.compute.delete_url_map(name) self.url_map = None - def create_target_proxy(self, ipv6=False): - if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) - else: - name = self.make_resource_name(self.TARGET_PROXY_NAME) - + def create_target_proxy(self): + name = self.make_resource_name(self.TARGET_PROXY_NAME) if self.backend_service_protocol is BackendServiceProtocol.GRPC: target_proxy_type = "GRPC" create_proxy_fn = self.compute.create_target_grpc_proxy @@ -631,58 +628,58 @@ def create_target_proxy(self, ipv6=False): target_proxy_type, self.url_map.name, ) + self.target_proxy = create_proxy_fn(name, self.url_map) - if ipv6: - self.target_proxy_v6 = create_proxy_fn(name, self.url_map) - else: - self.target_proxy = create_proxy_fn(name, self.url_map) + def create_target_proxy_ipv6(self): + name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) + target_proxy_type = "HTTP" # TODO: Support GRPC target proxy as well + create_proxy_fn = self.compute.create_target_http_proxy - def delete_target_grpc_proxy(self, force=False, ipv6=False): + logger.info( + 'Creating target %s proxy "%s" to URL map %s', + name, + target_proxy_type, + self.url_map.name, + ) + self.target_proxy_ipv6 = create_proxy_fn(name, self.url_map) + + def delete_target_grpc_proxy(self, force=False): if force: - if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) - else: - name = self.make_resource_name(self.TARGET_PROXY_NAME) - elif ipv6: - if self.target_proxy_v6: - name = self.target_proxy_v6.name + name = self.make_resource_name(self.TARGET_PROXY_NAME) elif self.target_proxy: name = self.target_proxy.name else: return - logger.info('Deleting Target GRPC proxy "%s"', name) self.compute.delete_target_grpc_proxy(name) - if ipv6: - self.target_proxy_v6 = None - else: - self.target_proxy = None - + self.target_proxy = None self.target_proxy_is_http = False - def delete_target_http_proxy(self, force=False, ipv6=False): + def delete_target_http_proxy(self, force=False): if force: - if ipv6: - name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) - else: - name = self.make_resource_name(self.TARGET_PROXY_NAME) - elif ipv6: - if self.target_proxy_v6 and self.target_proxy_is_http: - name = self.target_proxy_v6.name + name = self.make_resource_name(self.TARGET_PROXY_NAME) elif self.target_proxy and self.target_proxy_is_http: name = self.target_proxy.name else: return - logger.info('Deleting HTTP Target proxy "%s"', name) self.compute.delete_target_http_proxy(name) - if ipv6: - self.target_proxy_v6 = None - else: - self.target_proxy = None - + self.target_proxy = None self.target_proxy_is_http = False + def delete_target_proxy_ipv6(self, force=False): + if force: + name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) + elif self.target_proxy_ipv6: + name = self.target_proxy_ipv6.name + else: + return + logger.info('Deleting Target HTTP proxy "%s"', name) + self.compute.delete_target_http_proxy( + name + ) # TODO: Support GRPC target proxy as well + self.target_proxy_ipv6 = None + def create_alternative_target_proxy(self): name = self.make_resource_name(self.ALTERNATIVE_TARGET_PROXY_NAME) if self.backend_service_protocol is BackendServiceProtocol.GRPC: @@ -724,60 +721,59 @@ def find_unused_forwarding_rule_port( # TODO(sergiitk): custom exception raise RuntimeError("Couldn't find unused forwarding rule port") - def create_forwarding_rule( - self, src_port: int, ipv6=False, ip_address="0.0.0.0" - ): - if ipv6: - name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) - target_proxy = self.target_proxy_v6 - else: - name = self.make_resource_name(self.FORWARDING_RULE_NAME) - target_proxy = self.target_proxy + def create_forwarding_rule(self, src_port: int): + name = self.make_resource_name(self.FORWARDING_RULE_NAME) + src_port = int(src_port) + logging.info( + 'Creating forwarding rule "%s" in network "%s": 0.0.0.0:%s -> %s', + name, + self.network, + src_port, + self.target_proxy.url, + ) + resource = self.compute.create_forwarding_rule( + name, src_port, self.target_proxy, self.network_url + ) + self.forwarding_rule = resource + return resource + + def create_forwarding_rule_ipv6(self, src_port: int): + name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) src_port = int(src_port) logging.info( - 'Creating forwarding rule "%s" in network "%s": %s:%s -> %s', + 'Creating forwarding rule "%s" in network "%s": [::]:%s -> %s', name, self.network, src_port, - ip_address, - target_proxy.url, + self.target_proxy_ipv6.url, ) resource = self.compute.create_forwarding_rule( name, src_port, - target_proxy, + self.target_proxy_ipv6, self.network_url, - ip_address=ip_address, + ip_address="::", ) - - if ipv6: - self.forwarding_rule_v6 = resource - else: - self.forwarding_rule = resource + self.forwarding_rule_ipv6 = resource return resource - def delete_forwarding_rule(self, force=False, ipv6=False): + def delete_forwarding_rule(self, force=False): if force: - if ipv6: - name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) - else: - name = self.make_resource_name(self.FORWARDING_RULE_NAME) - elif ipv6: - if self.forwarding_rule_v6: - name = self.forwarding_rule_v6.name + name = self.make_resource_name(self.FORWARDING_RULE_NAME) + name_ipv6 = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) elif self.forwarding_rule: name = self.forwarding_rule.name + name_ipv6 = self.forwarding_rule_ipv6.name else: return - logger.info('Deleting Forwarding rule "%s"', name) self.compute.delete_forwarding_rule(name) - if ipv6: - self.forwarding_rule_v6 = None - else: - self.forwarding_rule = None + self.forwarding_rule = None + logger.info('Deleting Forwarding rule "%s"', name_ipv6) + self.compute.delete_forwarding_rule(name_ipv6) + self.forwarding_rule_ipv6 = None def create_alternative_forwarding_rule( self, src_port: int, ip_address="0.0.0.0" diff --git a/framework/test_app/runners/k8s/k8s_base_runner.py b/framework/test_app/runners/k8s/k8s_base_runner.py index aa37c39f..a5b9834f 100644 --- a/framework/test_app/runners/k8s/k8s_base_runner.py +++ b/framework/test_app/runners/k8s/k8s_base_runner.py @@ -1000,10 +1000,10 @@ def _wait_pod_started(self, name, **kwargs) -> k8s.V1Pod: pod = self.k8s_namespace.get_pod(name) if hasattr(pod.status, "pod_ip_s"): # if running with dualstack support - pod_ip_s = pod.status.pod_ip_s + pod_ips = pod.status.pod_ip_s else: - pod_ip_s = pod.status.pod_ip - logger.info("Pod %s ready, IP: %s", pod.metadata.name, pod_ip_s) + pod_ips = pod.status.pod_ip + logger.info("Pod %s ready, IP: %s", pod.metadata.name, pod_ips) return pod def _pod_started_logic(self, pod: k8s.V1Pod) -> bool: diff --git a/framework/xds_flags.py b/framework/xds_flags.py index 4991fac0..9070326e 100644 --- a/framework/xds_flags.py +++ b/framework/xds_flags.py @@ -190,10 +190,10 @@ help="Whether to enable GFE debug headers and what value to use.", ) -ADD_DUALSTACK_SUPPORT = flags.DEFINE_bool( - "add_dualstack_support", +ENABLE_DUALSTACK = flags.DEFINE_bool( + "enable_dualstack", default=False, - help=(f"Add support for Dual Stack resources to the framework."), + help=("Enable support for Dual Stack resources to the framework."), ) diff --git a/kubernetes-manifests/client.deployment.yaml b/kubernetes-manifests/client.deployment.yaml index 4e6ad5b7..6f5ada29 100644 --- a/kubernetes-manifests/client.deployment.yaml +++ b/kubernetes-manifests/client.deployment.yaml @@ -66,7 +66,7 @@ spec: value: "true" - name: GRPC_EXPERIMENTAL_XDS_ENABLE_OVERRIDE_HOST value: "true" - % if add_dualstack_support: + % if enable_dualstack: - name: GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST value: "true" - name: GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS diff --git a/kubernetes-manifests/server.service.yaml b/kubernetes-manifests/server.service.yaml index ce2f6ae7..bfb13c00 100644 --- a/kubernetes-manifests/server.service.yaml +++ b/kubernetes-manifests/server.service.yaml @@ -10,7 +10,7 @@ metadata: cloud.google.com/neg: '{"exposed_ports": {"${test_port}":{"name":"${neg_name}"}}}' spec: type: ClusterIP - % if add_dualstack_support: + % if enable_dualstack: ipFamilyPolicy: PreferDualStack ipFamilies: - IPv6 From dc74c632fdb0f69f5360b4896c32661bf55ae27c Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Sat, 20 Jul 2024 12:14:38 +0530 Subject: [PATCH 09/18] Address Sergii's comments: Refactor enable_dualstack flag to xds_k8s_testcase, add enable_dualstack to client and server deployment --- framework/infrastructure/gcp/api.py | 2 +- framework/infrastructure/traffic_director.py | 23 ++++++++++++++----- .../runners/k8s/k8s_xds_client_runner.py | 1 + .../runners/k8s/k8s_xds_server_runner.py | 3 +++ framework/xds_flags.py | 2 +- framework/xds_k8s_testcase.py | 14 ++++++++++- 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/framework/infrastructure/gcp/api.py b/framework/infrastructure/gcp/api.py index 84b8446c..e85f69b9 100644 --- a/framework/infrastructure/gcp/api.py +++ b/framework/infrastructure/gcp/api.py @@ -157,7 +157,7 @@ def compute(self, version: str): return self._build_from_discovery_v1(api_name, version) elif version == "v1alpha": return self._build_from_discovery_v1(api_name, "alpha") - elif version in ["v1beta", "v1beta1"]: + elif version == "v1beta": return self._build_from_discovery_v1(api_name, "beta") raise NotImplementedError(f"Compute {version} not supported") diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 2e191eed..23f31629 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -109,6 +109,7 @@ def __init__( resource_suffix: str, network: str = "default", compute_api_version: str = "v1", + enable_dualstack: bool = False, ): # API self.compute = _ComputeV1( @@ -123,6 +124,7 @@ def __init__( self.network: str = network self.resource_prefix: str = resource_prefix self.resource_suffix: str = resource_suffix + self.enable_dualstack: bool = enable_dualstack # Managed resources self.health_check: Optional[GcpResource] = None @@ -138,7 +140,6 @@ def __init__( self.forwarding_rule: Optional[GcpResource] = None self.forwarding_rule_ipv6: Optional[GcpResource] = None self.alternative_forwarding_rule: Optional[GcpResource] = None - self.enable_dualstack = xds_flags.ENABLE_DUALSTACK.value # Backends. self.backends = set() @@ -189,6 +190,7 @@ def cleanup(self, *, force=False): self.delete_target_grpc_proxy(force=force) if self.enable_dualstack: self.delete_target_proxy_ipv6(force=force) + self.delete_forwarding_rule_ipv6(force=force) self.delete_alternative_target_grpc_proxy(force=force) self.delete_url_map(force=force) self.delete_alternative_url_map(force=force) @@ -740,7 +742,6 @@ def create_forwarding_rule(self, src_port: int): def create_forwarding_rule_ipv6(self, src_port: int): name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) - src_port = int(src_port) logging.info( 'Creating forwarding rule "%s" in network "%s": [::]:%s -> %s', name, @@ -762,17 +763,23 @@ def create_forwarding_rule_ipv6(self, src_port: int): def delete_forwarding_rule(self, force=False): if force: name = self.make_resource_name(self.FORWARDING_RULE_NAME) - name_ipv6 = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) elif self.forwarding_rule: name = self.forwarding_rule.name - name_ipv6 = self.forwarding_rule_ipv6.name else: return logger.info('Deleting Forwarding rule "%s"', name) self.compute.delete_forwarding_rule(name) self.forwarding_rule = None - logger.info('Deleting Forwarding rule "%s"', name_ipv6) - self.compute.delete_forwarding_rule(name_ipv6) + + def delete_forwarding_rule_ipv6(self, force=False): + if force: + name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) + elif self.forwarding_rule_ipv6: + name = self.forwarding_rule_ipv6.name + else: + return + logger.info('Deleting Forwarding rule "%s"', name) + self.compute.delete_forwarding_rule(name) self.forwarding_rule_ipv6 = None def create_alternative_forwarding_rule( @@ -914,6 +921,7 @@ def __init__( resource_suffix: Optional[str] = None, network: str = "default", compute_api_version: str = "v1", + enable_dualstack: bool = False, ): super().__init__( gcp_api_manager, @@ -922,6 +930,7 @@ def __init__( resource_suffix=resource_suffix, network=network, compute_api_version=compute_api_version, + enable_dualstack=enable_dualstack, ) # API @@ -1038,6 +1047,7 @@ def __init__( resource_suffix: Optional[str] = None, network: str = "default", compute_api_version: str = "v1", + enable_dualstack: bool = False, ): super().__init__( gcp_api_manager, @@ -1046,6 +1056,7 @@ def __init__( resource_suffix=resource_suffix, network=network, compute_api_version=compute_api_version, + enable_dualstack=enable_dualstack, ) # API diff --git a/framework/test_app/runners/k8s/k8s_xds_client_runner.py b/framework/test_app/runners/k8s/k8s_xds_client_runner.py index 9c5110af..db98bd2f 100644 --- a/framework/test_app/runners/k8s/k8s_xds_client_runner.py +++ b/framework/test_app/runners/k8s/k8s_xds_client_runner.py @@ -31,6 +31,7 @@ class ClientDeploymentArgs: enable_csm_observability: bool = False csm_workload_name: str = "" csm_canonical_service_name: str = "" + enable_dualstack: bool = False def as_dict(self): return dataclasses.asdict(self) diff --git a/framework/test_app/runners/k8s/k8s_xds_server_runner.py b/framework/test_app/runners/k8s/k8s_xds_server_runner.py index ffe60c39..045e57d0 100644 --- a/framework/test_app/runners/k8s/k8s_xds_server_runner.py +++ b/framework/test_app/runners/k8s/k8s_xds_server_runner.py @@ -108,6 +108,7 @@ def __init__( # pylint: disable=too-many-locals debug_use_port_forwarding: bool = False, enable_workload_identity: bool = True, deployment_args: Optional[ServerDeploymentArgs] = None, + enable_dualstack: bool = False, ): super().__init__( k8s_namespace, @@ -142,6 +143,7 @@ def __init__( # pylint: disable=too-many-locals self.td_bootstrap_image = td_bootstrap_image self.network = network self.xds_server_uri = xds_server_uri + self.enable_dualstack = enable_dualstack # Workload identity settings: if self.enable_workload_identity: @@ -223,6 +225,7 @@ def run( # pylint: disable=arguments-differ,too-many-branches deployment_name=self.deployment_name, neg_name=self.gcp_neg_name, test_port=test_port, + enable_dualstack=self.enable_dualstack, ) self._wait_service_neg_status_annotation(self.service_name, test_port) diff --git a/framework/xds_flags.py b/framework/xds_flags.py index 9070326e..3960b27f 100644 --- a/framework/xds_flags.py +++ b/framework/xds_flags.py @@ -193,7 +193,7 @@ ENABLE_DUALSTACK = flags.DEFINE_bool( "enable_dualstack", default=False, - help=("Enable support for Dual Stack resources to the framework."), + help="Enable support for Dual Stack resources to the framework.", ) diff --git a/framework/xds_k8s_testcase.py b/framework/xds_k8s_testcase.py index 58e0ed10..49106b19 100644 --- a/framework/xds_k8s_testcase.py +++ b/framework/xds_k8s_testcase.py @@ -65,6 +65,7 @@ TrafficDirectorSecureManager = traffic_director.TrafficDirectorSecureManager XdsTestServer = server_app.XdsTestServer XdsTestClient = client_app.XdsTestClient +ClientDeploymentArgs = k8s_xds_client_runner.ClientDeploymentArgs KubernetesServerRunner = k8s_xds_server_runner.KubernetesServerRunner KubernetesClientRunner = k8s_xds_client_runner.KubernetesClientRunner _LoadBalancerStatsResponse = grpc_testing.LoadBalancerStatsResponse @@ -146,6 +147,7 @@ class XdsKubernetesBaseTestCase(base_testcase.BaseTestCase): _prev_sigint_handler: Optional[_SignalHandler] = None _handling_sigint: bool = False yaml_highlighter: framework.helpers.highlighter.HighlighterYaml = None + enable_dualstack: bool = False @staticmethod def is_supported(config: skips.TestConfig) -> bool: @@ -179,6 +181,7 @@ def setUpClass(cls): cls.td_bootstrap_image = xds_k8s_flags.TD_BOOTSTRAP_IMAGE.value cls.xds_server_uri = xds_flags.XDS_SERVER_URI.value cls.compute_api_version = xds_flags.COMPUTE_API_VERSION.value + cls.enable_dualstack = xds_flags.ENABLE_DUALSTACK.value # Firewall cls.ensure_firewall = xds_flags.ENSURE_FIREWALL.value @@ -796,7 +799,11 @@ def setUp(self): self.client_namespace = KubernetesClientRunner.make_namespace_name( self.resource_prefix, self.resource_suffix ) - self.client_runner = self.initKubernetesClientRunner() + self.client_runner = self.initKubernetesClientRunner( + deployment_args=ClientDeploymentArgs( + enable_dualstack=self.enable_dualstack + ) + ) # Create healthcheck firewall rules if necessary. if self.ensure_firewall: @@ -940,6 +947,7 @@ def initTrafficDirectorManager(self) -> TrafficDirectorManager: resource_suffix=self.resource_suffix, network=self.network, compute_api_version=self.compute_api_version, + enable_dualstack=self.enable_dualstack, ) def initKubernetesServerRunner(self, **kwargs) -> KubernetesServerRunner: @@ -957,6 +965,7 @@ def initKubernetesServerRunner(self, **kwargs) -> KubernetesServerRunner: network=self.network, debug_use_port_forwarding=self.debug_use_port_forwarding, enable_workload_identity=self.enable_workload_identity, + enable_dualstack=self.enable_dualstack, **kwargs, ) @@ -1014,6 +1023,7 @@ def initTrafficDirectorManager(self) -> TrafficDirectorAppNetManager: resource_suffix=self.resource_suffix, network=self.network, compute_api_version=self.compute_api_version, + enable_dualstack=self.enable_dualstack, ) @@ -1051,6 +1061,7 @@ def initTrafficDirectorManager(self) -> TrafficDirectorSecureManager: resource_suffix=self.resource_suffix, network=self.network, compute_api_version=self.compute_api_version, + enable_dualstack=self.enable_dualstack, ) def initKubernetesServerRunner(self, **kwargs) -> KubernetesServerRunner: @@ -1068,6 +1079,7 @@ def initKubernetesServerRunner(self, **kwargs) -> KubernetesServerRunner: xds_server_uri=self.xds_server_uri, deployment_template="server-secure.deployment.yaml", debug_use_port_forwarding=self.debug_use_port_forwarding, + enable_dualstack=self.enable_dualstack, **kwargs, ) From dd087fbc638b7813ad684d836d3813f2377d18ef Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 15:21:56 +0530 Subject: [PATCH 10/18] Fix order of deletion of forwarding rule and target proxy --- framework/infrastructure/traffic_director.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 23f31629..ebc2dec6 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -189,8 +189,8 @@ def cleanup(self, *, force=False): self.delete_target_http_proxy(force=force) self.delete_target_grpc_proxy(force=force) if self.enable_dualstack: - self.delete_target_proxy_ipv6(force=force) self.delete_forwarding_rule_ipv6(force=force) + self.delete_target_proxy_ipv6(force=force) self.delete_alternative_target_grpc_proxy(force=force) self.delete_url_map(force=force) self.delete_alternative_url_map(force=force) From 5eac361d6aebaaf4e2f336534d4a4293037ff4b1 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:54:11 +0530 Subject: [PATCH 11/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 1 - 1 file changed, 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index ebc2dec6..5c0b251d 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -737,7 +737,6 @@ def create_forwarding_rule(self, src_port: int): name, src_port, self.target_proxy, self.network_url ) self.forwarding_rule = resource - return resource def create_forwarding_rule_ipv6(self, src_port: int): From 8638800d6380b7fbc93cbb4a5ac517c50181e811 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:54:30 +0530 Subject: [PATCH 12/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 5c0b251d..1422f3bb 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -777,7 +777,7 @@ def delete_forwarding_rule_ipv6(self, force=False): name = self.forwarding_rule_ipv6.name else: return - logger.info('Deleting Forwarding rule "%s"', name) + logger.info('Deleting IPv6 Forwarding rule "%s"', name) self.compute.delete_forwarding_rule(name) self.forwarding_rule_ipv6 = None From 7b13f1a19bb9a7ae2ce4c714a262d5c88975c771 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:54:43 +0530 Subject: [PATCH 13/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 1 - 1 file changed, 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 1422f3bb..52460589 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -756,7 +756,6 @@ def create_forwarding_rule_ipv6(self, src_port: int): ip_address="::", ) self.forwarding_rule_ipv6 = resource - return resource def delete_forwarding_rule(self, force=False): From 3993272272a51e5e7a0265fa012938bf076f3f1d Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:54:50 +0530 Subject: [PATCH 14/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 52460589..af447cb7 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -742,7 +742,7 @@ def create_forwarding_rule(self, src_port: int): def create_forwarding_rule_ipv6(self, src_port: int): name = self.make_resource_name(self.FORWARDING_RULE_NAME_IPV6) logging.info( - 'Creating forwarding rule "%s" in network "%s": [::]:%s -> %s', + 'Creating IPv6 forwarding rule "%s" in network "%s": [::]:%s -> %s', name, self.network, src_port, From 9f25d548bced3389c4696c39c0cf5a6c0f10d465 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:56:18 +0530 Subject: [PATCH 15/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index af447cb7..74e68252 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -676,7 +676,7 @@ def delete_target_proxy_ipv6(self, force=False): name = self.target_proxy_ipv6.name else: return - logger.info('Deleting Target HTTP proxy "%s"', name) + logger.info('Deleting IPv6 Target HTTP proxy "%s"', name) self.compute.delete_target_http_proxy( name ) # TODO: Support GRPC target proxy as well From c1d72a113397caf7e75f4808e7df2d1a2f4ad3e7 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:56:25 +0530 Subject: [PATCH 16/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 74e68252..44634445 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -638,7 +638,7 @@ def create_target_proxy_ipv6(self): create_proxy_fn = self.compute.create_target_http_proxy logger.info( - 'Creating target %s proxy "%s" to URL map %s', + 'Creating IPv6 target %s proxy "%s" to URL map %s', name, target_proxy_type, self.url_map.name, From 1cb6ac0bf7f7049329c977f8be121027bb68bdc3 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Tue, 23 Jul 2024 22:56:48 +0530 Subject: [PATCH 17/18] Update framework/infrastructure/traffic_director.py Co-authored-by: Sergii Tkachenko --- framework/infrastructure/traffic_director.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index 44634445..b1e9585a 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -634,7 +634,8 @@ def create_target_proxy(self): def create_target_proxy_ipv6(self): name = self.make_resource_name(self.TARGET_PROXY_NAME_IPV6) - target_proxy_type = "HTTP" # TODO: Support GRPC target proxy as well + # TODO(lsafran): Support GRPC target proxy as well + target_proxy_type = "HTTP" create_proxy_fn = self.compute.create_target_http_proxy logger.info( From 05061bb229e885add7d72cf42aba6c30e107256f Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Tue, 23 Jul 2024 11:05:47 -0700 Subject: [PATCH 18/18] Update framework/infrastructure/traffic_director.py --- framework/infrastructure/traffic_director.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/framework/infrastructure/traffic_director.py b/framework/infrastructure/traffic_director.py index b1e9585a..0004ad9d 100644 --- a/framework/infrastructure/traffic_director.py +++ b/framework/infrastructure/traffic_director.py @@ -677,10 +677,9 @@ def delete_target_proxy_ipv6(self, force=False): name = self.target_proxy_ipv6.name else: return + # TODO: Delete Target GRPC Proxy when added in create_target_proxy_ipv6. logger.info('Deleting IPv6 Target HTTP proxy "%s"', name) - self.compute.delete_target_http_proxy( - name - ) # TODO: Support GRPC target proxy as well + self.compute.delete_target_http_proxy(name) self.target_proxy_ipv6 = None def create_alternative_target_proxy(self):