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

[Improvement]: Ingress configration for ams when deploying with Helm. #2020

Merged
merged 6 commits into from
Sep 28, 2023

Conversation

baiyangtx
Copy link
Contributor

@baiyangtx baiyangtx commented Sep 21, 2023

Why are the changes needed?

Add ingress configuration in Chart. Make Helm user easier to access AMS Dashborad.

By default, applications within a Kubernetes cluster with Ingress are not accessible from outside the cluster. But when an IngressClass is installed in the Kubernetes cluster, it is possible to access the installed application pages on the cluster from outside the cluster by creating Ingress resources.

More information about ingress refer here: https://kubernetes.io/docs/concepts/services-networking/ingress/

Brief change log

  • Add Ingress config in Chart

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes )
  • If yes, how is the feature documented? (not documented)

@baiyangtx baiyangtx changed the title [IMPROVEMENT]: Create Ingress for ams when deploying with helm. [IMPROVEMENT]: Ingress configration for ams when deploying with Helm. Sep 21, 2023
@baiyangtx
Copy link
Contributor Author

baiyangtx commented Sep 21, 2023

values.yaml

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

ingress

$ kubectl get ingress
NAME    CLASS    HOSTS                          ADDRESS        PORTS   AGE
amoro   <none>   amoro.ingress.minikube.local   192.168.49.2   80      87m

$ kubectl describe ingress
Name:             amoro
Labels:           app.kubernetes.io/component=ingress
                  app.kubernetes.io/instance=amoro
                  app.kubernetes.io/managed-by=Helm
                  app.kubernetes.io/name=amoro
                  helm.sh/chart=amoro-0.1.0
Namespace:        default
Address:          192.168.49.2
Ingress Class:    <none>
Default backend:  <default>
Rules:
  Host                          Path  Backends
  ----                          ----  --------
  amoro.ingress.minikube.local  
                                /   amoro-rest:rest (10.244.0.10:1630)
Annotations:                    meta.helm.sh/release-name: amoro
                                meta.helm.sh/release-namespace: default
Events:                         <none>

Visit from ingress url.

image

$ helm install amoro ./amoro -f values.yaml 
NAME: amoro
LAST DEPLOYED: Fri Sep 22 14:11:10 2023
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The Amoro Helm chart has been installed!

If you want to go ams web ui,you can use this cmd forward to local:


  http://amoro.ingress.minikube.local/


Additional Resources Docs

================================================================================================================

        * Documentation: https://amoro.netease.com/docs/latest/
        * Version build Info : https://github.com/NetEase/amoro/releases/tag/v0.5.0

================================================================================================================

@baiyangtx baiyangtx marked this pull request as ready for review September 22, 2023 06:13
@baiyangtx baiyangtx changed the title [IMPROVEMENT]: Ingress configration for ams when deploying with Helm. [Improvement]: Ingress configration for ams when deploying with Helm. Sep 22, 2023
@czy006
Copy link
Contributor

czy006 commented Sep 24, 2023

When I try to value.yaml set ingress config,when "helm dependency build" before install helm chart

values.yaml

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

It have something problem

  • install deployment service account can't create
  • set ingress annotations can't merge have error msg

Error: INSTALLATION FAILED: template: amoro/templates/amoro-ingress.yaml:27:22: executing "amoro/templates/amoro-ingress.yaml" at <include "common.tplvalues.merge" (dict "values" (list .Values.ingress.annotations .Values.commonAnnotations) "context" .)>: error calling include: template: no template "common.tplvalues.merge" associated with template "gotpl"

image

Env:

  • Docker Desktop Version: 4.23.0 (120376)
  • K8S Version: 1.27.2
  • Helm Version: 3.12.3

cc @baiyangtx

Copy link
Contributor

Choose a reason for hiding this comment

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

Hard Add Apache LICENSE 2.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is generated by Helm to record the version information of dependent libraries.

app.kubernetes.io/component: ingress
{{- if or .Values.ingress.annotations .Values.commonAnnotations }}
{{- $annotations := include "common.tplvalues.merge" ( dict "values" ( list .Values.ingress.annotations .Values.commonAnnotations ) "context" . ) }}
annotations: {{- include "common.tplvalues.render" ( dict "value" $annotations "context" $) | nindent 4 }}
Copy link
Contributor

Choose a reason for hiding this comment

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

when I install it set

  annotations:
    kubernetes.io/ingress.class: nginx

that have error helm merge msg: Error: INSTALLATION FAILED: template: amoro/templates/amoro-ingress.yaml:27:22: executing "amoro/templates/amoro-ingress.yaml" at <include "common.tplvalues.merge" (dict "values" (list .Values.ingress.annotations .Values.commonAnnotations) "context" .)>: error calling include: template: no template "common.tplvalues.merge" associated with template "gotpl"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have fixed by upgrading bitnamicharts/common to version 2.11.1

@XBaith
Copy link
Contributor

XBaith commented Sep 25, 2023

I tried installing the chart with the following values, but it doesn't seem to work:

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

@baiyangtx
Copy link
Contributor Author

I tried installing the chart with the following values, but it doesn't seem to work:

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

Do you have an default IngressClass implementation in your cluster ?

@baiyangtx
Copy link
Contributor Author

When I try to value.yaml set ingress config,when "helm dependency build" before install helm chart

values.yaml

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

It have something problem

* install deployment service account can't create

* set ingress annotations can't merge have error msg

Error: INSTALLATION FAILED: template: amoro/templates/amoro-ingress.yaml:27:22: executing "amoro/templates/amoro-ingress.yaml" at <include "common.tplvalues.merge" (dict "values" (list .Values.ingress.annotations .Values.commonAnnotations) "context" .)>: error calling include: template: no template "common.tplvalues.merge" associated with template "gotpl"
image

Env:

* Docker Desktop Version: 4.23.0 (120376)

* K8S Version: 1.27.2

* Helm Version: 3.12.3

cc @baiyangtx

I have fixed the ANNOTATION problems and account problems.

@HuangFru
Copy link
Contributor

Env:

  • Docker:24.0.2-rd
  • Minikube:v1.31.2

values.yaml

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

process:

➜  helm install amoro ../amoro -f values.yaml 
NAME: amoro
LAST DEPLOYED: Mon Sep 25 16:03:12 2023
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The Amoro Helm chart has been installed!

If you want to go ams web ui,you can use this cmd forward to local:


  http://amoro.ingress.minikube.local/


Additional Resources Docs

================================================================================================================

        * Documentation: https://amoro.netease.com/docs/latest/
        * Version build Info : https://github.com/NetEase/amoro/releases/tag/v0.5.0

================================================================================================================

➜  kubectl get ingress
NAME    CLASS    HOSTS                          ADDRESS   PORTS   AGE
amoro   <none>   amoro.ingress.minikube.local             80      19s
➜  kubectl describe ingress
Name:             amoro
Labels:           app.kubernetes.io/component=ingress
                  app.kubernetes.io/instance=amoro
                  app.kubernetes.io/managed-by=Helm
                  app.kubernetes.io/name=amoro
                  app.kubernetes.io/version=0.5.0
                  helm.sh/chart=amoro-0.1.0
Namespace:        default
Address:          
Ingress Class:    <none>
Default backend:  <default>
Rules:
  Host                          Path  Backends
  ----                          ----  --------
  amoro.ingress.minikube.local  
                                /   amoro-rest:rest (10.244.0.5:1630)
Annotations:                    meta.helm.sh/release-name: amoro
                                meta.helm.sh/release-namespace: default
Events:                         <none>

It does not work for me, I can not visit AMS web UI via ' http://amoro.ingress.minikube.local/'. It seems that I lost the address in the ingress.

@baiyangtx
Copy link
Contributor Author

Env:

* Docker:24.0.2-rd

* Minikube:v1.31.2

values.yaml

ingress: 
  enabled: true
  hostname: amoro.ingress.minikube.local

process:

➜  helm install amoro ../amoro -f values.yaml 
NAME: amoro
LAST DEPLOYED: Mon Sep 25 16:03:12 2023
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The Amoro Helm chart has been installed!

If you want to go ams web ui,you can use this cmd forward to local:


  http://amoro.ingress.minikube.local/


Additional Resources Docs

================================================================================================================

        * Documentation: https://amoro.netease.com/docs/latest/
        * Version build Info : https://github.com/NetEase/amoro/releases/tag/v0.5.0

================================================================================================================
➜  kubectl get ingress
NAME    CLASS    HOSTS                          ADDRESS   PORTS   AGE
amoro   <none>   amoro.ingress.minikube.local             80      19s
➜  kubectl describe ingress
Name:             amoro
Labels:           app.kubernetes.io/component=ingress
                  app.kubernetes.io/instance=amoro
                  app.kubernetes.io/managed-by=Helm
                  app.kubernetes.io/name=amoro
                  app.kubernetes.io/version=0.5.0
                  helm.sh/chart=amoro-0.1.0
Namespace:        default
Address:          
Ingress Class:    <none>
Default backend:  <default>
Rules:
  Host                          Path  Backends
  ----                          ----  --------
  amoro.ingress.minikube.local  
                                /   amoro-rest:rest (10.244.0.5:1630)
Annotations:                    meta.helm.sh/release-name: amoro
                                meta.helm.sh/release-namespace: default
Events:                         <none>

It does not work for me, I can not visit AMS web UI via ' http://amoro.ingress.minikube.local/'. It seems that I lost the address in the ingress.

You have not been installed an IngressClass on you k8s Cluster.

run kubectl get ingressclass

@HuangFru
Copy link
Contributor

For macOS M series chips to use the ingress addons in minikube, according to kubernetes/minikube#13510:

  • minikube tunnel is needed.
  • Mapping your domain to 127.0.0.1 instead of the output of minikube ip in the hosts file.

After these things, this PR works fine for me.

Copy link
Contributor

@HuangFru HuangFru left a comment

Choose a reason for hiding this comment

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

LGTM.

@czy006
Copy link
Contributor

czy006 commented Sep 27, 2023

LGTM.

@baiyangtx
Copy link
Contributor Author

For macOS M series chips to use the ingress addons in minikube, according to kubernetes/minikube#13510:

* `minikube tunnel` is needed.

* Mapping your domain to 127.0.0.1 instead of the output of `minikube ip` in the hosts file.

After these things, this PR works fine for me.

Thanks your testing. This is helpful for using on Mac, and I will add this additional configuration to the documentation.

Copy link
Contributor

@majin1102 majin1102 left a comment

Choose a reason for hiding this comment

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

LGTM

@majin1102 majin1102 merged commit b61e6d3 into apache:master Sep 28, 2023
1 check passed
@baiyangtx baiyangtx deleted the helm-ingress branch September 28, 2023 02:59
@shidayang shidayang mentioned this pull request Oct 17, 2023
70 tasks
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
…apache#2020)

* helm: add ingress

* helm: service name

* add helm:build docker and test ingress

* notes improvements

* ignore tgz file

* fix review problems.
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

6 participants