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

support ingress class_name customization #113

Merged
merged 1 commit into from
Nov 3, 2021

Conversation

jmazzitelli
Copy link
Contributor

@jmazzitelli jmazzitelli commented Oct 29, 2021

part of: kiali/kiali#4342
operator PR: kiali/kiali-operator#440
docs PR: kiali/kiali.io#466

This refactors the ingress config so it is all under the dict "ingress" found under deployment:

deployment:
  ingress:
    class_name: nginx
    #enabled:
    #override_yaml:

We now will turn off ingress by default when on Kubernetes (we still enable it by default on OpenShift because the default auth strategy (openshift) on OpenShift requires the route - so we should enable the ingress (aka Route) when on OpenShift by default).

If you enable the ingress (set deployment.ingress.enabled=true) when on non-OpenShift Kubernetes, then the default class name will be set to ingressClassName: nginx in the Ingress resource. If you do NOT want any class name defined in the Ingress resource (for example, you are on an older k8s cluster that does not support spec.ingressClassName) then set deployment.ingress.class_name to an empty string (e.g. --set deployment.ingress.class_name=).

We are going to have our defaults specific to NGinx (hence why class_name defaults to "nginx"). Our annotations are already specific to NGinx so it makes sense to default the class name to nginx. For other ingress types, the user is free to change class_name or go further and set override_yaml as before (specifying either metadata.annotation or spec or both).

Along with this PR and kiali/kiali-operator#440 we are going to update the Kiali.io documentation to better indicate to users that the default Ingress that is created is geared towards NGinx ingress implementation when enabled.

@jmazzitelli
Copy link
Contributor Author

jmazzitelli commented Nov 2, 2021

Test procedures.

  1. First, install Minikube with the ingress addon. You can use the hack/k8s-minikube.sh for this.
  2. Build the helm charts make clean build-helm-charts.
  3. Make sure the operator PR is checked out and push the dev container images to the minikube cluster: make -e CLUSTER_TYPE=minikube build cluster-push

Now that everything is setup - you can test the operator and server helm charts.

=== Test via operator helm chart ===

  1. Install the operator, use the dev images (so the operator PR changes are picked up) and enable the ingress:
helm install -n kiali-operator --create-namespace kiali-operator _output/charts/kiali-operator-*-SNAPSHOT.tgz \
  --set allowAdHocKialiImage=true \
  --set image.repo=localhost:5000/kiali/kiali-operator \
  --set image.tag=dev \
  --set cr.spec.deployment.image_name=localhost:5000/kiali/kiali \
  --set cr.spec.deployment.image_version=dev \
  --set cr.create=true \
  --set cr.spec.deployment.ingress.enabled=true
  1. Now confirm the Ingress is created and you can access Kiali.

First make sure the ingress was created (kubectl get ingress kiali -n kiali-operator). Then access the Kiali UI:

IP="$(kubectl get ingress kiali -n kiali-operator -o jsonpath='{.status.loadBalancer.ingress[0].ip}')"
xdg-open http://${IP}/kiali

Your browser should open and show you the Kiali login screen asking for a token. If you see the token login screen, the test passed.

  1. Edit the Kiali CR (kubectl edit kiali kiali -n kiali-operator) and remove the deployment.ingress.enabled setting. Do NOT specify any value for the ingress enabled flag. This will force the operator to fallback to the default value which is "false" when on non-OpenShift clusters.

  2. Wait for the reconciliation to finish and you should see the Ingress get deleted. Run kubectl get ingress kiali -n kiali-operator and see that it results in 0 resources.

  3. Uninstall the CR and then the operator: kubectl delete kiali kiali -n kiali-operator && helm uninstall -n kiali-operator kiali-operator

=== Test via server helm chart ===

  1. Install the Kiali server
helm install -n istio-system kiali-server _output/charts/kiali-server-*-SNAPSHOT.tgz \
  --set deployment.image_name=localhost:5000/kiali/kiali \
  --set deployment.image_version=dev \
  --set deployment.ingress.enabled=true
  1. Now confirm the Ingress is created and you can access Kiali.

First make sure the ingress was created (kubectl get ingress kiali -n istio-system). Then access the Kiali UI:

IP="$(kubectl get ingress kiali -n istio-system -o jsonpath='{.status.loadBalancer.ingress[0].ip}')"
xdg-open http://${IP}/kiali

Your browser should open and show you the Kiali login screen asking for a token. If you see the token login screen, the test passed.

  1. Update the server install but do NOT specify the deployment.ingress.enabled flag - let it default to 'false':
helm upgrade -n istio-system kiali-server _output/charts/kiali-server-*-SNAPSHOT.tgz \
  --set deployment.image_name=localhost:5000/kiali/kiali \
  --set deployment.image_version=dev
  1. You should see the Ingress get deleted. Run kubectl get ingress kiali -n istio-system and see that it results in 0 resources.

  2. Uninstall the server: helm uninstall -n istio-system kiali-server

@jmazzitelli jmazzitelli requested a review from nrfox November 3, 2021 15:18

== Chart Source

Kiali Operator helm chart source is found in the link:./kiali-operator[kiali-operator folder].
Kiali Server helm chart source is found in the link:./kiali-server[kiali-server folder].

== Developer Notes
Copy link
Contributor

Choose a reason for hiding this comment

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

These are helpful, thanks!

@jmazzitelli jmazzitelli merged commit ad516b1 into kiali:master Nov 3, 2021
@jmazzitelli jmazzitelli deleted the 4342-ingress-class branch November 3, 2021 18:52
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.

2 participants