docker-selenium
docker-selenium copied to clipboard
SeleniumHQ
Adds helm-chart releaseName to all selectors in resources
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
This enables us to create multiple deployments of Selenium in the same namespace.
E.g. "selenium-v4" with the images being version 4.X and "selenium-v3" with the images being 3.X
Motivation and Context
I need to run multiple versions of the Selenium Grid in the same namespace. It is not enough to just use nameOverride for all components, because the underlying selectors, e.g., of the service will route to both pods.
The issue arises if you have different versions of Selenium running in the same namespace. Although I provide nameOverride for the selenium-hub (and other components) the selenium-hub service will route to all pods with labels app: selenium-hub. Hence, the service will also route to the other deployment.
Using the chart release name & the app name in tandem might be a better selector, which is what I implemented.
Types of changes
- [X] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist
- [X] 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.
- [X] All new and existing tests passed.
Hi @Xcalizorz Do you know if adding this property to the charts affect others who don't use that property? Will this be a seamless experience for others with already-deployed grids? Can you help us validate this? Sorry if this is a silly question. The person who would normally review this PR is away for the time being.
Sorry for the delay. And yeah, and that person is not me to clarify. 😅 But happy to peek in the meantime.
It looks like the selectors are already added on deployment via _helpers.tpl, so I think the only thing that needs to be added is the matchLabels (to the deployments). On that note, can this also be done w/in the helper file (e.g.,)?
Let me know if I'm off-base here.
trunk kubectl get pod --selector="app.kubernetes.io/instance=selenium-grid"
NAME READY STATUS RESTARTS AGE
selenium-chrome-node-7cc9c98d4-kwwr9 1/1 Running 0 112s
selenium-distributor-5d658b676f-jcxpb 1/1 Running 0 112s
selenium-edge-node-85d7b66684-pk6zg 1/1 Running 0 112s
selenium-event-bus-586fdf9895-9xg2j 1/1 Running 0 112s
selenium-firefox-node-7745686c76-vdsv8 1/1 Running 0 112s
selenium-router-6578c54658-9dxnj 1/1 Running 0 112s
selenium-session-map-7d4cf44687-86gfj 1/1 Running 0 112s
selenium-session-queue-769fd547f8-76tx6 1/1 Running 0 112s
trunk kubectl get svc --selector="app.kubernetes.io/instance=selenium-grid"
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
selenium-chrome-node ClusterIP 10.110.17.193 <none> 6900/TCP 2m48s
selenium-distributor ClusterIP 10.98.116.20 <none> 5553/TCP 2m48s
selenium-edge-node ClusterIP 10.110.216.43 <none> 6900/TCP 2m48s
selenium-event-bus ClusterIP 10.96.60.2 <none> 5557/TCP,4442/TCP,4443/TCP 2m48s
selenium-firefox-node ClusterIP 10.99.133.250 <none> 6900/TCP 2m48s
selenium-router ClusterIP 10.108.187.70 <none> 4444/TCP 2m48s
selenium-session-map ClusterIP 10.109.82.130 <none> 5556/TCP 2m48s
selenium-session-queue ClusterIP 10.98.92.121 <none> 5559/TCP 2m48s
trunk kubectl get deployments --selector="app.kubernetes.io/instance=selenium-grid"
NAME READY UP-TO-DATE AVAILABLE AGE
selenium-chrome-node 1/1 1 1 2m57s
selenium-distributor 1/1 1 1 2m57s
selenium-edge-node 1/1 1 1 2m57s
selenium-event-bus 1/1 1 1 2m57s
selenium-firefox-node 1/1 1 1 2m57s
selenium-router 1/1 1 1 2m57s
selenium-session-map 1/1 1 1 2m57s
selenium-session-queue 1/1 1 1 2m57s
helm install selenium-grid docker-selenium/selenium-grid --set isolateComponents=true --debug --dry-run:
....
# Source: selenium-grid/templates/chrome-node-service.yaml
apiVersion: v1
kind: Service
metadata:
name: selenium-chrome-node
namespace: default
labels:
name: selenium-chrome-node
app.kubernetes.io/managed-by: helm
app.kubernetes.io/instance: selenium-grid
app.kubernetes.io/version: 4.4.0-20220831
app.kubernetes.io/component: selenium-grid-4.4.0-20220831
helm.sh/chart: selenium-grid-0.10.0
....
# Source: selenium-grid/templates/chrome-node-deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: selenium-chrome-node
namespace: default
labels: &chrome_node_labels
app: selenium-chrome-node
app.kubernetes.io/name: selenium-chrome-node
app.kubernetes.io/managed-by: helm
app.kubernetes.io/instance: selenium-grid
app.kubernetes.io/version: 4.4.0-20220831
app.kubernetes.io/component: selenium-grid-4.4.0-20220831
helm.sh/chart: selenium-grid-0.10.0
....
Hi @jamesmortensen As far as I know, changing the selectors will force a recreate (you can not edit selectors like resource labels, you have to recreate the resource).
Steps I used for testing
- deployed the current charts from trunk
- used
helm upgrade selenium-grid .in my own fork
TL; DR: People who already deployed Selenium would need to uninstall and install... which causes a short downtime. For new users it would make no difference since there will only be a difference if you deploy multiple instances of selenium in the same namespace.
As expected you can not edit selectors (immutable objects), you have to recreate:
▶ helm upgrade selenium-grid .
Error: UPGRADE FAILED: cannot patch "selenium-chrome-node" with kind Deployment: Deployment.apps "selenium-chrome-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-chrome-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "selenium-edge-node" with kind Deployment: Deployment.apps "selenium-edge-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-edge-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "selenium-firefox-node" with kind Deployment: Deployment.apps "selenium-firefox-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-firefox-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "selenium-hub" with kind Deployment: Deployment.apps "selenium-hub" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-hub", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable
Unfortionately there is no --recreate flag in helm, yet. Using force does not help:
▶ helm upgrade selenium-grid . --force
Error: UPGRADE FAILED: failed to replace object: Deployment.apps "selenium-chrome-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-chrome-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && failed to replace object: Deployment.apps "selenium-edge-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-edge-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && failed to replace object: Deployment.apps "selenium-firefox-node" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-firefox-node", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && failed to replace object: Deployment.apps "selenium-hub" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app":"selenium-hub", "app.kubernetes.io/instance":"selenium-grid"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable
Sorry for the delay. And yeah, and that person is not me to clarify. sweat_smile But happy to peek in the meantime.
It looks like the selectors are already added on deployment via _helpers.tpl, so I think the only thing that needs to be added is the
matchLabels(to the deployments). On that note, can this also be done w/in the helper file (e.g.,)?Let me know if I'm off-base here.
trunk kubectl get pod --selector="app.kubernetes.io/instance=selenium-grid" NAME READY STATUS RESTARTS AGE selenium-chrome-node-7cc9c98d4-kwwr9 1/1 Running 0 112s selenium-distributor-5d658b676f-jcxpb 1/1 Running 0 112s selenium-edge-node-85d7b66684-pk6zg 1/1 Running 0 112s selenium-event-bus-586fdf9895-9xg2j 1/1 Running 0 112s selenium-firefox-node-7745686c76-vdsv8 1/1 Running 0 112s selenium-router-6578c54658-9dxnj 1/1 Running 0 112s selenium-session-map-7d4cf44687-86gfj 1/1 Running 0 112s selenium-session-queue-769fd547f8-76tx6 1/1 Running 0 112s trunk kubectl get svc --selector="app.kubernetes.io/instance=selenium-grid" NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE selenium-chrome-node ClusterIP 10.110.17.193 <none> 6900/TCP 2m48s selenium-distributor ClusterIP 10.98.116.20 <none> 5553/TCP 2m48s selenium-edge-node ClusterIP 10.110.216.43 <none> 6900/TCP 2m48s selenium-event-bus ClusterIP 10.96.60.2 <none> 5557/TCP,4442/TCP,4443/TCP 2m48s selenium-firefox-node ClusterIP 10.99.133.250 <none> 6900/TCP 2m48s selenium-router ClusterIP 10.108.187.70 <none> 4444/TCP 2m48s selenium-session-map ClusterIP 10.109.82.130 <none> 5556/TCP 2m48s selenium-session-queue ClusterIP 10.98.92.121 <none> 5559/TCP 2m48s trunk kubectl get deployments --selector="app.kubernetes.io/instance=selenium-grid" NAME READY UP-TO-DATE AVAILABLE AGE selenium-chrome-node 1/1 1 1 2m57s selenium-distributor 1/1 1 1 2m57s selenium-edge-node 1/1 1 1 2m57s selenium-event-bus 1/1 1 1 2m57s selenium-firefox-node 1/1 1 1 2m57s selenium-router 1/1 1 1 2m57s selenium-session-map 1/1 1 1 2m57s selenium-session-queue 1/1 1 1 2m57s
helm install selenium-grid docker-selenium/selenium-grid --set isolateComponents=true --debug --dry-run:.... # Source: selenium-grid/templates/chrome-node-service.yaml apiVersion: v1 kind: Service metadata: name: selenium-chrome-node namespace: default labels: name: selenium-chrome-node app.kubernetes.io/managed-by: helm app.kubernetes.io/instance: selenium-grid app.kubernetes.io/version: 4.4.0-20220831 app.kubernetes.io/component: selenium-grid-4.4.0-20220831 helm.sh/chart: selenium-grid-0.10.0 .... # Source: selenium-grid/templates/chrome-node-deployment.yaml apiVersion: apps/v1 kind: Deployment metadata: name: selenium-chrome-node namespace: default labels: &chrome_node_labels app: selenium-chrome-node app.kubernetes.io/name: selenium-chrome-node app.kubernetes.io/managed-by: helm app.kubernetes.io/instance: selenium-grid app.kubernetes.io/version: 4.4.0-20220831 app.kubernetes.io/component: selenium-grid-4.4.0-20220831 helm.sh/chart: selenium-grid-0.10.0 ....
Sorry for the delay. And yeah, and that person is not me to clarify. sweat_smile But happy to peek in the meantime.
It looks like the selectors are already added on deployment via _helpers.tpl, so I think the only thing that needs to be added is the
matchLabels(to the deployments). On that note, can this also be done w/in the helper file (e.g.,)?Let me know if I'm off-base here.
trunk kubectl get pod --selector="app.kubernetes.io/instance=selenium-grid" NAME READY STATUS RESTARTS AGE selenium-chrome-node-7cc9c98d4-kwwr9 1/1 Running 0 112s selenium-distributor-5d658b676f-jcxpb 1/1 Running 0 112s selenium-edge-node-85d7b66684-pk6zg 1/1 Running 0 112s selenium-event-bus-586fdf9895-9xg2j 1/1 Running 0 112s selenium-firefox-node-7745686c76-vdsv8 1/1 Running 0 112s selenium-router-6578c54658-9dxnj 1/1 Running 0 112s selenium-session-map-7d4cf44687-86gfj 1/1 Running 0 112s selenium-session-queue-769fd547f8-76tx6 1/1 Running 0 112s trunk kubectl get svc --selector="app.kubernetes.io/instance=selenium-grid" NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE selenium-chrome-node ClusterIP 10.110.17.193 <none> 6900/TCP 2m48s selenium-distributor ClusterIP 10.98.116.20 <none> 5553/TCP 2m48s selenium-edge-node ClusterIP 10.110.216.43 <none> 6900/TCP 2m48s selenium-event-bus ClusterIP 10.96.60.2 <none> 5557/TCP,4442/TCP,4443/TCP 2m48s selenium-firefox-node ClusterIP 10.99.133.250 <none> 6900/TCP 2m48s selenium-router ClusterIP 10.108.187.70 <none> 4444/TCP 2m48s selenium-session-map ClusterIP 10.109.82.130 <none> 5556/TCP 2m48s selenium-session-queue ClusterIP 10.98.92.121 <none> 5559/TCP 2m48s trunk kubectl get deployments --selector="app.kubernetes.io/instance=selenium-grid" NAME READY UP-TO-DATE AVAILABLE AGE selenium-chrome-node 1/1 1 1 2m57s selenium-distributor 1/1 1 1 2m57s selenium-edge-node 1/1 1 1 2m57s selenium-event-bus 1/1 1 1 2m57s selenium-firefox-node 1/1 1 1 2m57s selenium-router 1/1 1 1 2m57s selenium-session-map 1/1 1 1 2m57s selenium-session-queue 1/1 1 1 2m57s
helm install selenium-grid docker-selenium/selenium-grid --set isolateComponents=true --debug --dry-run:.... # Source: selenium-grid/templates/chrome-node-service.yaml apiVersion: v1 kind: Service metadata: name: selenium-chrome-node namespace: default labels: name: selenium-chrome-node app.kubernetes.io/managed-by: helm app.kubernetes.io/instance: selenium-grid app.kubernetes.io/version: 4.4.0-20220831 app.kubernetes.io/component: selenium-grid-4.4.0-20220831 helm.sh/chart: selenium-grid-0.10.0 .... # Source: selenium-grid/templates/chrome-node-deployment.yaml apiVersion: apps/v1 kind: Deployment metadata: name: selenium-chrome-node namespace: default labels: &chrome_node_labels app: selenium-chrome-node app.kubernetes.io/name: selenium-chrome-node app.kubernetes.io/managed-by: helm app.kubernetes.io/instance: selenium-grid app.kubernetes.io/version: 4.4.0-20220831 app.kubernetes.io/component: selenium-grid-4.4.0-20220831 helm.sh/chart: selenium-grid-0.10.0 ....
@anthonybaldwin You're right, the labels are added to the resources correctly. The issue is that the selectors only check for e.g.
...
spec:
selector:
app: selenium-hub
See here: https://github.com/SeleniumHQ/docker-selenium/blob/trunk/charts/selenium-grid/templates/hub-service.yaml#L18
Although the labels are correctly set on the resources, the selectors do not look for all labels set. It might even make sense to select for all labels.
Hence, for example the hub service will look for pods by selecting via app: selenium-hub. If you now have two Selenium Grids running, the deployments will have these labels:
Selenium Grid 1
# Source: selenium-grid/templates/chrome-node-service.yaml
....
spec:
type: {{ .Values.chromeNode.service.type }}
selector:
app: selenium-chrome-node
# Source: selenium-grid/templates/chrome-node-deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: selenium-chrome-node-team-a # changed via nameOverride
namespace: default
labels: &chrome_node_labels
app: selenium-chrome-node # <- Only this will be of interest for the service
app.kubernetes.io/name: selenium-chrome-node
app.kubernetes.io/managed-by: helm
app.kubernetes.io/instance: selenium-grid-team-a
app.kubernetes.io/version: 4.4.0-20220831
app.kubernetes.io/component: selenium-grid-4.4.0-20220831
helm.sh/chart: selenium-grid-0.10.0
Selenium Grid 2
# Source: selenium-grid/templates/chrome-node-service.yaml
....
spec:
type: {{ .Values.chromeNode.service.type }}
selector:
app: selenium-chrome-node
# Source: selenium-grid/templates/chrome-node-deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
name: selenium-chrome-node-team-b # changed via nameOverride
namespace: default
labels: &chrome_node_labels
app: selenium-chrome-node # <- Only this will be of interest for the service
app.kubernetes.io/name: selenium-chrome-node
app.kubernetes.io/managed-by: helm
app.kubernetes.io/instance: selenium-grid-team-b
app.kubernetes.io/version: 4.4.0-20220831
app.kubernetes.io/component: selenium-grid-4.4.0-20220831
helm.sh/chart: selenium-grid-0.10.0
Hence, the services will each route to both deployments, because the selector only checks for the hardcoded label app: selenium-chrome-node. It makes sense to keep that label as is, because the app label should actually indicate the real application name.
You can try two deployments by overriding the names of all components and then using the service to connect to them. The service will route to all pods instead of the pods specific to the release.
Hi @anthonybaldwin thanks for jumping in and providing some feedback.
@Xcalizorz Thanks for working on this. Please note we're looking for another reviewer as well. It may take a little longer to get reviewed, but just know we're acknowledging this pull request and will get to it at some point in the near future.