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

[EKS] Change Kubernetes control plane to EKS (the eks branch) #7808

Merged
merged 536 commits into from
Mar 6, 2025
Merged

Conversation

mikkeloscar
Copy link
Contributor

@mikkeloscar mikkeloscar commented Jul 5, 2024

This branch contains a Proof of Concept setup for running a Zalando Kubernetes cluster using EKS as the control plane. All the EKS specific configuration is behind feature flags: {{ if eq .Cluster.Provider "zalando-eks"}} making it compatible with non-eks setup.

This PR is open to facilitate merging changes from dev into the branch regularly and to get it into a state that makes it mergeable.

Note: This branch currently only works with a modified version of CLM: zalando-incubator/cluster-lifecycle-manager#777

@mikkeloscar mikkeloscar added do-not-merge architectural New features and architectural changes, e.g. framework changes, migrations, rollout of new services. labels Jul 5, 2024
@linki linki changed the title [WIP] EKS based cluster PoC [EKS] Change Kubernetes control plane to EKS (the eks branch) Sep 3, 2024
@linki linki marked this pull request as ready for review September 3, 2024 08:07
Name: '{{.Cluster.ID}}:worker-iam-role'
Value: !Ref WorkerIAMRole
RemoteFilesEncryptionKey:
Name: "{{.Cluster.ID}}:eks-oidc-url"
Copy link
Member

Choose a reason for hiding this comment

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

Should we use .Cluster.Name instead of .ID here (and other places?). PR that introduced the field: https://github.com/zalando-incubator/cluster-lifecycle-manager/pull/829/files#diff-61dbe200b05c4fa62230dc5e9efedf4ac8d35088d3dfb7a1cde9163b79b41066

- "@127.0.0.1"
{{- else }}
- "::1"
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we enter this branch for legacy clusters (IPv4) as well. Does this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

And should it be @::1?

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 is unbound which is not yet being activated in EKS clusters. Let's defer fixing this to later.

args:
- tcp
# TODO: ipv6
- --bind=0.0.0.0:9054
Copy link
Member

Choose a reason for hiding this comment

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

Should this bind to all interfaces (for IPv6)?

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 is unbound which I didn't invest in making ipv6 compatible yet. We use dnsmasq in eks clusters for now so we can defer fixing this to later.

@@ -123,8 +144,8 @@ spec:
- --neg-ttl=60
# send requests to the last server first, only fallback to the previous ones if it's unreachable
- --strict-order
- --server=10.5.0.11#53
- --server=127.0.0.1#9254
- --server=10.5.0.11#53 # TODO: fix this for ipv6
Copy link
Member

Choose a reason for hiding this comment

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

Should this be an IPv6 address?

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 think in practice this address is never used as it's a fallback if the local CoreDNS is not available. Hence we didn't notice issues in IPv6 clusters.

We should fix this, but I would say we don't have to fix it before merging this.

@@ -52,6 +53,7 @@ spec:
- --aws-zones-cache-duration={{ .Cluster.ConfigItems.external_dns_zones_cache_duration }}
- --annotation-filter=external-dns.alpha.kubernetes.io/exclude notin (true)
- --policy={{ .Cluster.ConfigItems.external_dns_policy }}
- --log-level=debug
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop this at some point.

@linki linki added the frozen new updates to the dev branch won't be added to dev-to-alpha pull request label Mar 6, 2025
@linki
Copy link
Member

linki commented Mar 6, 2025

👍

1 similar comment
@mikkeloscar
Copy link
Contributor Author

👍

@mikkeloscar mikkeloscar merged commit d9eb3b3 into dev Mar 6, 2025
12 of 14 checks passed
@mikkeloscar mikkeloscar deleted the eks branch March 6, 2025 14:03
@k8s-on-aws-manager-app k8s-on-aws-manager-app bot mentioned this pull request Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architectural New features and architectural changes, e.g. framework changes, migrations, rollout of new services. do-not-merge frozen new updates to the dev branch won't be added to dev-to-alpha pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants