fix(helm): add clusterIP to values.yaml #6657

eshiettjoseph · 2024-03-26T01:03:41Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Supports service.clusterIP template for autoscaler chart.

Which issue(s) this PR fixes:

Fixes #6631

linux-foundation-easycla · 2024-03-26T01:03:45Z

The committers listed above are authorized under a signed CLA.

✅ login: eshiettjoseph / name: Joseph Eshiett (af664a2, b8ca18d, 31ff8fa, 21e0c0b)

k8s-ci-robot · 2024-03-26T01:03:50Z

Welcome @eshiettjoseph!

It looks like this is your first PR to kubernetes/autoscaler 🎉. 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/autoscaler 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. 😃

k8s-ci-robot · 2024-03-26T01:03:50Z

Hi @eshiettjoseph. Thanks for your PR.

I'm waiting for a kubernetes 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/test-infra repository.

k8s-ci-robot · 2024-03-26T01:04:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: eshiettjoseph
Once this PR has been reviewed and has the lgtm label, please assign gjtempleton for approval. For more information see the Kubernetes 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:

charts/OWNERS

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

Shubham82 · 2024-03-26T07:22:32Z

Hi @eshiettjoseph
Please address the failed Test Cases i.e. Lint and Test Charts / Helm chart (pull_request) and Lint and Test Charts / Helm Docs (pull_request), that are required to be passed.

JFYI: you have to install Pre-commit hooks before working in the Charts.

Shubham82 · 2024-03-26T07:24:26Z

charts/cluster-autoscaler/values.yaml

@@ -349,6 +349,8 @@ service:
  # service.externalIPs -- List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/user-guide/services/#external-ips.


Suggested change

# service.externalIPs -- List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/user-guide/services/#external-ips.

# service.externalIPs -- List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/concepts/services-networking/service/#external-ips.

Also, update the above outdated link.

Shubham82 · 2024-03-27T07:39:18Z

charts/cluster-autoscaler/Chart.yaml

@@ -11,4 +11,4 @@ name: cluster-autoscaler
 sources:
  - https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler
 type: application
-version: 9.36.0
+version: 9.37.0


Suggested change

version: 9.37.0

version: 9.36.1

As we are resolving a bug not adding new functionality it should be a Patch version, not a Minor version

Shubham82 · 2024-03-27T11:57:04Z

/ok-to-test

Shubham82 · 2024-03-27T11:57:38Z

/lgtm

Shubham82 · 2024-04-30T05:40:44Z

Hi @gjtempleton
Please approve this PR so that it will merge.

Thanks!

k8s-ci-robot · 2024-05-09T01:45:35Z

PR needs rebase.

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.

mastersingh24 · 2024-08-23T10:36:44Z

charts/cluster-autoscaler/values.yaml

  externalIPs: []

+  # service.clusterIP -- IP address to assign to service
+  clusterIP: ""


I know the original issue noted this was missing, but is there a reason you would actually set clusterIP? I would think that we should actually remove it from the template itself?

jackfrancis · 2024-09-27T22:03:53Z

See this comment here: #6631 (comment)

The main thing we want to do here is to add clusterIP to the values.yaml file, and to run helm-docs command line tool to regenerate documentation.

If there are links in the comments that need to be updated for other properties that should be a discrete PR. Happy to review that quickly, if so.

IMO the changes here shouldn't need a helm chart version bump, but we'll see what the linter says. :)

jackfrancis · 2024-09-27T22:06:28Z

/lgtm cancel

/remove-kind/bug

jackfrancis · 2024-09-27T22:07:52Z

/label kind-bug

k8s-ci-robot · 2024-09-27T22:07:54Z

@jackfrancis: The label(s) /label kind-bug cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label kind-bug

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.

jackfrancis · 2024-09-27T22:08:24Z

/label kind/bug

k8s-ci-robot · 2024-09-27T22:08:26Z

@jackfrancis: The label(s) /label kind/bug cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label kind/bug

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.

Shubham82 · 2024-09-30T12:30:09Z

If there are links in the comments that need to be updated for other properties that should be a discrete PR. Happy to review that quickly, if so.

Hi @jackfrancis, we also have to update the link, it will be good if we can do it in this PR only but I'm also OK with another PR for it.

Shubham82 · 2024-09-30T12:33:52Z

/remove-kind bug

Shubham82 · 2024-09-30T12:35:37Z

/kind cleanup
(as per the comment)

jackfrancis · 2024-09-30T21:35:28Z

charts/cluster-autoscaler/Chart.yaml

@@ -11,4 +11,4 @@ name: cluster-autoscaler
 sources:
  - https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler
 type: application
-version: 9.36.0
+version: 9.36.1


IMO we don't need to rev the chart version, as we aren't adding or removing or changing any functionality. We can drop this change.

as per the comment, we need to change the chart version.

jackfrancis · 2024-09-30T21:36:21Z

charts/cluster-autoscaler/README.md

 | service.create | bool | `true` | If `true`, a Service will be created. |
-| service.externalIPs | list | `[]` | List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/user-guide/services/#external-ips. |
+| service.externalIPs | list | `[]` | List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/concepts/services-networking/service/#external-ips. |


Yes I know it's more work but let's drop these link fixes and submit a separate PR to address them.

ok @jackfrancis, I will open a seperate PR for this one.

Hi @eshiettjoseph, please update the PR.

I have raised a PR #7317 to fix the link.

Shubham82 · 2024-10-14T13:16:47Z

Hi @eshiettjoseph, Is there any update on this PR?

jackfrancis · 2024-10-21T22:08:07Z

@eshiettjoseph @Shubham82 this PR is next in the queue for merging, so if you rebase on top of master and rev the chart version again it should be the last time you have to do that!

Shubham82 · 2024-10-22T06:09:23Z

@eshiettjoseph, are you working on this PR, if Yes then could you please resolve the merge conflicts so that it will be merged?

Thanks!

Shubham82 · 2024-10-30T12:49:57Z

Hi @jackfrancis, no response from @eshiettjoseph till now, should we open a new PR for it and close this one, or should we wait more?
I just want issue #6631 to be closed, as it has been open for a long time.

Shubham82 · 2024-10-30T12:51:02Z

cc @gjtempleton @jackfrancis

If you are ok, then I will open a new PR for it.

jackfrancis · 2024-10-30T16:28:01Z

@Shubham82 I'll happily lgtm an equivalent version of this change in a new PR!

Shubham82 · 2024-11-01T10:13:31Z

@Shubham82 I'll happily lgtm an equivalent version of this change in a new PR!

Thanks @jackfrancis for the confirmation, I will open a new PR for this fix.

Shubham82 · 2024-11-01T10:39:35Z

I have raised a new PR #7452 to fix this.

Shubham82 · 2024-11-01T10:42:46Z

Close this PR once PR #7452 is merged.

Shubham82 · 2024-11-04T17:03:00Z

closing this PR, as PR #7452 is merged now.

Shubham82 · 2024-11-04T17:03:07Z

/close

k8s-ci-robot · 2024-11-04T17:03:12Z

@Shubham82: Closed this PR.

In response to this:

/close

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.

fix(helm): add clusterIP to values.yaml

af664a2

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 26, 2024

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 26, 2024

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 26, 2024

k8s-ci-robot added the area/helm-charts label Mar 26, 2024

k8s-ci-robot requested review from gjtempleton and MaciekPytel March 26, 2024 01:04

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 26, 2024

Shubham82 suggested changes Mar 26, 2024

View reviewed changes

eshiettjoseph added 2 commits March 26, 2024 14:24

chore(helm): update kubernetes docs url for external-ips

b8ca18d

chore(helm): bump Chart version

21e0c0b

eshiettjoseph requested a review from Shubham82 March 26, 2024 13:47

Shubham82 suggested changes Mar 27, 2024

View reviewed changes

chore: update patch version

31ff8fa

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 Mar 27, 2024

k8s-ci-robot assigned Shubham82 Mar 27, 2024

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

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2024

mastersingh24 reviewed Aug 23, 2024

View reviewed changes

k8s-ci-robot assigned jackfrancis Sep 27, 2024

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 27, 2024

k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Sep 30, 2024

k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Sep 30, 2024

jackfrancis reviewed Sep 30, 2024

View reviewed changes

k8s-ci-robot closed this Nov 4, 2024

		@@ -349,6 +349,8 @@ service:
		# service.externalIPs -- List of IP addresses at which the service is available. Ref: https://kubernetes.io/docs/user-guide/services/#external-ips.

fix(helm): add clusterIP to values.yaml #6657

fix(helm): add clusterIP to values.yaml #6657

Conversation

eshiettjoseph commented Mar 26, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

linux-foundation-easycla bot commented Mar 26, 2024 • edited Loading

k8s-ci-robot commented Mar 26, 2024

k8s-ci-robot commented Mar 26, 2024

k8s-ci-robot commented Mar 26, 2024

Shubham82 commented Mar 26, 2024

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

Shubham82 commented Mar 27, 2024

Shubham82 commented Mar 27, 2024

Shubham82 commented Apr 30, 2024

k8s-ci-robot commented May 9, 2024

Choose a reason for hiding this comment

jackfrancis commented Sep 27, 2024

jackfrancis commented Sep 27, 2024

jackfrancis commented Sep 27, 2024

k8s-ci-robot commented Sep 27, 2024

jackfrancis commented Sep 27, 2024

k8s-ci-robot commented Sep 27, 2024

Shubham82 commented Sep 30, 2024

Shubham82 commented Sep 30, 2024

Shubham82 commented Sep 30, 2024 • edited Loading

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

Shubham82 commented Oct 14, 2024

jackfrancis commented Oct 21, 2024

Shubham82 commented Oct 22, 2024

Shubham82 commented Oct 30, 2024

Shubham82 commented Oct 30, 2024

jackfrancis commented Oct 30, 2024

Shubham82 commented Nov 1, 2024

Shubham82 commented Nov 1, 2024

Shubham82 commented Nov 1, 2024

Shubham82 commented Nov 4, 2024

Shubham82 commented Nov 4, 2024

k8s-ci-robot commented Nov 4, 2024

linux-foundation-easycla bot commented Mar 26, 2024 •

edited

Loading

Shubham82 commented Sep 30, 2024 •

edited

Loading