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

Install offline Helm Chart for a multi-cluster #3897

Merged
merged 22 commits into from
May 17, 2023
Merged

Install offline Helm Chart for a multi-cluster #3897

merged 22 commits into from
May 17, 2023

Conversation

nioshield
Copy link
Contributor

@nioshield nioshield commented Jan 13, 2023

What problem does this PR solve?

Install the specified version Helm Chart offline to multiple clusters

What's changed and how it works?

Related changes

  • This change also requires further updates to the website (e.g. docs)
  • This change also requires further updates to the UI interface
  • Need to cheery-pick to release branches
    • release-2.5
    • release-2.4

Checklist

CHANGELOG

  • I have updated the CHANGELOG.md
  • I have labeled this PR with "no-need-update-changelog"

Tests

  • Unit test
  • E2E test
  • No code
  • Manual test (add steps below)

Side effects

  • Breaking backward compatibility

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@codecov
Copy link

codecov bot commented Jan 13, 2023

Codecov Report

Merging #3897 (a0ed2f9) into master (73117dc) will decrease coverage by 0.01%.
The diff coverage is 14.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3897      +/-   ##
==========================================
- Coverage   38.61%   38.61%   -0.01%     
==========================================
  Files         167      167              
  Lines       13729    13749      +20     
==========================================
+ Hits         5302     5309       +7     
- Misses       7998     8006       +8     
- Partials      429      434       +5     
Impacted Files Coverage Δ
...ntrollers/multicluster/remotecluster/controller.go 0.00% <0.00%> (ø)
pkg/config/controller.go 0.00% <ø> (ø)
pkg/helm/chart.go 36.00% <26.08%> (-9.46%) ⬇️

... and 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ad66d9...a0ed2f9. Read the comment docs.

@nioshield
Copy link
Contributor Author

/cc @STRRL

@STRRL
Copy link
Member

STRRL commented Jan 31, 2023

Hi @nioshield , thanks for your contribution! Awesome patch!

I am going to review it soon.

helm/chaos-mesh/values.yaml Outdated Show resolved Hide resolved
@@ -189,7 +189,7 @@ func (r *Reconciler) ensureHelmRelease(ctx context.Context, obj *v1alpha1.Remote
_, err = helmClient.GetRelease(obj.Spec.Namespace, chaosMeshReleaseName)
if err != nil {
if errors.Is(err, driver.ErrReleaseNotFound) {
chart, err := helm.FetchChaosMeshChart(ctx, chaosMeshReleaseVersion)
chart, err := helm.FetchChaosMeshChart(ctx, obj.Spec.Version, config.ControllerCfg.LocalHelmChartPath)
Copy link
Member

Choose a reason for hiding this comment

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

The logic about fetching Status.CurrentVersion needs updates.

Fields in Status means the observed status by the controller, it should NOT be setup by simply copying the value from Spec.Version.

What about making a refactor on helm.FetchChaosMeshChart, derive 2 method like:

  • helm.FetchChaosMeshChartFromRepository(ctx context.Context, version string) (chart, version ,err)
  • helm.FetchChaosMeshChartFromLocal(ctx context.Context, helmChartPath string) (chart, version ,err)

then setup the Status.CurrentVersion with the returned version value.

pkg/helm/chart.go Outdated Show resolved Hide resolved
Copy link
Member

@STRRL STRRL left a comment

Choose a reason for hiding this comment

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

Basically LGTM!

there are several comments about the codes. PTAL

Thanks! @nioshield

@nioshield
Copy link
Contributor Author

@STRRL fixed according to comments. PTAL

@nioshield nioshield requested review from STRRL and removed request for cwen0 February 13, 2023 11:19
Copy link
Member

@STRRL STRRL left a comment

Choose a reason for hiding this comment

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

rest LGTM!

also, please fix the DCO check

pkg/helm/chart.go Outdated Show resolved Hide resolved
nioshield and others added 11 commits March 17, 2023 11:17
Signed-off-by: nio <nioshield@gmail.com>
Signed-off-by: nio <nioshield@gmail.com>
Signed-off-by: STRRL <im@strrl.dev>
Signed-off-by: nio <nioshield@gmail.com>
* Update install.sh to work on macos

The default bash version on MacOS does not support
reading an array directly from a variable. Instead, use
`read -a` to accomplish separating version numbers.

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

* chore: update changelog

Signed-off-by: STRRL <im@strrl.dev>

---------

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
Signed-off-by: STRRL <im@strrl.dev>
Co-authored-by: STRRL <im@strrl.dev>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: nio <nioshield@gmail.com>
* update chaos-coredns version to v0.2.4

Signed-off-by: Xianglin Gao <xianglingao@tencent.com>

* update chaos-coredns version to v0.2.4

Signed-off-by: Xianglin Gao <xianglingao@tencent.com>

---------

Signed-off-by: Xianglin Gao <xianglingao@tencent.com>
Signed-off-by: nio <nioshield@gmail.com>
This reverts commit b031d88.

Signed-off-by: nio <nioshield@gmail.com>
* chore: init

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* test: add jest-dom types

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: add @mui/base

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: types

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: update

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: clear yarn usage

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: update changelog

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: update storyshots

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: downgrade react-testing-library

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: @ui/app tests

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: unknown unsafe-perm

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

---------

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: nio <nioshield@gmail.com>
* upgrade dns coredns image url to ghcr.io

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* add CHANGELOG

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* replace pingcap project with chaos-mesh

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* add back  docker-push-dns-server

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>

* chore: bump the version of chaos-coredns to v0.2.2

Signed-off-by: STRRL <im@strrl.dev>

* update Makefile

Signed-off-by: Cwen Yin <cwenyin0@gmail.com>

* chore: bump coredns image version to 0.2.4

Signed-off-by: STRRL <im@strrl.dev>

* test: use latest image

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: follow sonatype-lift suggestions

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* chore: update chaos-coredns to v0.2.5

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: changelog

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: typo

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: update dnsServer.image

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: remove pingcap

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: typo

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* fix: add ghcr.io registry

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Rename ChaosDNSImage to ChaosCoreDNSImage

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* feat: bump chaos-coredns version to v0.2.6, and enable dnsServer.create as default

Signed-off-by: STRRL <im@strrl.dev>

* chore: update the helm chart configs

Signed-off-by: STRRL <im@strrl.dev>

* chore: make check

Signed-off-by: STRRL <im@strrl.dev>

---------

Signed-off-by: YangKeao <yangkeao@chunibyo.icu>
Signed-off-by: STRRL <im@strrl.dev>
Signed-off-by: Cwen Yin <cwenyin0@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Co-authored-by: STRRL <im@strrl.dev>
Co-authored-by: Cwen Yin <cwenyin0@gmail.com>
Co-authored-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: nio <nioshield@gmail.com>
Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
Signed-off-by: nio <nioshield@gmail.com>
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
Signed-off-by: nio <nioshield@gmail.com>
Co-authored-by: Zhou Zhiqiang <im@strrl.dev>
Signed-off-by: nio <nioshield@gmail.com>
@STRRL
Copy link
Member

STRRL commented May 7, 2023

/merge

@STRRL
Copy link
Member

STRRL commented May 9, 2023

/lgtm

@chaotic-prow chaotic-prow bot added the lgtm label May 9, 2023
@STRRL
Copy link
Member

STRRL commented May 9, 2023

/approve

1 similar comment
@STRRL
Copy link
Member

STRRL commented May 9, 2023

/approve

@STRRL
Copy link
Member

STRRL commented May 9, 2023

/lgtm

@chaotic-prow chaotic-prow bot added the approved label May 9, 2023
@chaotic-prow chaotic-prow bot removed the lgtm label May 9, 2023
@g1eny0ung
Copy link
Member

/lgtm

@g1eny0ung
Copy link
Member

/retest

@chaotic-prow chaotic-prow bot removed the lgtm label May 17, 2023
@g1eny0ung
Copy link
Member

/lgtm

@chaotic-prow chaotic-prow bot added the lgtm label May 17, 2023
@chaotic-prow
Copy link

chaotic-prow bot commented May 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: g1eny0ung, STRRL

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

@chaotic-prow chaotic-prow bot merged commit a58b954 into chaos-mesh:master May 17, 2023
60 checks passed
@g1eny0ung
Copy link
Member

Supplement docs in chaos-mesh/website@ba2b18d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants