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

Modified the VPA content for the helm chart and Bump the CA image to 1.27.2 #5763

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

Shubham82
Copy link
Contributor

What type of PR is this?

/kind bug
/kind documentation

What this PR does / why we need it:

This PR modified the VPA content for the CA under the Charts directory. It will give a better understanding of the usage of VPA for the CA helm charts.

Which issue(s) this PR fixes:

Fixes #5758

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/helm-charts labels May 17, 2023
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 17, 2023
@Shubham82
Copy link
Contributor Author

Hi @gjtempleton, @MaciekPytel
PTAL!

@Shubham82 Shubham82 force-pushed the modify_content_of_vpa branch from 99ee3ee to c34a7a1 Compare May 18, 2023 11:23
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 18, 2023
@Shubham82
Copy link
Contributor Author

Previously few test cases failed because somehow I missed the pre-commit hook things, update the PR according to that.

@Shubham82
Copy link
Contributor Author

Shubham82 commented May 18, 2023

Now only Lint and Test Charts / Helm chart (pull_request) is failing, why is so?

on checking the details of the test case it is showing the following error:

Linting chart 'cluster-autoscaler => (version: "9.28.0", path: "charts/cluster-autoscaler")'
Checking chart 'cluster-autoscaler => (version: "9.28.0", path: "charts/cluster-autoscaler")' for a version bump...
Error: Error linting charts: Error processing charts
Old chart version: 9.28.0
New chart version: 9.28.0
------------------------------------------------------------------------------------------------------------------------
 ✖︎ cluster-autoscaler => (version: "9.28.0", path: "charts/cluster-autoscaler") > Chart version not ok. Needs a version bump!
------------------------------------------------------------------------------------------------------------------------
Error linting charts: Error processing charts
Error: Process completed with exit code 1.

but the latest chart version for CA is already 9.28.0. did I miss something here?

@gjtempleton could you please take a look?

@gjtempleton
Copy link
Member

Old chart version: 9.28.0
New chart version: 9.28.0

is the problem. You need to bump the chart version to 9.28.1 as it can't publish a new chart otherwise.

@Shubham82
Copy link
Contributor Author

Old chart version: 9.28.0
New chart version: 9.28.0

is the problem. You need to bump the chart version to 9.28.1 as it can't publish a new chart otherwise.

but there is no chart version 9.28.1 under the autoscaler/releases.

@Shubham82
Copy link
Contributor Author

Shubham82 commented May 22, 2023

Old chart version: 9.28.0
New chart version: 9.28.0

is the problem. You need to bump the chart version to 9.28.1 as it can't publish a new chart otherwise.

but there is no chart version 9.28.1 under the autoscaler/releases.

@gjtempleton, could you please tell me why we have to change it to 9.28.1

@gjtempleton
Copy link
Member

We use automated tooling to build and publish the releases of the helm chart: https://github.com/kubernetes/autoscaler/blob/master/.github/workflows/release.yaml#L4-L29

It does this based on the chart-version declared in Chart.yaml, so on merging a PR updating the chart and the chart version, it will create the release, and publish the associated tarball, as well as updating the gh-docs branch to allow discovery of the new version of the chart.

@Shubham82
Copy link
Contributor Author

We use automated tooling to build and publish the releases of the helm chart: https://github.com/kubernetes/autoscaler/blob/master/.github/workflows/release.yaml#L4-L29

It does this based on the chart-version declared in Chart.yaml, so on merging a PR updating the chart and the chart version, it will create the release, and publish the associated tarball, as well as updating the gh-docs branch to allow discovery of the new version of the chart.

Thanks for the above info.
I will update it.

@Shubham82
Copy link
Contributor Author

Hi @gjtempleton
As CA 1.27.0 / 1.27.1 has been released so should I update the chart version to 9.29.0 and appVersion to 1.27.1?

@Shubham82 Shubham82 changed the title Modified the VPA content for the helm chart. Modified the VPA content for the helm chart and Bump the CA image to 1.27.1 May 23, 2023
@Shubham82
Copy link
Contributor Author

Hi @gjtempleton
I have updated the PR, could you please take a look?

Thanks

@Shubham82
Copy link
Contributor Author

@gjtempleton
PTAL so that it will merge.

Thanks

charts/cluster-autoscaler/Chart.yaml Outdated Show resolved Hide resolved
charts/cluster-autoscaler/Chart.yaml Outdated Show resolved Hide resolved
charts/cluster-autoscaler/README.md Outdated Show resolved Hide resolved
@Shubham82
Copy link
Contributor Author

Shubham82 commented Jun 13, 2023

Thanks, @gjtempleton, for your above comments.

I will rebase it and bump the patch version.

@Shubham82 Shubham82 force-pushed the modify_content_of_vpa branch from 6acd0f3 to d47a800 Compare June 13, 2023 10:18
@Shubham82 Shubham82 changed the title Modified the VPA content for the helm chart and Bump the CA image to 1.27.1 Modified the VPA content for the helm chart and Bump the CA image to 1.27.2 Jun 13, 2023
@Shubham82
Copy link
Contributor Author

Hi @gjtempleton
PTAL!

@gjtempleton
Copy link
Member

Thanks!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gjtempleton, Shubham82

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit 3c5cc58 into kubernetes:master Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/helm-charts cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cant install VPA with Helm Configuration
3 participants