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

do not allow multiple Kiali CRs with CWA=true and the same instance_name #849

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

jmazzitelli
Copy link
Contributor

@jmazzitelli jmazzitelli commented Nov 18, 2024

part of: kiali/kiali#7922

see the docs PR that will warn against this condition

@jmazzitelli jmazzitelli added the requires docs PR Requires kiali.io or other documentation updates label Nov 18, 2024
@jmazzitelli jmazzitelli self-assigned this Nov 18, 2024
@jmazzitelli
Copy link
Contributor Author

jmazzitelli commented Nov 18, 2024

ran a couple of the molecule tests that are relevant... all pass

./hack/k8s-minikube.sh --dex-enabled true -mp ci start
./hack/k8s-minikube.sh -mp ci istio
make -e CLUSTER_TYPE=minikube MINIKUBE_PROFILE=ci build build-ui test cluster-push
./hack/run-molecule-tests.sh --client-exe "$(which kubectl)" --minikube-exe $(which minikube) --minikube-profile ci --cluster-type minikube -udi true -at "instance-name-test cluster-wide-access-test accessible-namespaces-test"
=====================
=== TEST RESULTS: ===
=====================

                      instance-name-test... success [2m 56s]
                cluster-wide-access-test... success [8m 44s]
              accessible-namespaces-test... success [7m 25s]

@jmazzitelli
Copy link
Contributor Author

To test:

  1. Install minikube and Istio
 ./hack/k8s-minikube.sh start && ./hack/k8s-minikube.sh -mp ci istio
  1. Build, push, and deploy the operator and server
make -e CLUSTER_TYPE=minikube build build-ui cluster-push operator-create kiali-create
  1. At this point, you have a dev build of the operator and server running, with the server deployed with CWA=true. Now try to create another Kiali CR with CWA=true in a different namespace, but leave the instance_name the same as the original (which is the default kiali):
kubectl apply -f - <<EOM
apiVersion: kiali.io/v1alpha1
kind: Kiali
metadata:
  name: kiali
  namespace: default
spec:
  auth:
    strategy: anonymous
  deployment:
    cluster_wide_access: true
    namespace: default
    image_name: localhost:5000/kiali/kiali
    image_version: dev
    service_type: ClusterIP
  istio_namespace: istio-system
  version: default
EOM
  1. Look at the status field of the Kiali CR and see that eventually the operator will report a failure and the reconciliation will fail:
kubectl -n default get kiali kiali -o jsonpath='{.status}' | jq

results in:

{
  "conditions": [
    {
      "ansibleResult": {
        "changed": 1,
        "completion": "2024-11-19T16:04:49.327172",
        "failures": 1,
        "ok": 23,
        "skipped": 14
      },
      "lastTransitionTime": "2024-11-19T16:04:49Z",
      "message": "There is already a Kiali Server installed with `deployment.instance_name` of [kiali] that has 'deployment.cluster_wide_access' set to true. You must use a different instance name.",
      "reason": "Failed",
      "status": "True",
      "type": "Failure"
    },
...
  1. Update your Kiali CR with a different instance_name but leave everything else the same. This will allow the installation to succeed:
kubectl patch kiali kiali -n default --type=merge -p '{"spec":{"deployment":{"instance_name":"anotherkiali"}}}'
  1. Eventually, it will succeed:
kubectl -n default get kiali kiali -o jsonpath='{.status}' | jq

shows normal status. And you should have a Kiali pod in the default namespace:

$ kubectl get pods -n default -l app.kubernetes.io/name=kiali
NAME                            READY   STATUS    RESTARTS   AGE
anotherkiali-65d745d756-76hhs   1/1     Running   0          105s

@jmazzitelli jmazzitelli marked this pull request as ready for review November 19, 2024 16:12
@ferhoyos ferhoyos self-requested a review November 25, 2024 07:49
Copy link
Collaborator

@ferhoyos ferhoyos left a comment

Choose a reason for hiding this comment

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

LGTM. Tested the provided instructions and received the expected error message.

image

When I change the instance name, the installation is successful:

image

@jmazzitelli jmazzitelli merged commit f4a0d4c into kiali:master Nov 25, 2024
1 check passed
@jmazzitelli jmazzitelli deleted the multiple-cwa branch November 25, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires docs PR Requires kiali.io or other documentation updates
Projects
Development

Successfully merging this pull request may close these issues.

2 participants