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

Add support for Istio multi-cluster #447

Merged
merged 6 commits into from
Feb 20, 2020

Conversation

viditganpi
Copy link

Body:-
If applied this commit will resolve the following reported issue #437
Have added support for consuming kubeconfig of istio host cluster where istio
resources will be created.

Body:-
If applied this commit will resolve the following reported issue fluxcd#437
Have added support for consuming kubeconfig of istio host cluster where istio
resources will be created.
@stefanprodan stefanprodan changed the title Subject:- Canary support for istio multi cluster Add support for Istio multi-cluster Feb 18, 2020
@stefanprodan
Copy link
Member

stefanprodan commented Feb 18, 2020

@viditganpi can you please describe how was this tested? Which of the multi-cluster setups have you used https://istio.io/docs/setup/install/multicluster/?

@viditganpi
Copy link
Author

@viditganpi can you please describe how was this tested? Which of the multi-cluster setups have you used https://istio.io/docs/setup/install/multicluster/?

I have used this setup -> https://istio.io/docs/setup/install/multicluster/shared-vpn/

@stefanprodan
Copy link
Member

Ok so for this to work, Flagger Helm chart should have a istio.kubeconfig option that allows users to set the secret name for the host cluster. When the secret is set, we should create and mount a volume inside Flagger container and point -kubeconfig-host to that path.

   and service kubeconfig
2. Modified deployment to pass and mount secrets
   for configs.
@viditganpi
Copy link
Author

Ok so for this to work, Flagger Helm chart should have a istio.kubeconfig option that allows users to set the secret name for the host cluster. When the secret is set, we should create and mount a volume inside Flagger container and point -kubeconfig-host to that path.

done that

charts/flagger/values.yaml Outdated Show resolved Hide resolved
@@ -111,20 +113,26 @@ func main() {
if err != nil {
logger.Fatalf("Error building kubeconfig: %v", err)
}

fmt.Println(cfg)
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please.

- -kubeconfig=/tmp/istio-source/config1
{{- end }}
{{- if .Values.kubeconfigHost }}
- -kubeconfig-host=/tmp/istio-host/config1
Copy link
Member

Choose a reason for hiding this comment

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

Replace config1 with kubeconfig

@@ -90,6 +110,12 @@ spec:
{{- if .Values.eventWebhook }}
- -event-webhook={{ .Values.eventWebhook }}
{{- end }}
{{- if .Values.kubeconfigSource }}
- -kubeconfig=/tmp/istio-source/config1
Copy link
Member

Choose a reason for hiding this comment

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

Replace config1 with kubeconfig

    1. Modified deployment.yaml to remove source config
    2. Modified values to change default kubeconfigHost values
    3. Removed debugging logs
@stefanprodan
Copy link
Member

CI is failing because the code is not properly formatted, please run go fmt on main.go

@stefanprodan
Copy link
Member

You've pushed a binary file called main to the PR, please remove it.

@viditganpi
Copy link
Author

@viditganpi can you please describe how was this tested? Which of the multi-cluster setups have you used https://istio.io/docs/setup/install/multicluster/?

To test I created a secret from host cluster kubeconfig and mounted it with flagger deployment in service cluster.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @viditganpi

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