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

docs: correct helm repo #657

Merged
merged 4 commits into from
Oct 13, 2021
Merged

docs: correct helm repo #657

merged 4 commits into from
Oct 13, 2021

Conversation

SergeBakharev
Copy link
Contributor

Please answer these questions before submitting a pull request

  • Why submit this pull request?

  • [ x ] Bugfix

  • New feature provided

  • Improve performance

  • Backport patches

  • Related issues


Bugfix

  • Description
    Documentation for helm chart repo was out of date

@gxthrj
Copy link
Contributor

gxthrj commented Sep 1, 2021

Please add a choice about using helm repo to install apisix ingress .

@SergeBakharev
Copy link
Contributor Author

Happy to write some extra notes, but I don't understand myself. Why would someone install just the ingress-controller without APISIX? Is this something you would do to enable helm upgrade capability to target specifically the ingress-controller component without upgrading APISIX version as well?

@tokers
Copy link
Contributor

tokers commented Sep 3, 2021

Happy to write some extra notes, but I don't understand myself. Why would someone install just the ingress-controller without APISIX? Is this something you would do to enable helm upgrade capability to target specifically the ingress-controller component without upgrading APISIX version as well?

Well, we can install apisix and apisix-ingress-controller within one helm install command, and unfortunately, we're not capable to control the start order of apisix-ingress-controller and apisix. So here we use the initContainer to detect the apisix.

@tao12345666333 tao12345666333 added this to the 1.3.0 milestone Sep 5, 2021
@tao12345666333 tao12345666333 self-assigned this Sep 14, 2021
@tao12345666333
Copy link
Member

Hello, do you have time to continue to adjust this PR? @SergeBakharev

I suggest to keep two ways

  • Use chart published to Helm repo
  • Use the source code repository of helm-chart

@tao12345666333
Copy link
Member

We plan to enter the code freeze period and release the v1.3 version. If possible, I hope to include this change

@tao12345666333
Copy link
Member

@SergeBakharev We plan to release the v1.3 version, and I will improve it on the basis of your PR to merge it

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

The current helm chart can using CI automatic release, I think it is unnecessary to keep the method of installing from the source code repository of helm chart.

Let's merge it and move forward

@gxthrj gxthrj merged commit a290f12 into apache:master Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants