Add helm charts for lws. #266

wqlparallel · 2024-11-24T06:34:14Z

What type of PR is this?

/kind feature

What this PR does / why we need it

Added helm charts for lws.

Which issue(s) this PR fixes

Fixes #260

Special notes for your reviewer

Does this PR introduce a user-facing change?

k8s-ci-robot · 2024-11-24T06:34:24Z

Hi @wqlparallel. 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.

kannon92 · 2024-11-24T20:54:34Z

/ok-to-test

kannon92 · 2024-11-25T01:45:07Z

charts/lws/README.md

+##### Install chart using Helm v3.0+
+
+```bash
+$ git clone [email protected]:kubernetes-sigs/kueue.git


Suggested change

$ git clone [email protected]:kubernetes-sigs/kueue.git

$ git clone [email protected]:kubernetes-sigs/lws.git

kannon92 · 2024-11-25T01:45:37Z

charts/lws/README.md

+##### Verify that controller pods are running properly.
+
+```bash
+$ kubectl get deploy -n kueue-system


Suggested change

$ kubectl get deploy -n kueue-system

$ kubectl get deploy lws-system

sorry，I referenced the queue project during development because the helm charts for queue and lws are quite similar. I've corrected the submission now. The helm chart has been tested and appears to be functioning correctly.

kerthcet · 2024-11-27T16:18:17Z

/hold

Put my concern here: #260 (comment)

ahg-g

can we add a link to the helm-based installation instructions in https://github.com/kubernetes-sigs/lws/blob/main/docs/setup/install.md

ahg-g · 2024-12-10T04:29:40Z

charts/lws/Chart.yaml

+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "v0.4.2"


is this the only place that we need to update every time we make a release?

I think this refers to the lws version, but we need to update the release version as well.

In each release, we should update at least two fields: appVersion and version . We typically update appVersion when we're releasing a new version of LWS. But, always update the version field with any change to the Helm chart. If there are no other changes to the Helm chart-liske adding more parameters or configuration-then you'll only need to focus on those two.

please add release instructions, and I will make them part of the release template when this gets merged

ahg-g · 2024-12-10T04:30:35Z

charts/lws/README.md

+$ git clone [email protected]:kubernetes-sigs/lws.git
+$ cd charts
+$ helm install lws lws --create-namespace --namespace lws-system
+```


how do we install this from a release package? can we add instructions to making the helm charts an artifact of the release?

Maybe we can store the helm packages in github with gh-pages. But that's the next step.

Installing lws from a release package sounds good, and I agree that it's a step we consider next.

Install in a different namespace is another thing I'm concerned, but that can be follow up together.

can you please open issues to track the follow up items?

ticketed here #289

kerthcet

Hi @wqlparallel could you solve the comments? And does this support install lws in a different namespace rather than lws-system?

kerthcet · 2024-12-13T05:59:39Z

charts/lws/README.md

+
+#### Prerequisites
+
+- [Helm](https://helm.sh/docs/intro/quickstart/#install-helm)


Add (Optional) [Cert-manager](https://cert-manager.io/docs/installation/) as well.

Thank you for your comment! I believe that Cert-manager should not be listed as a prerequisite for the Helm chart since it can be installed optionally if the user sets enableCertManager: true in values.yaml. In this context, prerequisites should only include Helm itself, while Cert-manager is treated as an application dependency that users can choose to enable.

That's why it's optional

kerthcet · 2024-12-13T06:02:50Z

charts/lws/Chart.yaml

+# incremented each time you make changes to the application. Versions are not expected to
+# follow Semantic Versioning. They should reflect the version the application is using.
+# It is recommended to use it with quotes.
+appVersion: "v0.4.2"


I think this refers to the lws version, but we need to update the release version as well.

kerthcet · 2024-12-13T06:13:01Z

charts/lws/README.md

+$ git clone [email protected]:kubernetes-sigs/lws.git
+$ cd charts
+$ helm install lws lws --create-namespace --namespace lws-system
+```


Maybe we can store the helm packages in github with gh-pages. But that's the next step.

kerthcet · 2024-12-13T06:14:17Z

/hold cancel

wqlparallel · 2024-12-16T06:20:39Z

Hi @wqlparallel could you solve the comments? And does this support install lws in a different namespace rather than lws-system?

I have committed the code to address the comments. And this Helm chart supports installing the lws in a different namespace when referencing the main image. However, it may not work as expected in the latest release (v0.4.2) because the commit that deployed in the specified namespace hasn’t been merged yet.

kerthcet · 2024-12-17T06:43:09Z

Please squash, I'm ok with the PR. Leave to @ahg-g for final approval.

Signed-off-by: wqlparallel <[email protected]>

ahg-g · 2024-12-17T07:42:23Z

/label tide/merge-method-squash

ahg-g · 2024-12-17T19:13:07Z

/lgtm
/approve

k8s-ci-robot · 2024-12-17T19:13:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, wqlparallel

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

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 added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 24, 2024

k8s-ci-robot requested review from ahg-g and liurupeng November 24, 2024 06:34

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 24, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 24, 2024

kannon92 reviewed Nov 25, 2024

View reviewed changes

wqlparallel force-pushed the helm branch 3 times, most recently from 2b986bd to d2bcf84 Compare November 25, 2024 01:56

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2024

ahg-g reviewed Dec 10, 2024

View reviewed changes

kerthcet reviewed Dec 13, 2024

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2024

ahg-g mentioned this pull request Dec 16, 2024

Helm chart for JobSet kubernetes-sigs/jobset#726

Open

3 tasks

Add helm charts for lws.

b34b2ce

Signed-off-by: wqlparallel <[email protected]>

wqlparallel force-pushed the helm branch from d20fe40 to b34b2ce Compare December 17, 2024 06:53

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 17, 2024

k8s-ci-robot assigned ahg-g Dec 17, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 17, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2024

k8s-ci-robot merged commit a1ea3ef into kubernetes-sigs:main Dec 17, 2024
8 checks passed

kerthcet mentioned this pull request Dec 18, 2024

helm install with assigned version or namespace #289

Open

3 tasks

kerthcet mentioned this pull request Jan 21, 2025

Webhook certs generation error #318

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add helm charts for lws. #266

Add helm charts for lws. #266

wqlparallel commented Nov 24, 2024

k8s-ci-robot commented Nov 24, 2024

kannon92 commented Nov 24, 2024

kannon92 Nov 25, 2024

kannon92 Nov 25, 2024

wqlparallel Nov 25, 2024

kerthcet commented Nov 27, 2024

ahg-g left a comment

ahg-g Dec 10, 2024

kerthcet Dec 13, 2024

wqlparallel Dec 16, 2024

ahg-g Dec 16, 2024

ahg-g Dec 10, 2024

kerthcet Dec 13, 2024

wqlparallel Dec 16, 2024

kerthcet Dec 17, 2024

ahg-g Dec 17, 2024

kerthcet Dec 18, 2024 •

edited

Loading

kerthcet left a comment

kerthcet Dec 13, 2024

wqlparallel Dec 16, 2024

kerthcet Dec 16, 2024

kerthcet Dec 13, 2024

kerthcet Dec 13, 2024

kerthcet commented Dec 13, 2024

wqlparallel commented Dec 16, 2024

kerthcet commented Dec 17, 2024

ahg-g commented Dec 17, 2024

ahg-g commented Dec 17, 2024

k8s-ci-robot commented Dec 17, 2024

	$ git clone [email protected]:kubernetes-sigs/kueue.git
	$ git clone [email protected]:kubernetes-sigs/lws.git

	$ kubectl get deploy -n kueue-system
	$ kubectl get deploy lws-system


		#### Prerequisites

		- [Helm](https://helm.sh/docs/intro/quickstart/#install-helm)

Add helm charts for lws. #266

Add helm charts for lws. #266

Conversation

wqlparallel commented Nov 24, 2024

What type of PR is this?

What this PR does / why we need it

Which issue(s) this PR fixes

Special notes for your reviewer

Does this PR introduce a user-facing change?

k8s-ci-robot commented Nov 24, 2024

kannon92 commented Nov 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kerthcet commented Nov 27, 2024

ahg-g left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kerthcet Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

kerthcet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kerthcet commented Dec 13, 2024

wqlparallel commented Dec 16, 2024

kerthcet commented Dec 17, 2024

ahg-g commented Dec 17, 2024

ahg-g commented Dec 17, 2024

k8s-ci-robot commented Dec 17, 2024

kerthcet Dec 18, 2024 •

edited

Loading