kueue icon indicating copy to clipboard operation
kueue copied to clipboard

Update Helm Template to support custom ingress host

Open btwseeu78 opened this issue 5 months ago • 15 comments

/kind bug /kind cleanup /kind feature

What this PR does / why we need it:

  1. Current helm chart has no way to configure host for ingress in kueueviz it ends up referring a random TLS-Secret(bug)
  2. PriorityClass only added to manager not extended to kueueviz , frontend or backend deployments

Fixes #

  • https://github.com/kubernetes-sigs/kueue/issues/5437

Does this PR introduce a user-facing change?

1. Add Support for configuring host for ingress in kueueviz.
2. changing PriorityClassName support extended to kueueviz  frontend or backend deployments.
3. Added test cases for kueueviz components in helm unit-test 

btwseeu78 avatar Jun 24 '25 17:06 btwseeu78

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: btwseeu78 / name: Arpan Chatterjee (d8ae2da89b808b3a015d08a447b5b13b0aad5bbc, 90958ef0a0c058f0075bdfccaf0c57b658e2abed, f0026efe91dbbcc78fc4288e434f8cdb3a935919)

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: btwseeu78 Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Jun 24 '25 17:06 k8s-ci-robot

Welcome @btwseeu78!

It looks like this is your first PR to kubernetes-sigs/kueue 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kueue has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

k8s-ci-robot avatar Jun 24 '25 17:06 k8s-ci-robot

Hi @btwseeu78. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot avatar Jun 24 '25 17:06 k8s-ci-robot

Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
Latest commit f0026efe91dbbcc78fc4288e434f8cdb3a935919
Latest deploy log https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6862301602d41400080d3077

netlify[bot] avatar Jun 24 '25 17:06 netlify[bot]

@btwseeu78 please sign CLA

mimowo avatar Jun 24 '25 17:06 mimowo

/ok-to-test

mimowo avatar Jun 24 '25 17:06 mimowo

@btwseeu78 please also fix the release note to remove the do-not-merge/release-note-label-needed label

mimowo avatar Jun 24 '25 17:06 mimowo

/remove-kind cleanup /remove-kind bug I think this is actually a feature, but which can be cherry-picked. We did it in the past, see example

mimowo avatar Jun 24 '25 17:06 mimowo

/retest-required

btwseeu78 avatar Jun 24 '25 19:06 btwseeu78

❯ make helm-unit-test
go: helm.sh/helm/[email protected] requires go >= 1.24.0; switching to go1.24.4
/Users/arpan-k8s/GolandProjects/kueue/bin/helm unittest charts/kueue --strict --debug

### Chart [ kueue ] charts/kueue

 PASS  test kueueviz deployment charts/kueue/tests/kueueviz_test.yaml
 PASS  test manager deployment  charts/kueue/tests/manager_test.yaml

Charts:      1 passed, 1 total
Test Suites: 2 passed, 2 total
Tests:       10 passed, 10 total
Snapshot:    0 passed, 0 total
Time:        48.912042ms

not very sure why prow test failed,some guide will be appreciated

btwseeu78 avatar Jun 24 '25 19:06 btwseeu78

❯ make helm-unit-test
go: helm.sh/helm/[email protected] requires go >= 1.24.0; switching to go1.24.4
/Users/arpan-k8s/GolandProjects/kueue/bin/helm unittest charts/kueue --strict --debug

### Chart [ kueue ] charts/kueue

 PASS  test kueueviz deployment charts/kueue/tests/kueueviz_test.yaml
 PASS  test manager deployment  charts/kueue/tests/manager_test.yaml

Charts:      1 passed, 1 total
Test Suites: 2 passed, 2 total
Tests:       10 passed, 10 total
Snapshot:    0 passed, 0 total
Time:        48.912042ms

not very sure why prow test failed,some guide will be appreciated

I've not dug much into it but I can reproduce your failure if I run make verify.

kannon92 avatar Jun 24 '25 20:06 kannon92

/test pull-kueue-verify-main

btwseeu78 avatar Jun 24 '25 20:06 btwseeu78

@kannon92 The problem is on step update-helm in makefile , its regenerates all the manifests which is overwriting this changes.so there is no way to update helm than generating it,i will check the modifications,it was a nice learn :)

btwseeu78 avatar Jun 24 '25 21:06 btwseeu78

/retest-required

btwseeu78 avatar Jun 25 '25 09:06 btwseeu78