-
Notifications
You must be signed in to change notification settings - Fork 2k
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
AppProtect gRPC automation tests #1603
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @vepatel
please see my questions and comments
tests/suite/test_app_protect_grpc.py
Outdated
Client sends request to /sayhello | ||
""" | ||
syslog_pod = kube_apis.v1.list_namespaced_pod(test_namespace).items[-1].metadata.name | ||
block_response = subprocess.run(["binaries/./grpc_client", "-address", f"{backend_setup.ingress_host}:{backend_setup.ssl_port}"], capture_output=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to use https://github.com/grpc/grpc/blob/v1.37.1/examples/python/helloworld/greeter_client.py ? (providing we can make it work). this way there will be no need to add any binaries to the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm checking https://pypi.org/project/grpc-requests/ which is based on the same, will update if it works!
spec: | ||
containers: | ||
- name: greeter | ||
image: nginxkic/test-grpc-server:0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we publish the sources similarly to how we published the sources for the TCP and UDP servers? so we don't loose it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too sure about publishing sources for this one as the code isn't written by our team members unlike TCP/UDP server one which @soneillf5 wrote. Also If we're to add source in the tests, i'd prefer adding them to examples/
first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the case of the backend, it's just a Dockerfile, right? we don't really modify the backend code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does the test-grpc-server come from then ?
tests/suite/test_app_protect_grpc.py
Outdated
print("------------------------- Deploy Syslog -----------------------------") | ||
src_syslog_yaml = f"{TEST_DATA}/appprotect/syslog.yaml" | ||
create_items_from_yaml(kube_apis, src_syslog_yaml, test_namespace) | ||
wait_before_test(40) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this wait is necessary here. is it for syslog pods? if that is the case, can we wait until the pod is ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great suggestion but there is an issue at times when the pod is ready but endpoint isn't which is why the wait is for (see error below), I'll reduce the wait and use it in conjunction with waiting for pod.
line 85, in backend_setup
.subsets[0]
TypeError: 'NoneType' object is not subscriptable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. maybe we can wait for that condition (we have 1 endpoint), instead of for the pod being ready?
Proposed changes
Checklist
Before creating a PR, run through this checklist and mark each as complete.