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

fix(chart): Error setting name in helm release #2006 #2007 #2009

Merged
merged 10 commits into from
Nov 14, 2023

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Nov 13, 2023

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

Fix: Error setting name in helm release #2006 & #2007

Motivation and Context

After PR #1989, template .fullName was updated in podTemplate. However, it caused a mismatch between selector and the template labels, which caused the helm command gets failed when component set .nameOverride
In part of this PR also contains:

  • Add workflow to test chart, the first test is used to cover this use case
    • Workflow trigger events on push, pull request with paths 'charts/selenium-grid/**'
    • Chart install with multiple values can be appended later under charts/selenium-grid/ci/*-values.yaml. This folder is excluded from package chart production via .helmignore to avoid confusion from the user.
    • Config for ct action is located user tests/chart-test.yaml

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.

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 marked this pull request as draft November 13, 2023 20:35
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 marked this pull request as ready for review November 13, 2023 22:44
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 changed the title Bug: Error setting name in helm release #2006 Bug: Error setting name in helm release #2006 #2007 Nov 14, 2023
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96
Copy link
Member Author

It looks like helm/kind-action has a limitation on computing, the workflow helm-chart-test is unstable sometimes due to 0/1 nodes are available: 1 Insufficient cpu(actions/runner-images#7107) when bringing up basic setup 1 hub & 3 nodes together.
In order to make the workflow stable, we will break down and test 1 hub : 1 node at a time, and repeat 3 times for 3 types of browser nodes by creating 3 separate values files under ci/*values.yaml
Changeset 356754d

@diemol
Copy link
Member

diemol commented Nov 14, 2023

@VietND96 can you please solve the merge conflicts?

# Conflicts:
#	.github/workflows/helm-chart-test.yml
#	charts/selenium-grid/Chart.yaml
@VietND96
Copy link
Member Author

@diemol, I resolved conflicts. Can you approve workflow runs to check?
In my change has set check-version-increment: false, I think it would resolve failure in another PR.

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Thank you, @VietND96!

@diemol
Copy link
Member

diemol commented Nov 14, 2023

charts/selenium-grid/ci/overrideNameChrome-values.yaml
Error: 1:2 [comments] missing starting space in comment

@VietND96
Copy link
Member Author

charts/selenium-grid/ci/overrideNameChrome-values.yaml
Error: 1:2 [comments] missing starting space in comment

Ohh, Lint issue under testing values :( I am fixing it now

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@diemol diemol merged commit eec5ecc into SeleniumHQ:trunk Nov 14, 2023
5 checks passed
@VietND96 VietND96 changed the title Bug: Error setting name in helm release #2006 #2007 fix(chart): Error setting name in helm release #2006 #2007 Dec 27, 2023
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.

[🐛 Bug]: Can't get variables from /videos/uploadpipe [🐛 Bug]: Error setting name in helm release
2 participants