Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch up AppNetCsm test with latest changes #98

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

stanley-cheung
Copy link
Contributor

@stanley-cheung stanley-cheung commented Jun 6, 2024

#91, #95, #96, #97 should all be done to the AppNetCsm test as well, just that the test was merged after the fact.

Next step is to refactor out all the common logic into a Mixin class.

@stanley-cheung stanley-cheung requested a review from sergiitk June 6, 2024 18:10
@stanley-cheung
Copy link
Contributor Author

stanley-cheung commented Jun 6, 2024

Copy link
Member

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this change, but I think we'll benefit much more from moving common functionality into a mixin, similar to https://github.com/grpc/psm-interop/blob/main/framework/test_cases/session_affinity_mixin.py.

@sergiitk
Copy link
Member

sergiitk commented Jun 6, 2024

cc @ejona86 @zasweq @XuanWang-Amos

Copy link
Member

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running ad-hoc test for the rest of the languages, and the oldest cpp branch.

tests/app_net_csm_observability_test.py Show resolved Hide resolved
Copy link
Member

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go and Python failed.

Go:

Traceback (most recent call last):
  File "/tmp/tmp.6tlW3Jub2y/psm-interop/tests/app_net_csm_observability_test.py", line 331, in test_csm_observability
    self.assertDictEqual(
AssertionError: {'csm_mesh_id': <ANY>, 'csm_remote_workload_canonical_service': 'csm_canonical_service_name_from_server', 'csm_remote_workload_cluster_name': <ANY>, 'csm_remote_workload_location': <ANY>, 'csm_remote_workload_name': 'csm_workload_name_from_server', 'csm_remote_workload_namespace_name': 'psm-csm-server-20240606-2158-i8zi1', 'csm_remote_workload_project_id': 'grpc-testing', 'csm_remote_workload_type': 'gcp_kubernetes_engine', 'csm_service_name': 'psm-csm-backend-service-20240606-2158-i8zi1', 'csm_service_namespace_name': 'unknown', 'csm_workload_canonical_service': 'csm_canonical_service_name_from_client', 'grpc_method': 'grpc.testing.TestService/UnaryCall', 'grpc_status': 'OK', 'grpc_target': <ANY>, 'otel_scope_name': <ANY>, 'otel_scope_version': <ANY>, 'pod': 'psm-grpc-client-5888bfb897-xkl6x'} != {'csm_mesh_id': 'psm-csm-mesh-20240606-2158-i8zi1', 'csm_remote_workload_canonical_service': 'csm_canonical_service_name_from_server', 'csm_remote_workload_cluster_name': 'psm-interop-csm', 'csm_remote_workload_location': 'us-central1', 'csm_remote_workload_name': 'csm_workload_name_from_server', 'csm_remote_workload_namespace_name': 'psm-csm-server-20240606-2158-i8zi1', 'csm_remote_workload_project_id': 'grpc-testing', 'csm_remote_workload_type': 'gcp_kubernetes_engine', 'csm_service_name': 'psm-csm-backend-service-20240606-2158-i8zi1', 'csm_workload_canonical_service': 'csm_canonical_service_name_from_client', 'grpc_method': 'grpc.testing.TestService/UnaryCall', 'grpc_status': 'OK', 'grpc_target': 'xds:///psm-grpc-server:58628', 'otel_scope_name': 'grpc-go', 'otel_scope_version': '1.65.0-dev', 'pod': 'psm-grpc-client-5888bfb897-xkl6x'}
Missing entries:
'csm_service_namespace_name': 'unknown'

Python:

Traceback (most recent call last):
  File "/tmp/tmp.xm4HFPozRO/psm-interop/framework/xds_k8s_testcase.py", line 879, in tearDown
    self.assertEqual(
AssertionError: 5 != 0 : Server container unexpectedly restarted 5 times during test. In most cases, this is caused by the test client app crash.

Copy link
Member

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go fix merged: grpc/grpc#36827.

@sergiitk sergiitk merged commit ca63359 into grpc:main Jun 11, 2024
7 checks passed
@stanley-cheung stanley-cheung deleted the update-app-net-csm-test branch June 13, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants