test(chart): Sanity tests Selenium Grid chart via Makefile commands

[VietND96]

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Sanity tests Selenium Grid chart via Makefile commands

Motivation and Context

Take advantage from PR #2027, there are some enhancements on sanity tests for the Selenium Grid chart (link to #1975)

1. Makefile is updated new make commands for a few purposes. Someone is able to test their changes in their local before pushing and running workflow. Try to have the same environment in development and CI

# Setup Kubernetes environment
make chart_setup_env

# Setup Kubernetes cluster
make chart_cluster_setup

# Test Selenium Grid on Kubernetes
make chart_test

# Cleanup Kubernetes cluster
make chart_cluster_cleanup

2. Add charts/selenium-grid/TESTING.md to capture all related testing to chart

3. Update chart test workflow to use make commands.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[VietND96]

Hi @amardeep2006, can you please also have a review on this? Thanks!

[diemol]

Some minor comments.

[diemol]

Thank you, @VietND96!

[amardeep2006]

Hi @amardeep2006, can you please also have a review on this? Thanks!

1. I was looking at the run logs and seems few tests are failing with error 504. Probably Kubernetes needs some time to make the Browser Nodes operational.
2. Also It seems that even after failing the tests, the GitHub action does not break. Shouldn't it have failed ?
3. Is this possible to make kubernetes/helm/kubectl installation optional ? There may be developer who have kubernetes already up an running on the machine (e.g. I use Rancher desktop mostly that comes with helm,kubectl, ingress and k3s).
   They can simply apply chart and run ct lint / Selenium test cases.

https://github.com/SeleniumHQ/docker-selenium/actions/runs/7017524980/job/19090970950

[VietND96]

Yes, I was also aware of this flaky and incorrect behavior. I am trying to make it stable

[VietND96]

Is this possible to make kubernetes/helm/kubectl installation optional ? There may be developer who have kubernetes already up an running on the machine (e.g. I use Rancher desktop mostly that comes with helm,kubectl, ingress and k3s).
They can simply apply chart and run ct lint / Selenium test cases.

On this use case, how about if you jump to run make chart_test? Is it working?
Later I will break down the script with individual tool/component install...

[amardeep2006]

Yes, I was also aware of this flaky and incorrect behavior. I am trying to make it stable

I was using K8sSmokeTest.py for this. Basically it makes sure cluster is up an running before I fired the tests.

[VietND96]

I was using K8sSmokeTest.py for this. Basically it makes sure cluster is up an running before I fired the tests.

Yes, while integrating autoscaling & ingress tests, I also tried to merge your script into using SmokeTest.py, since it is same func to wait for Hub is up, just some timeout values are pulled out as arguments, K8s tests set timeout a little bit longer due to scaling interval and time for node replicas from 0 to 1.
Regarding the 504 error, I found looks like same as kubernetes/ingress-nginx#3976. The default timeout is 60s, if a request in the session queue over 60s without node serve on time, the client web driver will throw this error.
In the override values of ingress, under ingress.annotations I added

    nginx.ingress.kubernetes.io/proxy-connect-timeout: "360"
    nginx.ingress.kubernetes.io/proxy-read-timeout: "360"
    nginx.ingress.kubernetes.io/proxy-send-timeout: "360"

Let us monitor how it is stable

Regarding incorrect behavior, failed tests without job failed, it is due to in bash script handle trapping the error signal inproperly, step to cleanup between return 0 replaces the first error code from tests.
All those things will be fixed in the next PR