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

CA-Chart - Mark Cluster-Autoscaler-Chart as Deprecated #3719

Conversation

gjtempleton
Copy link
Member

@gjtempleton gjtempleton commented Nov 25, 2020

There will be a follow-up PR to remove these files as only one release marked as deprecated is required.

This diff looks very big, however this is due to copying back in the files for the cluster-autoscaler-chart Helm chart as it was pre the renaming to cluster-autoscaler performed by #3679 .

The actual changes performed by this PR to the last previous release of the cluster-autoscaler-chart chart marked by https://github.com/kubernetes/autoscaler/tree/cluster-autoscaler-chart-1.1.1 can be seen by running

git diff cluster-autoscaler-chart-1.1.1 charts/cluster-autoscaler-chart/

diff --git a/charts/cluster-autoscaler-chart/Chart.yaml b/charts/cluster-autoscaler-chart/Chart.yaml
index 4f86dcc4d..b288ae4a8 100644
--- a/charts/cluster-autoscaler-chart/Chart.yaml
+++ b/charts/cluster-autoscaler-chart/Chart.yaml
@@ -17,4 +17,5 @@ name: cluster-autoscaler-chart
 sources:
   - https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler
 type: application
-version: 1.1.1
+version: 2.0.0
+deprecated: true
diff --git a/charts/cluster-autoscaler-chart/README.md b/charts/cluster-autoscaler-chart/README.md
...

The changes to the readme are expected as pre-commit hooks are now enforced and have regenerated the readme from the template.

Performs a major version bump of cluster-autoscaler-chart to mark it as deprecated
in favour of cluster-autoscaler chart

This is required to hide the deprecated old version of the chart from the artifact hub UI, as currently users are shown both the old and new charts: https://artifacthub.io/packages/search?page=1&ts_query_web=cluster-autoscaler

Marking the chart as deprecated in the Chart.yaml will hide all releases of the old chart version from the UI. Performs a major version bump of the old version of the chart to 2.0.0 to mark the deprecation.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 25, 2020
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 25, 2020
@gjtempleton gjtempleton changed the title CA-Chart - Mark Cluster-Autoscaler-Chart as Deprecated [WIP] - CA-Chart - Mark Cluster-Autoscaler-Chart as Deprecated Nov 25, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2020
@mwielgus
Copy link
Contributor

Is this still needed/worked on?

Perform major version bump of cluster-autoscaler-chart to mark it as deprecated
in favour of cluster-autoscaler chart

Make only change version number and deprecation notice
@gjtempleton gjtempleton force-pushed the cluster-autoscaler-chart-Chart-Deprecate-Final-Version branch from 0d9ef68 to cf4e5db Compare January 26, 2021 22:41
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 26, 2021
@gjtempleton gjtempleton changed the title [WIP] - CA-Chart - Mark Cluster-Autoscaler-Chart as Deprecated CA-Chart - Mark Cluster-Autoscaler-Chart as Deprecated Jan 26, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 26, 2021
@gjtempleton
Copy link
Member Author

Yep, had some discussions and the preference is for the option that makes this PR look rather messy unfortunately. I've updated the PR description to include instructions for seeing the changes from the last release of the chart as it used to be named here, the only meaningful changes being to bump the major version and mark the chart as deprecated.

@mwielgus
Copy link
Contributor

/retest

@mwielgus mwielgus closed this Jan 27, 2021
@mwielgus mwielgus reopened this Jan 27, 2021
Copy link
Contributor

@mwielgus mwielgus left a comment

Choose a reason for hiding this comment

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

/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 Feb 12, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 merged commit 962bbd0 into kubernetes:master Feb 12, 2021
@gjtempleton gjtempleton deleted the cluster-autoscaler-chart-Chart-Deprecate-Final-Version branch February 12, 2021 19:57
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants