-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add charts url setting #43530
Merged
Merged
Add charts url setting #43530
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rohitsakala
reviewed
Nov 15, 2023
rohitsakala
requested changes
Nov 15, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented
94f6d2d
to
71fbb41
Compare
71fbb41
to
a9c3245
Compare
rohitsakala
requested changes
Dec 15, 2023
a9c3245
to
5ca23c7
Compare
rohitsakala
previously approved these changes
Dec 15, 2023
7833fa5
to
c076500
Compare
c076500
to
857f757
Compare
857f757
to
d163969
Compare
This adds a new setting, named `CATTLE_CHART_DEFAULT_URL`, to control the origin of system charts, which can now be tested with custom versions prior to releasing corresponding RCs to `rancher/charts`.
This provides new options to replace URLs for those repositories, as done for the system charts repository. All three charts repositories may need to be replaced with custom repos in test/CI setups.
This moves each default branch setting, for the partner and RKE2 charts repositories, next to the default URL settings for the respective repository, to make configuration easier and improve documentation.
This adds line breaks to avoid longer lines, which are harder to read especially in diffs.
This needs to be done if the repo is found, now that the repo URL may differ from the default one.
As dedicated settings are now available for all 3 charts repo types (system, partner and RKE2), this commit adapts the warning message to mention the repository type for which a URL has diverged from the default.
This commit fixes a linting error by running `gofmt`.
d163969
to
77f8482
Compare
rohitsakala
approved these changes
Jan 19, 2024
This was referenced Jun 4, 2024
weyfonk
added a commit
to weyfonk/rancher
that referenced
this pull request
Jun 6, 2024
* Add default chart URL setting This adds new settings to control the origin of system, partner and RKE2 charts, which can now be tested with custom versions prior to releasing corresponding RCs to their respective repositories: * `ChartDefaultURL` (`rancher/charts`) * `PartnerChartDefaultURL` (`rancher/partner- charts`) * `RKE2ChartDefaultURL` (`rancher/rke2- charts`) These options are intended for internal use only. When in use, Rancher issues a warning suggesting to unset them should any issues be encountered when fetching charts. * Group settings by repo type This moves each default branch setting, for the partner and RKE2 charts repositories, next to the default URL settings for the respective repository, to make configuration easier and improve documentation.
weyfonk
added a commit
that referenced
this pull request
Jun 13, 2024
* Add default chart URL setting This adds new settings to control the origin of system, partner and RKE2 charts, which can now be tested with custom versions prior to releasing corresponding RCs to their respective repositories: * `ChartDefaultURL` (`rancher/charts`) * `PartnerChartDefaultURL` (`rancher/partner- charts`) * `RKE2ChartDefaultURL` (`rancher/rke2- charts`) These options are intended for internal use only. When in use, Rancher issues a warning suggesting to unset them should any issues be encountered when fetching charts. * Group settings by repo type This moves each default branch setting, for the partner and RKE2 charts repositories, next to the default URL settings for the respective repository, to make configuration easier and improve documentation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue: #43527
Problem
Testing any system chart against Rancher requires following one of these methods:
rancher/charts
branch to be specified inCATTLE_CHART_DEFAULT_BRANCH
, which would end up pollutingrancher/charts
with test branchesADD
ing that custom repository over the clone in the rancher/rancher imageThere should be an easier way.
Solution
Expose new environment variables and their corresponding settings for their respective charts repositories:
CATTLE_CHART_DEFAULT_URL
ChartDefaultURL
rancher/charts
CATTLE_PARTNER_CHART_DEFAULT_URL
PartnerChartDefaultURL
rancher/partner-charts
CATTLE_RKE2_CHART_DEFAULT_URL
RKE2ChartDefaultURL
rancher/rke2-charts
These options are intended for internal use only. When in use, Rancher issues a warning suggesting to unset it should any issues be encountered when fetching charts.
Testing
Engineering Testing
Manual Testing
Having set
CATTLE_FLEET_VERSION
to103.1.0+up0.9.0-rc.5
(no longer available inrancher/charts
branchdev-v2.8
, since Fleet0.9.0
has been un-RC-ed), tested the following use cases:Leaving new setting
CATTLE_CHART_DEFAULT_URL
unset: Fleet is installed in version0.9.0
(bug fixed via [2.8] Fix check for exact version in Helm index #43498 ; using code from that PR leads to Rancher not installing Fleet and outputting an error instead, as expected)Setting
CATTLE_CHART_DEFAULT_URL
to an empty value: same behaviour as in 1.Having created a fork of
rancher/charts
, created atest-revert-0-9-0
branch on that fork, reverting the commit un-RC-ing Fleet 0.9.0, so that Fleet0.9.0-rc.5
would be available on that branch. SetCATTLE_CHART_DEFAULT_BRANCH
totest-revert-0-9-0
andCATTLE_CHART_DEFAULT_URL
to the fork's URL, namelyhttps://github.com/weyfonk/charts
. Could see Fleet installed in version0.9.0-rc.5
.Automated Testing
QA Testing Considerations
Regressions Considerations
N/A