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

Unhardcode cluster.local domain #34

Closed
kvaps opened this issue Jun 16, 2024 · 10 comments
Closed

Unhardcode cluster.local domain #34

kvaps opened this issue Jun 16, 2024 · 10 comments

Comments

@kvaps
Copy link

kvaps commented Jun 16, 2024

Hi, Cozystack.io team here.

We have a problem of adopting fluxcd-opeator, because it expects user to explicitly specify cluster domain.

In fact user might have different domains for their clusters, and requirement for explicitly specifying the domain can be ommited which simplifying user experience and lowering down their cognitive load:

For the implementation, this default value should be removed:

This check:

options.EventsAddr = fmt.Sprintf("http://%s.%s.svc.%s./", options.NotificationController, options.Namespace, options.ClusterDomain)

should be extended to somewhat like

  if options.ClusterDomain == "" {
  options.EventsAddr = fmt.Sprintf("http://%s.%s.svc/", options.NotificationController, options.Namespace)
  } else {
  options.EventsAddr = fmt.Sprintf("http://%s.%s.svc.%s./", options.NotificationController, options.Namespace, options.ClusterDomain)
  }
@stefanprodan
Copy link
Member

The cluster domain is not hardcoded, we just provide a default, in the same way upstream Flux does it. Users can specify a different value in the FluxInstance custom resource.

@stefanprodan
Copy link
Member

Also simplifying user experience would result in major performance issues as every single Flux request between controllers will result in x5 DNS calls. Given that upstream Flux does not allow users to remove the cluster domain, neither will the operator.

@kvaps
Copy link
Author

kvaps commented Jun 16, 2024

every single Flux request between controllers will result in x5 DNS calls

Okay how about using single DNS name?
Looks like go client produces the same amount of DNS requests as in case with FQDN:

package main

import (
	"fmt"
	"net"
	"os"
)

func main() {
	lookupName := "source-controller"

	// 06:12:03.205420 eth0  Out IP 10.244.0.110.54912 > 10.244.0.21.53: 24129+ [1au] AAAA? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:12:03.205590 eth0  Out IP 10.244.0.110.46616 > 10.244.0.99.53: 2891+ [1au] A? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:12:03.205934 eth0  In  IP 10.244.0.99.53 > 10.244.0.110.46616: 2891*- 1/0/1 A 10.96.152.120 (133)
	// 06:12:03.208859 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.54912: 24129*- 0/1/1 (157)

	lookupName := "source-controller.cozy-fluxcd.svc"

	// 06:15:10.821547 eth0  Out IP 10.244.0.110.58012 > 10.244.0.99.53: 28057+ [1au] AAAA? source-controller.cozy-fluxcd.svc.cozy-fluxcd.svc.cozy.local. (89)
	// 06:15:10.821550 eth0  Out IP 10.244.0.110.60395 > 10.244.0.99.53: 62286+ [1au] A? source-controller.cozy-fluxcd.svc.cozy-fluxcd.svc.cozy.local. (89)
	// 06:15:10.822577 eth0  In  IP 10.244.0.99.53 > 10.244.0.110.58012: 28057 NXDomain*- 0/1/1 (173)
	// 06:15:10.822620 eth0  In  IP 10.244.0.99.53 > 10.244.0.110.60395: 62286 NXDomain*- 0/1/1 (173)
	// 06:15:10.822731 eth0  Out IP 10.244.0.110.43877 > 10.244.0.21.53: 11069+ [1au] AAAA? source-controller.cozy-fluxcd.svc.svc.cozy.local. (77)
	// 06:15:10.822884 eth0  Out IP 10.244.0.110.43062 > 10.244.0.21.53: 13794+ [1au] A? source-controller.cozy-fluxcd.svc.svc.cozy.local. (77)
	// 06:15:10.824150 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.43062: 13794 NXDomain*- 0/1/1 (161)
	// 06:15:10.824183 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.43877: 11069 NXDomain*- 0/1/1 (161)
	// 06:15:10.824261 eth0  Out IP 10.244.0.110.60994 > 10.244.0.99.53: 2853+ [1au] AAAA? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:15:10.824305 eth0  Out IP 10.244.0.110.51709 > 10.244.0.21.53: 10284+ [1au] A? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:15:10.824388 eth0  In  IP 10.244.0.99.53 > 10.244.0.110.60994: 2853*- 0/1/1 (157)
	// 06:15:10.824755 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.51709: 10284*- 1/0/1 A 10.96.152.120 (133)

	lookupName := "source-controller.cozy-fluxcd.svc.cluster.local."

	// 06:14:24.713070 eth0  Out IP 10.244.0.110.41175 > 10.244.0.21.53: 59733+ [1au] AAAA? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:14:24.713097 eth0  Out IP 10.244.0.110.46419 > 10.244.0.21.53: 26406+ [1au] A? source-controller.cozy-fluxcd.svc.cozy.local. (73)
	// 06:14:24.715069 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.41175: 59733*- 0/1/1 (157)
	// 06:14:24.715130 eth0  In  IP 10.244.0.21.53 > 10.244.0.110.46419: 26406*- 1/0/1 A 10.96.152.120 (133)

	addrs, err := net.LookupHost(lookupName)
	if err != nil {
		fmt.Fprintf(os.Stderr, "Failed to lookup host: %v\n", err)
		os.Exit(1)
	}
	fmt.Printf("Addresses for %s: %v\n", lookupName, addrs)
}

@stefanprodan
Copy link
Member

We did use single DNS name back when we started Flux v2 and it didn't work on GOV private cloud hosted on GKE and AKS clusters, as CoreDNS is configured there to skip resolving names without the cluster domain.

@kvaps
Copy link
Author

kvaps commented Jun 16, 2024

Roger that. I will think how to workaround this issue.
We probably can add the second cluster domain into coredns configuration.

Thanks for expansion

@stefanprodan
Copy link
Member

How did you used Flux until now? The Flux CLI, TF provider and the community Helm chart all are setting the cluster domain.

@kvaps
Copy link
Author

kvaps commented Jun 16, 2024

We're preserving all the helm charts and images in our repo, just in case to implement on-fly fixes on them:
The fluxcd helm chart was fixed to not use FQDNs

Currently fluxcd is the only application which requires this.

@stefanprodan
Copy link
Member

If you're using the operator, then it's super easy to drop the cluster domain, just do:

apiVersion: fluxcd.controlplane.io/v1
kind: FluxInstance
metadata:
  name: flux
  namespace: flux-system
spec:
  distribution:
    version: "2.x"
    registry: "ghcr.io/fluxcd"
  kustomize:
    patches:
      - target:
          kind: Deployment
          name: source-controller
        patch: |
          - op: add
            path: /spec/template/spec/containers/0/args/-
            value: --storage-adv-addr=source-controller
          - op: add
            path: /spec/template/spec/containers/0/args/-
            value: --events-addr=http://notification-controller/
      - target:
          kind: Deployment
          name: (kustomize-controller|helm-controller|image-reflector-controller|image-automation-controller)
        patch: |
          - op: add
            path: /spec/template/spec/containers/0/args/-
            value: --events-addr=http://notification-controller/

@kvaps
Copy link
Author

kvaps commented Jun 16, 2024

@stefanprodan it seems this is the most elegant solution. Thank you!

@kvaps
Copy link
Author

kvaps commented Jun 17, 2024

Just noticed the problem with singlenames, we use kubeapps dashboard which is deployed in different namespace, it takes url from status.artifact.url which is now looking like:

http://source-controller/helmrepository/cozy-public/cozystack-apps/index-4220e6b6ac4dc145f454f2681cf608bb9f1526288639994b2f83660e61160535.yaml

and obviously can't reach it

I0617 13:30:45.550692       1 repo.go:653] +indexOneRepo: [cozystack-apps], index URL: [http://source-controller/helmrepository/cozy-public/cozystack-apps/index.yaml]
failed due to: Get "http://source-controller/helmrepository/cozy-public/cozystack-apps/index.yaml": dial tcp: lookup source-controller on 10.96.0.10:53: no such host

full status:

status:
  artifact:
    digest: sha256:d14a8fd7d8e5805c4118fcaa6500413fe303071bbe60271d7eb667f913987fe8
    lastUpdateTime: "2024-06-17T13:25:58Z"
    path: helmrepository/cozy-public/cozystack-apps/index-4220e6b6ac4dc145f454f2681cf608bb9f1526288639994b2f83660e61160535.yaml
    revision: sha256:4220e6b6ac4dc145f454f2681cf608bb9f1526288639994b2f83660e61160535
    size: 18496
    url: http://source-controller/helmrepository/cozy-public/cozystack-apps/index-4220e6b6ac4dc145f454f2681cf608bb9f1526288639994b2f83660e61160535.yaml

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

No branches or pull requests

2 participants